After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 653033 - implement body-hyperlinks
implement body-hyperlinks
Status: RESOLVED OBSOLETE
Product: gnome-shell
Classification: Core
Component: message-tray
3.0.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 745984 (view as bug list)
Depends on:
Blocks: 651875
 
 
Reported: 2011-06-20 17:55 UTC by Ben Liblit
Modified: 2021-07-05 14:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
messageTray: support <a href=""></a> in notification body (5.86 KB, patch)
2011-07-24 19:28 UTC, Maxim Ermilov
needs-work Details | Review
messageTray: introduce parser for markup (10.41 KB, patch)
2011-08-29 22:48 UTC, Maxim Ermilov
needs-work Details | Review
messageTray: support img in notification body (6.42 KB, patch)
2011-08-29 22:49 UTC, Maxim Ermilov
none Details | Review
messageTray: introduce parser for markup (10.94 KB, patch)
2011-09-05 03:26 UTC, Maxim Ermilov
none Details | Review

Description Ben Liblit 2011-06-20 17:55:39 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.
Comment 1 Dan Winship 2011-06-20 17:58:17 UTC
except that we already have code to linkify urls, so it should be easy to implement <a></a> as well
Comment 2 Ben Liblit 2011-06-20 18:30:40 UTC
Sure, that works too!  :-D
Comment 3 Maxim Ermilov 2011-07-24 19:28:49 UTC
Created attachment 192572 [details] [review]
messageTray: support <a href=""></a> in notification body
Comment 4 Dan Winship 2011-07-24 21:21:21 UTC
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
Comment 5 Jasper St. Pierre (not reading bugmail) 2011-07-24 21:44:29 UTC
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.
Comment 6 Maxim Ermilov 2011-07-24 21:53:42 UTC
(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/
Comment 7 Jasper St. Pierre (not reading bugmail) 2011-07-24 22:06:07 UTC
Isn't pango/notification-daemon markup XML, not HTML?
Comment 8 Maxim Ermilov 2011-07-24 22:17:22 UTC
(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"
Comment 9 Dan Winship 2011-07-25 14:40:14 UTC
(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.
Comment 10 Maxim Ermilov 2011-08-29 22:48:29 UTC
Created attachment 195132 [details] [review]
messageTray: introduce parser for markup

also support <a href=""></a> in notification body
Comment 11 Maxim Ermilov 2011-08-29 22:49:07 UTC
Created attachment 195133 [details] [review]
messageTray: support img in notification body
Comment 12 Dan Winship 2011-08-30 17:06:06 UTC
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
Comment 13 Maxim Ermilov 2011-08-31 23:43:11 UTC
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)
Comment 14 Dan Winship 2011-09-01 21:24:00 UTC
(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.
Comment 15 Maxim Ermilov 2011-09-05 03:26:48 UTC
Created attachment 195653 [details] [review]
messageTray: introduce parser for markup
Comment 16 Jasper St. Pierre (not reading bugmail) 2012-06-01 11:26:24 UTC
No.
Comment 17 Ben Liblit 2012-06-01 15:13:53 UTC
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")?
Comment 18 Jasper St. Pierre (not reading bugmail) 2012-06-01 15:16:55 UTC
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.
Comment 19 Ben Liblit 2012-06-01 15:58:49 UTC
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.
Comment 20 Owen Taylor 2012-06-04 16:18:29 UTC
I dont understand the WONTFIX here - even if the patch is rejected, this seems like a legitimate RFE.
Comment 21 Jasper St. Pierre (not reading bugmail) 2012-06-05 04:19:20 UTC
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.
Comment 22 Florian Müllner 2015-03-10 19:32:07 UTC
*** Bug 745984 has been marked as a duplicate of this bug. ***
Comment 23 GNOME Infrastructure Team 2021-07-05 14:36:04 UTC
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.