! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
exo-open fails on local URLs with anchors
Status:
RESOLVED: FIXED

Comments

Description Colin Leroy 2008-11-17 08:50:50 CET
Created attachment 1987 
Patch to handle file:///path/to/file.html#anchor URLs

Hello,

On Claws Mail we use help buttons that redirect to local html files with anchors, like

file:///usr/local/share/doc/claws-mail/manual/en/claws-mail-manual.html#adv_plugins

exo-open fails on this, displaying:

Failed to open URL "file:///usr/local/share/doc/claws-mail/manual/en/claws-mail-manual.html#adv_plugins".
The URL "file:///usr/local/share/doc/claws-mail/manual/en/claws-mail-manual.html#adv_plugins" is not supported.

Running exo-open "file:///usr/local/share/doc/claws-mail/manual/en/claws-mail-manual.html" works.

Attached is a patch that strips out anchors from local URIs if it exists, so that this class of URIs are handled.
Comment 1 Yves-Alexis Perez editbugs 2008-11-17 09:50:32 CET
On beta2:

corsac@hidalgo: exo-open "/usr/share/doc/asciidoc/asciidoc.html#X6"
Error showing url: The location or file could not be found.
(a popup is displayed saying the url is not supported)

corsac@hidalgo: exo-open /usr/share/doc/asciidoc/asciidoc.html

works and opens asciidoc.html in my default browser

corsac@hidalgo: exo-open file:///usr/share/doc/asciidoc/asciidoc.html

works and opens asciidoc.html in my default browser


corsac@hidalgo: exo-open "file:///usr/share/doc/asciidoc/asciidoc.html#X6"

works but opens the URL in a new epiphany window (while my default browser is epiphany --new-tab).
Comment 2 Brian J. Tarricone (not reading bugmail) 2008-11-17 11:57:14 CET
Jannis, please revert or otherwise fix svn rev 28598.  You appear to have broken URL parsing by trying to be too smart.  A comma at the end of a URL, while unlikely, is a perfectly valid URL.  For example, exo-open claims that "http://example.com/foo.html," is an unsupported URL.
Comment 3 Brian J. Tarricone (not reading bugmail) 2008-11-17 11:58:24 CET
(On a side note, even if that trailing-comma URL is indeed invalid, I'd rather exo-open pass that through to my web browser so it can error out and I can easily edit the URL into a correct one.  With the new behavior I have to select and copy the text from the originating application manually and paste it over to my web browser.  Certainly not an improvement.)
Comment 4 Jannis Pohlmann editbugs 2008-11-17 12:58:14 CET
(In reply to comment #2)
> Jannis, please revert or otherwise fix svn rev 28598.  You appear to have
> broken URL parsing by trying to be too smart.  A comma at the end of a URL,
> while unlikely, is a perfectly valid URL.  For example, exo-open claims that
> "http://example.com/foo.html," is an unsupported URL.

Yeah, that's right. I have the feeling that the first regular expression for browser URLs is wrong:

#define MATCH_BROWSER1  
  "^((file|https?|ftps?)://(" USER "@)?)[" HOSTCHARS ".]+(:[0-9]+)?" \
  "(/[-A-Za-z0-9_$.+!*(),;:@&=?/~#%]*[^]'.}>) \t\r\n,\\\"])?$"

A possible fix would be to just remove the trailing $ from the browser URL expressions, but for some reason this doesn't seem to work for # in file:-URIs. With this "fix"

  exo-open file://usr/local/share/xfce4/doc/C/xfce4-use.html#foo

successfully opens the browser (but due to only two slashes after file: the browser raises an error) but 

  exo-open file:///usr/local/share/xfce4/doc/C/xfce4-use.html#foo

does not work (it doesn't even show the exo-open error dialog). So to me it seems that my commit only made the *real* issue visible.
Comment 5 Jannis Pohlmann editbugs 2008-11-17 17:54:17 CET
I wonder if we shouldn't just use the regular expression listed on this site:

  http://labs.apache.org/webarch/uri/rev-2002/rfc2396bis.html#regexp

It's less strict in that it allows any protocol (like http://, ftp://, irc:// etc.) to be part of the URI and also matches email addresses, so the general idea would have to be:

1. Check if the URI refers to a local file => let Thunar handle it
2. Check if the URI refers to a mail address => open it in the mail client
3. Check if the URI matches the URI pattern => open it in the browser

Thoughts?
Comment 6 Colin Leroy 2008-11-17 18:11:28 CET
(In reply to comment #5)
> I wonder if we shouldn't just use the regular expression listed on this site:
> 
>   http://labs.apache.org/webarch/uri/rev-2002/rfc2396bis.html#regexp
> 
> It's less strict in that it allows any protocol (like http://, ftp://, irc://
> etc.) to be part of the URI and also matches email addresses, so the general
> idea would have to be:
> 
> 1. Check if the URI refers to a local file => let Thunar handle it
> 2. Check if the URI refers to a mail address => open it in the mail client
> 3. Check if the URI matches the URI pattern => open it in the browser
> 
> Thoughts?

Sounds simple and hence, good :)
Comment 7 Jannis Pohlmann editbugs 2008-11-19 17:28:18 CET
Created attachment 1990 
New patch to make URL opening work properly

This is a new patch which incorporates the new (and slightly modified) regular expression for URIs and several other changes to make everything work properly:

  1. Modify _exo_url_to_local_path() so that it works like this: file://-URIs 
     are parsed, absolute paths remain untouched and relative "paths" are set to 
     NULL if they don't exist.
  2. Open absolute paths, existing relative paths and file://-URIs using the 
     file manager. Spawn Thunar synchroneously so that we can check its status 
     (and thereby verify whether these paths exist or not).

Please give this one some testing if you can.
Comment 8 Brian J. Tarricone (not reading bugmail) 2008-11-19 18:55:26 CET
(In reply to comment #7)

>      file manager. Spawn Thunar synchroneously so that we can check its status 
>      (and thereby verify whether these paths exist or not).

Nuh-uh.  If the thunar daemon isn't running, then this will block forever.
Comment 9 Jannis Pohlmann editbugs 2008-11-19 19:42:39 CET
(In reply to comment #8)
> (In reply to comment #7)
> 
> >      file manager. Spawn Thunar synchroneously so that we can check its status 
> >      (and thereby verify whether these paths exist or not).
> 
> Nuh-uh.  If the thunar daemon isn't running, then this will block forever.

I should have said this along with the patch: this doesn't happen on my machine.
Comment 10 Jannis Pohlmann editbugs 2008-11-20 00:21:04 CET
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > 
> > >      file manager. Spawn Thunar synchroneously so that we can check its status 
> > >      (and thereby verify whether these paths exist or not).
> > 
> > Nuh-uh.  If the thunar daemon isn't running, then this will block forever.
> 
> I should have said this along with the patch: this doesn't happen on my
> machine.

Okay, of course you we're right. I've committed the patch (without spawning Thunar synchronously) in revision 28866:

        * exo/exo-url.c (_exo_url_to_local_path, exo_url_show_on_screen):
          Modify _exo_url_to_local_path() so that it converts file:// URIs to
          paths, does not touch absolute paths and converts relative paths to
          absolute paths only if they exist in the local filesystem. Set
          relative paths to NULL otherwise.
          Replace the two regular expression for browser URIs by a more
          generic one and try to open URIs in the following order: file
          manager, email client and web browser as a last option.
          Alltogether this should fix bug #4627.

Along with this fix goes a change in Thunar:

	* thunar/thunar-application.c (thunar_application_process_filenames):
	  Always show an error dialog if one of the files passed via the
	  command line cannot be opened. Required for exo-open to give proper
	  visual feedback because spawning Thunar asynchronously will always
	  make exo-open think that the file(s) were opened successfully.
	  Ideally Thunar would display an error dialog with a tree view
	  listing all the files that could not be opened (I'll file a bug for
	  this).

(I know, these commit messages are very verbose. I just want to make sure everyone (especially Benny) will be able to understand what I did and why.)
Comment 11 Colin Leroy 2008-11-25 07:51:12 CET
I can confirm this bug is fixed on latest SVN, feel free to close :-)

Thanks!
Comment 12 Colin Leroy 2008-12-16 08:12:51 CET
Marking FIXED :)

Bug #4627

Reported by:
Colin Leroy
Reported on: 2008-11-17
Last modified on: 2009-10-09

People

Assignee:
Nick Schermer
CC List:
2 users

Version

Version:
unspecified

Attachments

Additional information