ErikReider / SwayNotificationCenter

A simple GTK based notification daemon for SwayWM
GNU General Public License v3.0
1.17k stars 60 forks source link

Only support compliant markup tags #116

Closed ptrcnull closed 2 years ago

ptrcnull commented 2 years ago
$ swaync

(swaync:16054): Gtk-WARNING **: 04:27:35.762: Failed to set text '<thing> asdf' from markup due to error parsing markup: Error on line 1 char 29: Element “markup” was closed, but the currently open element is “thing”
Could for some reason not set markup. Text: &lt;thing&gt; asdf

(swaync:16054): Gtk-WARNING **: 04:27:35.815: Failed to set text '<thing> asdf' from markup due to error parsing markup: Error on line 1 char 29: Element “markup” was closed, but the currently open element is “thing”
Could for some reason not set markup. Text: &lt;thing&gt; asdf

To reproduce:

$ notify-send test '<thing> asdf'

This is usually an issue with IRC messages which use that format as the message body.

ErikReider commented 2 years ago

It only supports Pango Markup. "" isn't one of them.

Could you post an IRC example? :)

nekopsykose commented 2 years ago

irc messages (sometimes) get sent as notifications with the equivalent of notify-send "buffer/networkname" "<ircuser> message", so the message contents look like an open html tag

if that irc user is also named small and ends their message with \</small>, then it would ensmallen the notification text :)

ErikReider commented 2 years ago

irc messages (sometimes) get sent as notifications with the equivalent of notify-send "buffer/networkname" "<ircuser> message", so the message contents look like an open html tag

if that irc user is also named small and ends their message with , then it would ensmallen the notification text :)

Well that's inconvenient... I'll look into this

ptrcnull commented 2 years ago

there's a similar thing happening with spotify songs that have & in them:

(swaync:20712): Gtk-WARNING **: 02:52:23.916: Failed to set text 'Futurecop! - The Unicorn & the Lost City of Alvograth' from markup due to error parsing markup: Error on line 1: Entity did not end with a semicolon; most likely you used an ampersand character without intending to start an entity — escape ampersand as &amp;
Could for some reason not set markup. Text: Futurecop! - The Unicorn &amp; the Lost City of Alvograth

i don't think swaync should be parsing all that as html really

ErikReider commented 2 years ago

@ptrcnull Do the notifications display correctly? That's an internal warning then trying to parse invalid markup. I'll close the issue for now

https://github.com/ErikReider/SwayNotificationCenter/blob/c54bf8ba16d5558082d5337cf24a87f9956c067b/src/notification/notification.vala#L203-L226

ptrcnull commented 2 years ago

They do, because it falls back to the escaped text; it's still invalid behaviour in my opinion, given that the spec only specifies a handful of valid markup tags and no support for HTML entities at all:

https://specifications.freedesktop.org/notification-spec/notification-spec-latest.html#markup

ErikReider commented 2 years ago

So you're talking about only parsing valid tags? So if I run this notify-send "Summary" "<very_invalid_tag> normal text, <small>very tiny text</small>", only the "small" tag would be parsed?

ptrcnull commented 2 years ago

Yes, that (except that the <small> tag isn't technically supported per specification and " notifications should never take advantage of tags that are not listed above"), and also escaping the & sign before passing it to Pango - a lot of notifications can have a standalone & character and currently it prints 2 warnings to the stderr on every such notification.

As a sidenote, it would be cool to be able to just disable markup entirely in the options and skip exporting the body-markup capability; in the ideal situation, the protocol would have a field to determine whether the message should be parsed as markup, but sadly it doesn't...

ErikReider commented 2 years ago

@ptrcnull does the PR fix your issue?

nekopsykose commented 2 years ago

can confirm it fixes both the \<something> and the & cases