awesome

Welcome to awesome bug tracking system.
Tasklist

FS#876 - Libnotify spec requires that we handle some markup

Attached to Project: awesome
Opened by Andrei Thorp (garoth) - Monday, 14 March 2011, 19:25 GMT
Last edited by Uli Schlachter (psychon) - Thursday, 24 March 2011, 18:12 GMT
Task Type Bug Report
Category Lua libraries → naughty
Status Closed
Assigned To No-one
Operating System All
Severity Medium
Priority Normal
Reported Version 3.4.8
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

In an uncommon corner of the libnotify spec lurks this page:

http://www.galago-project.org/specs/notification/0.9/x161.html

The basic thing thing to take away is that the libnotify spec *optionally* requires that notify clients (such as awesome/naughty) interpret a limited HTML-style markup. Specifically, the <a href=, <b>, <u>, <i>, and <img src= tags. If we don't support them, they should be instead filtered out. It seems that Awesome supports some of these tags, but not <a>. Probably, what we do is just run the text through pango's markup system which doesn't entirely match the libnotify spec.

At the moment, not very many programs seem to use this markup, but you can test that we don't support it by using notify-send. Ex: notify-send "<a href=\"blah blah\"> test </a>"

Sorry, don't want to test against next/master while at work, though I think this bug probably still exists there.
This task depends upon

Closed by  Uli Schlachter (psychon)
Thursday, 24 March 2011, 18:12 GMT
Reason for closing:  Won't fix
Additional comments about closing:  Sorry, but I guess that all we can agree on is that the notification spec is ambigious.
We do work around "broken" markup by escaping "<" and ">" already for those apps that sent stuff like "<a>" in their notification - before, we just displayed an empty box.
Also, the next release will have naughty.config.notify_callback so that everyone can have his own interpretation.
Comment by Andrei Thorp (garoth) - Monday, 14 March 2011, 19:40 GMT
Hmm, actually, maybe this was fixed. I guess I should update and see -_-
Comment by Uli Schlachter (psychon) - Monday, 14 March 2011, 20:23 GMT
No changes in git/master in this regard.

Also, could someone please fix the spec to be unambigious?

The docs for GetCapabilities describe the "body-hyperlinks" cap like this:
The server supports hyperlinks in the notifications.
http://www.galago-project.org/specs/notification/0.9/x408.html#command-get-capabilities

We return "body", "body-markup" and "icon-static" from GetCapabilities.

So to my understanding, awesome implements the spec properly and actually announces that it can't handle hyperlinks.
(The same applies to "body-images" and "<img src=foo>")
Comment by Andrei Thorp (garoth) - Monday, 14 March 2011, 23:11 GMT
This is all kind of guesswork, but I guess the question is "who is it that choses whether or not to send hyperlinks" -- I bet it's the application. And further, I bet applications frequently can't be bothered to check what the server's settings are. As such, I would think that "not supporting something" would entail both announcing that you don't and enforcing it by filtering out whatever it is.

But yeah, the spec sucks.
Comment by Uli Schlachter (psychon) - Tuesday, 15 March 2011, 15:59 GMT
Filtering out <a> and <img> tags would require some ugly regex. (Well, doable, but all regex is ugly!)
Also, so far I didn't see any reports of apps actually sending any of this tags. And our backup plan of just displaying the "raw" markup works good enough IMHO.

I'm tempted to say that the current state is good enough (for me).
Comment by Andrei Thorp (garoth) - Tuesday, 15 March 2011, 19:06 GMT
Kind of not cool for me. mumble, which I use daily, does nothing but spam me with <a href tags that are an entire screen in width. I then must scramble to search through the massive ass string for who actually joined my channel or what they said.

I guess I can fix it myself if necessary, though I'm afraid of the master branch atm as it seems to buttrape obvious with some widget changes
Comment by Uli Schlachter (psychon) - Tuesday, 15 March 2011, 20:21 GMT
No one mentioned mumble so far. ;-)

What's the text for that <a> tags? I took a look at mumble's source and it's Log::log() which does notifications. However, I couldn't find anything call that with such a markup (but I didn't look too closely).
Comment by Andrei Thorp (garoth) - Wednesday, 16 March 2011, 15:21 GMT
To be honest, I haven't looked at it too closely. It's a pretty long jumbled hash or something. I'll see if I can grab a screenshot of it later when the opportunity comes up
Comment by Andrei Thorp (garoth) - Wednesday, 16 March 2011, 20:58 GMT
HA. CAUGHT ONE. HAH. Ignore my work/emails/whatever

Comment by Uli Schlachter (psychon) - Wednesday, 16 March 2011, 21:10 GMT
Uhm, <span> isn't mentioned anywhere in the spec (AFAIK) and I don't know what could handle the clientid URI-scheme. WTF?
Comment by Andrei Thorp (garoth) - Wednesday, 16 March 2011, 21:46 GMT
I know, right? Probably some bullshit like Gnome's notification handler handles some extension bullshit that they didn't bother documenting because they needed it that one time for some stupid hack and now other idiots caught on.

I'd be content if the span tags stay visible but the a ones get filtered out. Or just filter out all tags except b/u/i.
Comment by James (jpike) - Thursday, 17 March 2011, 09:17 GMT
Andrei: you could use the naughty.config.notity_callback together with a regular expression to strip out the <a> tags.

lua has a pitiful regular expression system but hopefully some bindings are provided to a decent regexp library.
Comment by Uli Schlachter (psychon) - Thursday, 17 March 2011, 15:47 GMT
I like the callback idea, but only awesome 3.4.10 will actually have the callback. Anyway, something like this (untested) might work:

naughty.config.notify_callback = function(args) args.text = string.gsub(args.text, '<[^>]+>', '') return args end -- Regex always look like a cat slept on the keyboard

For what the clientid:// URLs mean, I asked #mumble: <pcgod> hmm actually you can't handle them... they're not useful for anything outside the client...
Comment by Andrei Thorp (garoth) - Friday, 18 March 2011, 20:11 GMT
James: yeah, I'm pretty much and old awesome developer come back from the dead. I'm sure I can hack around this if I wanted, but that's not why I'm raising the bug.

The point is that it's my interpretation of the spec that we should be stripping out these tags in our core libnotify handler

Though clearly, mumble is doing some sketchy shit on their end also
Comment by Andrei Thorp (garoth) - Thursday, 24 March 2011, 19:51 GMT
So what's the decision here? Wontfix?
Comment by Andrei Thorp (garoth) - Thursday, 24 March 2011, 19:51 GMT
Oh, yes it is

Loading...