I should note that the first example the link wasn't a mailto:mccann@redhat.com but a url link to redhat.com.  And the second example the link was "of:this".

I've noticed that long chat notifications that have a link in them have the ellipses underlined even if the link itself is not a part of the text shown in the banner. I've also seen Gwibber notifications that don't have anything in their banner other then ellipses in the end.

I just have to say though that having the links in the notifications is awesome :)!

(In reply to comment #0)
> Evolution has notifications that include "Mail from mccann@redhat.com"

and you're seeing "mccann@redhat.com" get URLified? it doesn't happen if I send myself that with notify-send...

Created attachment 177691 [details]
screenshot

Sorry, I was mistaken about the part that was turned into a link.

Regardless of this bug the message from Evo should be improved...

Created attachment 185867 [details] [review]
findUrl: document the URL-matching regex

Explode the regex onto multiple lines, and add comments explaining the
pieces. Also, change ()s to (?:)s (non-capturing groups) where
appropriate, and replace the UTF-8 characters with \u escapes so that
they actually work.

Created attachment 185868 [details] [review]
findUrl: don't match non-:// URLs

Although "x:5" could theoretically be a URL with scheme "x" and path
"5", it probably isn't. Only URLify strings that use the "authority"
syntax ("foo://").

Created attachment 185869 [details] [review]
findUrl: be pickier about what can precede a URL

findUrl() was seeing strings like "You have 1 new message in
foo@example.com/Inbox" and finding the URL
"[http://]example.com/Inbox". Require that URLs either start at the
start of the string, or are preceded by whitespace or an open
paren/quote/etc.

(Since JS doesn't have look-behind assertions like perl does, we have
to actually match the URL-preceding character in the regex, and then
adjust the result findUrl returns accordingly.)

poke

Created attachment 191347 [details] [review]
tests: add a unit test for Misc.findUrls()

depends on the unit test fixage from bug 650298

Review of attachment 185867 [details] [review]:

This is increasingly reminding me of http://www.ex-parrot.com/pdw/Mail-RFC822-Address.html, but sure.

Review of attachment 185868 [details] [review]:

The only scheme that I can think of in major use without an "://" is mailto, but I doubt that's going to be in plaintext in a notification.

Review of attachment 185869 [details] [review]:

::: js/misc/util.js
@@ +13,3 @@
 // http://daringfireball.net/2010/07/improved_regex_for_matching_urls
 const _balancedParens = '\\((?:[^\\s()<>]+|(?:\\(?:[^\\s()<>]+\\)))*\\)';
+const _leadingJunk = '[\\s`(\\[{\'\\"<\u00AB\u201C\u2018]';

IMHO [;:.,] should be allowed too.

Review of attachment 191347 [details] [review]:

"No newline at end of file"

::: tests/unit/url.js
@@ +36,3 @@
+      output: [ { url: 'http://www.gnome.org:99/port', pos: 10 } ] },
+    { input: 'This is an ftp://www.gnome.org/ test.',
+      output: [ { url: 'ftp://www.gnome.org/', pos: 11 } ] },

Another mistake thrown on IRC a lot would help:

  'This is http://www.gnome.org.'
  'This is http://www.gnome.org/.'

@@ +50,3 @@
+    { input: 'This is not:/a.url/ test.',
+      output: [ ] },
+    { input: 'This is not@a.url/ test.',

Including Jon's origin test would be useful:

  'This is surely@not.a/url  test.'

> The only scheme that I can think of in major use without an "://" is mailto,
> but I doubt that's going to be in plaintext in a notification.

Yeah, added a comment about that to the commit message

> +const _leadingJunk = '[\\s`(\\[{\'\\"<\u00AB\u201C\u2018]';
> 
> IMHO [;:.,] should be allowed too.

given that this bug is about URLifying things that shouldn't be
urlified, I'm going to stick with the original regexp there. If this
turns out to miss important cases, then we can revisit it later.

> Another mistake thrown on IRC a lot would help:

(I think you're referring to the fact that xchat mistakenly includes a
trailing "," in a URL, which we don't do.)

> Including Jon's origin test would be useful:

I added both of the test cases you suggested.

Attachment 185867 [details] pushed as 92a8507 - findUrl: document the URL-matching regex
Attachment 185868 [details] pushed as 5632216 - findUrl: don't match non-:// URLs
Attachment 185869 [details] pushed as e2898be - findUrl: be pickier about what can precede a URL
Attachment 191347 [details] pushed as 6496205 - tests: add a unit test for Misc.findUrls()