mkiol / GNotifier

Thunderbird add-on that replaces built-in notifications with the OS native notifications
https://addons.mozilla.org/thunderbird/addon/gnotifier/
GNU General Public License v3.0
164 stars 25 forks source link

Thunderbird message %a produces invalid HTML #192

Closed bebehei closed 6 years ago

bebehei commented 7 years ago

Hi,

as the notification server should understand some a small subset of HTML tags, it also want's to parse the E-mail Address in the default title (%a => User Name <user@name.com>) as a tag. So, depending on the server, <user@name.com> either gets dropped or shown.

The <> should get properly escaped. The problem should be somewhere around here: https://github.com/mkiol/GNotifier/blob/master/source/lib/thunderbird.js#L430

I'm using the addon version from current master and Thunderbird 52.3.0.

For a possible workaround, you could use %n &lt;%m&gt; in the title (which should be the escaped equivalent).

But anyway: Thanks for that awesome plugin!

mkiol commented 7 years ago

I 100% agree.

According this spec only following tags are mandatory to support on server side: \, \, \, \, \ and nothing more. Gnotifier should discard every unsupported HTML symbols before sending data to notification server.

This issue is related to #185 and #24

bebehei commented 7 years ago

Gnotifier should discard every unsupported HTML symbols before sending data to notification server.

Someone from the dunst team mentioned this, too: You should check for the caps of the server. If the server doesn't support body-markup, you do not have to escape the content.

mkiol commented 7 years ago

I will fix this bug in the next release.

mkiol commented 6 years ago

The Issue has been fixed in the latest dev xpi.

bebehei commented 6 years ago

The Issue has been fixed in the latest dev xpi.

Yay, thanks for taking care of this. But I've built the plugin from current master (1b38ebff), and it seems, that %a still sends unescaped tags. From my point of view, this is not fixed.

mkiol commented 6 years ago

Are you referring to escaping of Summary/Title field? As per spec only Body supports some HTML tags, so it should be escaped. The Summary/Title field doesn't recognize HTML and therefore it should be pass to the notification server without modifications. Actually, GNOME and KDE display Title correctly without any escaping.

Body escaping is implemented here: https://github.com/mkiol/GNotifier/blob/master/source/lib/linux.js#L406. I have tested it several times and it works! I'm pretty sure.

bebehei commented 6 years ago

Ouch. I see, the fault is on my side.

You're reading the specs very carefully. Very good work!