Skip Menu |
 

This queue is for tickets about the Browser-Open CPAN distribution.

Report information
The Basics
Id: 58862
Status: open
Priority: 0/
Queue: Browser-Open

People
Owner: melo [...] cpan.org
Requestors: SLAFFAN [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 0.03
Fixed in: (no value)



Subject: open_browser does not find command on Windows
Download (untitled) / with headers
text/plain 970b
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.
Download (untitled) / with headers
text/plain 1.3k
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?
Download (untitled) / with headers
text/plain 1.8k
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,
Download (untitled) / with headers
text/plain 194b
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; }


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.