Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Receiving notification content with invalid pango markup breaks format markup #55

Closed
vilhalmer opened this issue Jul 3, 2018 · 5 comments
Labels
bug Something isn't working

Comments

@vilhalmer
Copy link
Collaborator

If the body of a notification contains something that pango considers to be invalid markup, the markup within the user-specified format will be ignored as well (see screenshot for an example). This is because we're parsing the entire formatted notification at once. This is required to have relative markup work correctly, but it could be made more robust. Here's what I propose:

  • Attempt to parse the body alone using pango_parse_markup, without saving the result.
  • If this reported success, continue as usual.
  • Otherwise, escape the body using the existing escape_markup function before formatting.
  • Parse the entire thing as we do now.

This will allow the user's format to continue working (e.g. to display the summary in bold) even when the body is unparseable. It's arguable that this isn't a mako bug, but the result of clients sending notifications that don't follow the spec. However, I've now seen this from two separate programs (including Slack), so it's probably worth putting a bandaid on.


screenshot_2018-07-03-131047
An example of the < character causing parsing to fail.

cannot parse pango markup: Error on line 2 char 7: '<' is not a valid name
@emersion
Copy link
Owner

emersion commented Jul 3, 2018

Hmm, yes, I've experienced this too. The spec allows us to advertise that we support markup, but doesn't allow clients to opt-out. :(

I think the fix you suggest is reasonable.

@emersion emersion added the bug Something isn't working label Jul 16, 2018
@progandy
Copy link

progandy commented Aug 9, 2018

Mako should probably filter unsupported html tags as well and only keep those that are defined in the freedesktop specification. I remember that was an problem in dunst.

Well, in the current version it simply tries pango markup and strips all tags if an error occurs. It seems that was greatly simplified some time ago.

@vilhalmer
Copy link
Collaborator Author

I don't know if there's a good way to do this without accidentally ruining some notifications' content. For example, if someone sends me a message on slack that contains a tag, I don't want that tag to be removed, since it was intended as the literal text of the tag.

@progandy
Copy link

I guess in that case keep the tags for now and when a bug report is filed, add a criteria option to strip tags when receiving notifications from known bad clients.

@vilhalmer
Copy link
Collaborator Author

Hmmm, perhaps the markup criteria should be modified to only affect the received content, but always allow parsing of the markup specified in the user's format? If the user really doesn't want markup at all, they should remove the tags from the format themselves anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants