skandasoft / open-in-browsers

Open in Different Browsers - IE/Chrome/Firefox/Opera/BrowserPlus
MIT License
21 stars 7 forks source link

Opened url in firefox when using "Switch LocalHost" setting is malformed (has backward slashes). #12

Closed JohnLukeBentley closed 8 years ago

JohnLukeBentley commented 8 years ago

Steps to reproduce.

Resulting url but in browser "http://localhost:8080/web/Examples\Html5Examples\html5-play.xhtml" (Observe the backward slashes)

This bug was recently introduced (I'm on 0.0.13).

URL is returned OK in IE and Chrome. "http://localhost:8080/web/Examples/Html5Examples/html5-play.xhtml"

TooSharpForYou commented 8 years ago

Hi,

add this to your 'open-in-browsers-view.coffee' located at '%USERPROFILE%.atom\packages\open-in-browsers\lib':

this line already exists (line 71): fpath = fpath.replace foldr,url

add this to the next line: fpath = fpath.replace /\/g,'/'

Restart Atom and it should work now.

greetz

JohnLukeBentley commented 8 years ago

Thanks @TooSharpForYou. That's the trick: effect a global regex replacement.

In the light of testing I took your idea and made three changes.

For reference https://github.com/skandasoft/open-in-browsers/blob/master/lib/open-in-browsers-view.coffee

  1. There's an existing attempt to replace backslashes at line 87 (further down from 71), in the open method (rather than the getfilePath method) . So I targeted changing that command. (This also ensures blackslash replacments are done regardless of whether serving from a web server or local file system).
  2. The backslash needs to be escaped in a regex command. E.g. '/\\/g' rather than '/\/g'
  3. For some strange reason the string specification of regex switches didn't work.

So

86: cmd = "#{cmd} \"#{fpath}\""
87: cmd = cmd.replace '/\\/g', '/'

... didn't work, so I declared a separate regex variable and feed it to the replace method

86: cmd = "#{cmd} \"#{fpath}\""
87: re = /\\/g;
88: cmd = cmd.replace re, '/'

[edit: I haven't taken the time to review the whole code and think about the best place to effect this replacement (perhaps better to clean up the getFilePath first; delete the code in the open method that attempts to change the backslashes; and put an assertion in the open method that the received path contains no backslashes).]

I haven't made any pulls and commits that effects this change in the public source as I'm not yet set up for this. So if someone want to do the honours, they can be my guest (and would be doing me a favour).

TooSharpForYou commented 8 years ago

Hi john,

just write the params in brackets and the regex without quotes and it should work without a separate variable

i.e. 87: cmd = cmd.replace /\/g, '/' or 87: cmd = cmd.replace(/\/g, '/')

Am 20.04.2016 um 02:01 schrieb JohnLukeBentley notifications@github.com:

Thanks @TooSharpForYou. That's the trick: effect a global regex replacement.

In the light of testing I took your idea and made three changes.

For reference https://github.com/skandasoft/open-in-browsers/blob/master/lib/open-in-browsers-view.coffee

There's an existing attempt to replace backslashes at line 87 (further down from 71). So I targeted changing that command. (This also ensures blackslash replacments are done regardless of whether serving from a web server or local file system). The backslash needs to be escaped in a regex command. E.g. '/\/g' rather than '/\/g' For some strange reason the string specification of regex switches didn't work. So

86: cmd = "#{cmd} \"#{fpath}\"" 87: cmd = cmd.replace '/\/g', '/' ... didn't work, so I declared a separate regex variable and feed it to the replace method

86: cmd = "#{cmd} \"#{fpath}\"" 87: re = /\/g; 88: cmd = cmd.replace re, '/' I haven't made any pulls and commits that effects this change in the public source as I'm not yet set up for this. So if someone want to do the honours, they can be my guest (and would be doing me a favour).

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

JohnLukeBentley commented 8 years ago

@TooSharpForYou thanks. I went with ...

 87: cmd = cmd.replace /\\/g, '/'

... and verified it works.

If someone else could do the honours and edit https://github.com/skandasoft/open-in-browsers/blob/master/lib/open-in-browsers-view.coffee , then that'd be great (I'm not yet set up for Git).

Your comment helps me get clear in distinguishing the following:

skandasoft commented 8 years ago

Check the latest version

JohnLukeBentley commented 8 years ago

Thanks, @skandasoft!

It is now working in Firefox with or without Switch to LocalHost.

Also tested Chrome, IE 11, and Edge.

Edge fails: The Edge browser is opened but no address is present in the address bar.

skandasoft commented 8 years ago

Can you check from your command prompt.

start microsoft-edge:"http://www.google.com" or with space in between colon..or some combination..if you get it working..can you update the issue..

JohnLukeBentley commented 8 years ago

Tried the following commands in powershell.

PS> start microsoft-edge:"http://www.google.com"
PS> start microsoft-edge:"http://localhost:8080/web/Libraries/Html5Library/html5-polyglot-template-e
xample.xhtml"

Both worked.

skandasoft commented 8 years ago

check the latest version..feel free to open if microsoft edge didn't work

JohnLukeBentley commented 8 years ago

Working in the latest versions of Edge, Chrome, IE 11, and Firefox.

Great work. Thank you kindly.

JohnLukeBentley commented 8 years ago

Spoke too soon.

It works in Edge, Chrome, IE 11, and Firefox when I'm using Switch to LocalHost with LocalHost Url filled in for my environment e.g. http://localhost:8080/web/.

With Switch to LocalHost unchecked Edge fails (but Chrome, IE 11, and Firefox work).

skandasoft commented 8 years ago

what do you mean by it fails

JohnLukeBentley commented 8 years ago

Edge will open without any value in the address bar and without any page loading.

skandasoft commented 8 years ago

I have the issue which you opened clearly explaining. I will keep that and closing this ..Thanks