GNOME Bugzilla – Bug 765138
Website links fails to show up when containing & signs
Last modified: 2016-09-06 19:42:45 UTC
If a website tag on an OSM object contains an & (using a URL format with request parameters) we get an error message and no link is rendered in the place bubble.
Created attachment 326153 [details] [review] placeBubble: Escape markup for website URIs
The attached patch kinda works (you can now see the Website link label), but the tooltip is broken. Something like: (gnome-maps:31330): Gtk-WARNING **: Failed to set text 'http://www.mersmak.org/index.php?option=com_wrapper&view=wrapper&Itemid=204' 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 & is printed by GTK+ To get an example, search for "bryggerimuséet arboga" (the website URL seems to have broken since I added it a couple of years ago, and now points to some Japanese page, possible some domain for sale thing).
Created attachment 326352 [details] [review] placeBubble: Escape markup for website URIs
With latest patch the tooltip displays properly. Thanks to Timm Bädert for the hint!
Review of attachment 326352 [details] [review]: Looks good! Fine to commit, if you agree with my nits, then please fix them up before committing! Otherwise just commit! ::: src/placeBubble.js @@ +201,3 @@ let text = expandedContent[row].linkText; + let uri = GLib.markup_escape_text(expandedContent[row].linkUrl, + -1); If we do not break this line, it will still not be as long as the "let a = " line? Maybe just leave this un-broken? @@ +203,3 @@ + -1); + /* we need to double-escape the tooltip text, as GTK+ treats it + as markup */ Could we get this to one line by re-formulate? Otherwise have this style of multi-line: /* [empty] * text line 1 * text line 2 */
(In reply to Jonas Danielsson from comment #5) > Review of attachment 326352 [details] [review] [review]: > > Looks good! > > Fine to commit, if you agree with my nits, then please fix them up before > committing! > Otherwise just commit! > > ::: src/placeBubble.js > @@ +201,3 @@ > let text = expandedContent[row].linkText; > + let uri = > GLib.markup_escape_text(expandedContent[row].linkUrl, > + -1); > > If we do not break this line, it will still not be as long as the "let a = " > line? > Maybe just leave this un-broken? > Sure, would probably be a good un-broken :) > @@ +203,3 @@ > + -1); > + /* we need to double-escape the tooltip text, as GTK+ > treats it > + as markup */ > > Could we get this to one line by re-formulate? Otherwise have this style of > multi-line: > > /* [empty] > * text line 1 > * text line 2 > */ Sure, can probably trim down that comment anyway!
Created attachment 334929 [details] [review] placeBubble: Escape markup for website URIs
Attachment 334929 [details] pushed as 706ea4d - placeBubble: Escape markup for website URIs
Thanks for the review!