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 755563 - Allow all URI schemes supported by the system
Allow all URI schemes supported by the system
Status: RESOLVED FIXED
Product: polari
Classification: Applications
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: Polari maintainers
Polari maintainers
: 755564 755566 755569 755593 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2015-09-24 19:14 UTC by Moo
Modified: 2015-11-06 00:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
utils: Linkify all supported URI schemes (2.09 KB, patch)
2015-11-04 15:58 UTC, Jonas Danielsson
none Details | Review
utils: Linkify all supported URI schemes (1.87 KB, patch)
2015-11-04 17:23 UTC, Jonas Danielsson
committed Details | Review

Description Moo 2015-09-24 19:14:07 UTC
When I come across a tel: URI such as "Hey, you should call me tel:123456 now".

Then it should hyperlink the tel: URI and let me open that with a VoIP client registered with that protocol handler.

Such as Empathy or Skype.
Comment 1 Florian Müllner 2015-09-24 21:13:46 UTC
It doesn't make any sense for us to bypass the desktop's URI handling by implementing our own system. However we could use g_vfs_get_supported_uri_schemes() to build a regex that covers all schemes supported by the system.
Rephrasing accordingly.
Comment 2 Florian Müllner 2015-09-24 21:13:53 UTC
*** Bug 755564 has been marked as a duplicate of this bug. ***
Comment 3 Florian Müllner 2015-09-24 21:13:57 UTC
*** Bug 755566 has been marked as a duplicate of this bug. ***
Comment 4 Florian Müllner 2015-09-24 21:14:00 UTC
*** Bug 755569 has been marked as a duplicate of this bug. ***
Comment 5 Florian Müllner 2015-09-24 21:14:12 UTC
*** Bug 755593 has been marked as a duplicate of this bug. ***
Comment 6 Moo 2015-09-24 21:44:01 UTC
Someone says "You should install VLC media player, apt:vlc"
Then it turns apt:vlc into a hyperlink that I can click on, and it opens with GNOME Software and lets me install it.

If someone mentions a a SSH address in the chat channel.
Example: "Yeah, you can connect to ssh://example.com as of yesterday!"
Then turn that into a hyperlink that let me run SSH in GNOME Terminal.
https://www.iana.org/assignments/uri-schemes/prov/ssh

If someone mentions a a SIP address in the chat channel.
Example: "Hey, call me SIP:someone@example.com now and we'll talk!"
Then turn that into a hyperlink and let me call that person with a SIP client such as Ekiga or Empathy.

When I stumble upon a email address in a chat conversation let me click on that and open up Evolution or Thunderbird mail client.
So I can write an email or add it to my contacts.
Comment 7 Jonas Danielsson 2015-11-04 14:53:40 UTC
(In reply to Florian Müllner from comment #1)
> It doesn't make any sense for us to bypass the desktop's URI handling by
> implementing our own system. However we could use
> g_vfs_get_supported_uri_schemes() to build a regex that covers all schemes
> supported by the system.
> Rephrasing accordingly.

So would an regexp of the style:

const _urlRegexp = new RegExp(
     '(^|' + _leadingJunk + ')' +
     '(' +
         '(?:' +
-            '(?:http|https|ftp)://' +             // scheme://
-            '|' +
-            'www\\d{0,3}[.]' +                    // www.
-            '|' +
-            '[a-z0-9.\\-]+[.][a-z]{2,4}/' +       // foo.xx/
-            '|' +
-            'geo:' + _float + ',' + _float +     // 'geo' URI
+            '(?:' + _uriSchemes.join('|') + ')' +

Be acceptable?

One thing I wonder, the code:
const _uriSchemes = Gio.Vfs.get_default().get_supported_uri_schemes();
log(_uriSchemes);

this gives the following output for me:
Gjs-Message: JS LOG: file,recent,afp,dns-sd,http,ftps,computer,sftp,ssh,davs,dav,ftp,google-drive,network,localtest,afc,mtp,cdda,smb,burn,archive,davs+sd,dav+sd,gphoto2,trash

No, 'geo:' for instance, even if I install the Maps handler. And I guess the function above gives me the supported URIs for GVfs, not the desktop in general.
How would one get that?

Thanks
Comment 8 Jonas Danielsson 2015-11-04 15:58:50 UTC
Created attachment 314835 [details] [review]
utils: Linkify all supported URI schemes

Instead of maintaining and adding individual URI schemes
to linkify we ask the system which are supported and match
against those.
Comment 9 Jonas Danielsson 2015-11-04 16:12:22 UTC
That code gives the following list of URI:
ghelp
help
info
man
http
https
mailto
vnc
pnm
mms
net
rtp
rtsp
mmsh
uvox
icy
icyx
geo
itms
itmss
telnet
rlogin
ssh
magnet
gitg
mms
rtmp
rtsp
mailto
pnm
mms
net
rtp
rtmp
rtsp
mmsh
uvox
icy
icyx
skype
mms
pnm
rtspt
rtspu
mailto
note
Comment 10 Bastien Nocera 2015-11-04 16:15:36 UTC
Review of attachment 314835 [details] [review]:

FWIW, this looks like the sort of changes that could do with a regression test.

::: src/utils.js
@@ -44,3 @@
-            '(?:http|https|ftp)://' +             // scheme://
-            '|' +
-            'www\\d{0,3}[.]' +                    // www.

You probably don't want to remove that sort of link.

@@ -46,3 @@
-            'www\\d{0,3}[.]' +                    // www.
-            '|' +
-            '[a-z0-9.\\-]+[.][a-z]{2,4}/' +       // foo.xx/

or that one.
Comment 11 Jonas Danielsson 2015-11-04 17:23:01 UTC
Created attachment 314845 [details] [review]
utils: Linkify all supported URI schemes

Instead of maintaining and adding individual URI schemes
to linkify we ask the system which are supported and match
against those.
Comment 12 Jonas Danielsson 2015-11-04 17:24:42 UTC
Review of attachment 314835 [details] [review]:

Yes, a regression test would be nice, At the moment there are no such tests. I think maybe we should add installed-tests to polari?
Or can the resource stuff be solved with local tests? importing the Utils module?

::: src/utils.js
@@ -46,3 @@
-            'www\\d{0,3}[.]' +                    // www.
-            '|' +
-            '[a-z0-9.\\-]+[.][a-z]{2,4}/' +       // foo.xx/

Ah, of course, thank you!
Comment 13 Florian Müllner 2015-11-05 18:54:06 UTC
Review of attachment 314845 [details] [review]:

If you agree with the approach of only linkifying known schemes, this looks good to me.

::: src/utils.js
@@ +81,3 @@
+    let apps = Gio.AppInfo.get_all();
+    let prefix = 'x-scheme-handler/';
+    let handlers = [];

Nit: 'handlers' doesn't sound like a list of scheme names to me, I'd prefer 'schemes'

@@ +84,3 @@
+
+    apps.forEach(function(app) {
+        let types = app.get_supported_types();

This takes care of schemes handled by installed applications, but not those handled by gvfs. Looking at that list, this is probably fine though. I don't think linkifying local resources (or ones on the local network) makes sense, which leaves ftp:// and ssh:// as far as I can see - and both throw an error if the resource isn't mounted already ...

@@ +89,3 @@
+
+        types.forEach(function(type) {
+            if (type.indexOf(prefix) >= 0)

Nitpick: the comparison is wrong, it matches 'fox-scheme-handler/hound', which then gets turned into 'fohound'
Just use
if (type.startsWith(prefix))
Comment 14 Jonas Danielsson 2015-11-05 23:54:58 UTC
Review of attachment 314845 [details] [review]:

I agree with that approach. I like the idea that adding a scheme handler to GNOME will automatically enable Polari to handle it. It makes this feel like the right thing to do.

Thanks for review!

::: src/utils.js
@@ +81,3 @@
+    let apps = Gio.AppInfo.get_all();
+    let prefix = 'x-scheme-handler/';
+    let handlers = [];

Agreed, especially since the function name is getURISchemes

@@ +89,3 @@
+
+        types.forEach(function(type) {
+            if (type.indexOf(prefix) >= 0)

Right.
Comment 15 Jonas Danielsson 2015-11-06 00:01:07 UTC
Attachment 314845 [details] pushed as 651c71f - utils: Linkify all supported URI schemes