Bug #58862 for Browser-Open: open_browser does not find command on Windows

Sun Jun 27 01:53:48 2010 SLAFFAN [...] cpan.org - Ticket created

Subject:

open_browser does not find command on Windows

Hello Pedro, This is a really useful module. However, there are a few issues when searching for the default command under Windows. The end result is that it does not find any command. 1. Neither "start" nor "open" are in the default path (that I could find on Vista), so _search_in_path() will not find them and returns undef. Maybe special handling for windows is warranted. 2. Also in _search_in_path(), splitting on /:/ does not work with windows path separators. It needs to be a semi-colon /;/. This can be corrected with check of the OS before this search, something like the code below (starting from line 89). I can provide this as a diff if you'd prefer. ---- my $sep = q{:}; # path list separator for *nix systems if ( $^O eq 'MSWin32' ) { $sep = q{;}; } for my $path (split($sep, $ENV{PATH})) { ---- The work-around to the above issues is to add an OS check in the calling code, but it does clutter things a bit. Regards, Shawn Laffan.

Sun Jun 27 02:46:41 2010 melo [...] cpan.org - Correspondence added

On Sun Jun 27 01:53:48 2010, SLAFFAN wrote: Show quoted text

> This is a really useful module.

Thank you :) Show quoted text

> However, there are a few issues when > searching for the default command under Windows.

I would guess so, I don't have any Windows boxes, and I don't own any Windows licenses. I need to search ADAMK post about some WIndows boxes where you can ask an account for testing... Show quoted text

> The end result is that > it does not find any command. > > 1. Neither "start" nor "open" are in the default path (that I could > find on Vista), so _search_in_path() will not find them and returns > undef. Maybe special handling for windows is warranted.

Can you suggest a windows-specifig logic to test? Should I just assume that "start URL" will work? Show quoted text

> 2. Also in _search_in_path(), splitting on /:/ does not work with > windows path separators. It needs to be a semi-colon /;/. This can be > corrected with check of the OS before this search, something like the > code below (starting from line 89). I can provide this as a diff if > you'd prefer. > > ---- > > my $sep = q{:}; # path list separator for *nix systems > if ( $^O eq 'MSWin32' ) { > $sep = q{;}; > } > for my $path (split($sep, $ENV{PATH})) { > > ----

This is fine. I'll apply this fix. I'll release a new version next week, probably late next week, a bit busy at work. Best regards, -- Hi, how are you?

Sun Jun 27 02:46:41 2010 The RT System itself - Status changed from 'new' to 'open'

Sun Jun 27 02:46:54 2010 melo [...] cpan.org - Taken

Sun Jun 27 18:51:08 2010 SLAFFAN [...] cpan.org - Correspondence added

Hi Pedro, I guess the logic for the windows boxes would just involve a check for windows and then calling "start URL". That's all I'm doing at the moment when I get undef back from open_browser(). It will also pass the check to windows to resolve, which will call the user's default. I look forward to the new release when you get a chance. Regards, Shawn. On Sun Jun 27 02:46:41 2010, MELO wrote: Show quoted text

> On Sun Jun 27 01:53:48 2010, SLAFFAN wrote:

> > This is a really useful module.

> > Thank you :) >

> > However, there are a few issues when > > searching for the default command under Windows.

> > I would guess so, I don't have any Windows boxes, and I don't own any > Windows licenses. I > need to search ADAMK post about some WIndows boxes where you can ask > an account for > testing... >

> > The end result is that > > it does not find any command. > > > > 1. Neither "start" nor "open" are in the default path (that I could > > find on Vista), so _search_in_path() will not find them and returns > > undef. Maybe special handling for windows is warranted.

> > Can you suggest a windows-specifig logic to test? Should I just assume > that "start URL" will > work? > >

> > 2. Also in _search_in_path(), splitting on /:/ does not work with > > windows path separators. It needs to be a semi-colon /;/. This can

> be

> > corrected with check of the OS before this search, something like

> the

> > code below (starting from line 89). I can provide this as a diff if > > you'd prefer. > > > > ---- > > > > my $sep = q{:}; # path list separator for *nix systems > > if ( $^O eq 'MSWin32' ) { > > $sep = q{;}; > > } > > for my $path (split($sep, $ENV{PATH})) { > > > > ----

> > This is fine. I'll apply this fix. > > I'll release a new version next week, probably late next week, a bit > busy at work. > > Best regards,

Thu Mar 17 08:57:02 2011 cpan [...] fireartist.com - Correspondence added

I can confirm this problem on windows 7 32bit. The attached patch provides a generic solution that can be used by other OSes if ever necessary. If this could be pushed to cpan, that'd be great!

Subject:

browser_open_pm.diff

Index: Open.pm =================================================================== --- Open.pm (revision 1) +++ Open.pm (working copy) @@ -20,7 +20,7 @@ ['', $ENV{BROWSER}], ['darwin', '/usr/bin/open', 1], ['cygwin', 'start'], - ['MSWin32', 'start'], + ['MSWin32', 'start', undef, 1], ['solaris', 'xdg-open'], ['solaris', 'firefox'], ['linux', 'xdg-open'], @@ -52,6 +52,7 @@ croak('Missing required parameter $url, ') unless $url; my $cmd = $all ? open_browser_cmd_all() : open_browser_cmd(); + return unless $cmd; return system($cmd, $url); @@ -72,11 +73,12 @@ my ($filter) = @_; foreach my $spec (@known_commands) { - my ($osname, $cmd, $exact) = @$spec; + my ($osname, $cmd, $exact, $no_search) = @$spec; next unless $cmd; next if $osname && $filter && $osname ne $filter; return $cmd if $exact && -x $cmd; + return $cmd if $no_search; $cmd = _search_in_path($cmd); return $cmd if $cmd; }