GNOME Bugzilla – Bug 653033
implement body-hyperlinks
Last modified: 2021-07-05 14:36:04 UTC
GNOME Shell's notification area does not support body hyperlinks as of gnome-shell-3.0.2. It correctly omits "body-hyperlinks" from its list of capabilities, but some applications may not check for this. If an application puts hyperlinks into a notification body, then the <a href="...">...</a> markup is visible in the displayed body. The desktop notification spec (http://developer.gnome.org/notification-spec/) states that "Notification servers that do not support these tags should filter them out." GNOME Shell should follow this recommendation. If body hyperlinks are not supported, then the markup should be filtered out.
except that we already have code to linkify urls, so it should be easy to implement <a></a> as well
Sure, that works too! :-D
Created attachment 192572 [details] [review] messageTray: support <a href=""></a> in notification body
Comment on attachment 192572 [details] [review] messageTray: support <a href=""></a> in notification body >+ let regExp = new RegExp('<a href=("[^"]+"|\'[^\']+\')>(([^<]|<(?!/a))+)</a>', 'i'); mmm... see also bug 650298. Maybe it would be better to use E4X to parse, validate, translate, and re-stringify the markup
Review of attachment 192572 [details] [review]: As discussed on IRC, I'd like to get this to being several separate Clutter actors that make up a message: a Link actor could be one, assuming that we can make word wrap work. The main motivation is to allow us to stick St.Icon actors in there so that I can implement emoticons. ::: js/ui/messageTray.js @@ +240,3 @@ let [x, y] = event.get_coords(); [success, x, y] = this.actor.transform_stage_point(x, y); + let findPos = -1; These style changes are unrelated and confusing.
(In reply to comment #4) > Maybe it would be better to use E4X to parse It work only with XML (not with HTML). Probably we can write simple "html" parser. Simelar to http://ejohn.org/blog/pure-javascript-html-parser/
Isn't pango/notification-daemon markup XML, not HTML?
(In reply to comment #7) > Isn't pango/notification-daemon markup XML, not HTML? From specification: "The markup consists of a small subset of HTML along with a few additional tags"
(In reply to comment #8) > From specification: > "The markup consists of a small subset of HTML along with a few additional > tags" Where are you quoting from? The current (http://developer.gnome.org/notification-spec/#markup) and previous versions both say "The markup is XML-based, and consists of a small subset of HTML...". ie, it is syntactically XML (note the trailing "/" on the <img> tag example), but the tag names are taken from HTML.
Created attachment 195132 [details] [review] messageTray: introduce parser for markup also support <a href=""></a> in notification body
Created attachment 195133 [details] [review] messageTray: support img in notification body
Comment on attachment 195132 [details] [review] messageTray: introduce parser for markup so... not really happy about this patch. It adds an HTML parser, to parse *XML*, when we already have a built-in XML parser (and a second via glib if we added a few new methods/annotations). Is there a particular reason you didn't use E4X? notes on this patch (which should not be construed as support for it): >+var MarkupParser = { move the comment with the source URL from the next commit here >+ "a": TagType.LINK, single quotes (throughout) >+ _dumpTextForTagType: { >+ 4: 'b', The fact that you can't use the enum in that context does not make it legitimate to put magic numbers there instead. >+ this._tree = { childs: [], type: TagType.BODY, attrs: {}, text: '' }; "childs" should be "children" everywhere >+ dump: function(ir, linkColor, pos) { i can't figure out what "ir" stands for
E4X does not store formatting of original text. <g></g> become <g/>. It put newline after each tag (in toString). > and a second via glib if we added a few new methods/annotations it fail to parse xml's with bracket (e.g. '<body> < </body>') Probably, It will be hard to escape unsupported tags during parse phase with this parser. xml dumper is not part of glib > i can't figure out what "ir" stands for intermediate representation (IR)
(In reply to comment #13) > it fail to parse xml's with bracket (e.g. '<body> < </body>') > Probably, It will be hard to escape unsupported tags during parse phase with > this parser. mmm... ok. makes sense. a comment explaining this would be good. and the other comments about the code still hold.
Created attachment 195653 [details] [review] messageTray: introduce parser for markup
No.
If handling hyperlink markup is unacceptable for some reason, can we go back to filtering markup out? As I wrote in my original bug report, the desktop notification spec (http://developer.gnome.org/notification-spec/) states that "Notification servers that do not support these tags should filter them out." GNOME Shell should follow this recommendation. If body hyperlinks are not supported, then the markup should be filtered out. Of course, filtering out markup still requires parsing in order to find the markup at all. Jasper St. Pierre, is the parsing itself what you find unacceptable? I realize it adds complexity. But isn't it a bit silly for GNOME's Desktop Notifications Specification to recommend behavior ("should") that GNOME's own Shell refuses to implement ("WONTFIX")?
Given the weird issues with markup parsing above, I'm hesitant to believe that you can come up with a sane strategy for stripping out markup.
OK, then this is really a bug in the GNOME Desktop Notifications Specification. After all, a sane specification shouldn't call for behavior that is simply not sanely implementable. I can open a new bug report for that, or we could change this report over to ... umm, what? What's the appropriate GNOME Bugzilla "Product" value for a specification defect? Unclear.
I dont understand the WONTFIX here - even if the patch is rejected, this seems like a legitimate RFE.
The issue that I'm concerned about is sane markup parsing. If we want to be stricter on clients to make sure that entitysize every < > & character, then I'm fine with it.
*** Bug 745984 has been marked as a duplicate of this bug. ***
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new ticket at https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/ Thank you for your understanding and your help.