Fix Mac OS X browser launching

[marcelofarias]

No description provided.

[nerdbeere]

Hey there, thanks for contributing.
Could you fix the linting error please?

@tomitm @mitchhentges I'm now on linux, can you verify this?

[mitchhentges]

Hey, thanks for the PR! I won't have access to a Mac for another two weeks, but I like this PR: I'm not really sure how this was working before, hah
Marcelo, what browser are you using that wasn't working? Were all of them failing before, and now Firefox/Chrome/Safari work?

[marcelofarias]

I can verify Chrome, Chrome Canary, Firefox, Safari and Opera are working on Mac OS X El Capitan.

I've tried james-browser-launcher as a drop in replacement for browser-launcher2, and it indeed fixed some problems I was seeing in a headless Linux CI environment. However, it broke browser launching on Mac OS X. After further investigation, I've noticed it was all related to malformation of the argument list.

For Chrome, arr.pop() was getting --noerrdialogs instead of the URL, because now the URL is at the beginning of the array, not at the end. This is what I was getting:

Also for Chrome, I had to add the URL after the -a <path to chrome>, otherwise it won't open the URL in a new tab in case Chrome is already open (it was opening a blank new window instead).

Safari was a similar situation.

Firefox is now working since we're doing the open -a mojo for all browsers.

Opera is also working.

Haven't tried phantomjs, though.

Any reason to not do open -a for all browsers?

About the lint error, it comes from the regexp in run.js line 125. ESlint got more aggressive lately: eslint/eslint#7656. Not sure if you really want to remove the escape before /], though.

Cheers
Marcelo

[mitchhentges]

Yeah, since -a is used for specifying "the application to use for opening the file", it's bizarre that it's only appended to args depending on the browser. Though, when this project was forked from browser-launcher2, that was the behaviour.. Perhaps in refactoring the logic, I broke some other logic.

Thanks again for the PR! If you don't mind, just update this branch with master to include your eslint fix (:heart:) and we should be good-to-go. I'll make a release once this is in.

[marcelofarias]

Thought the same. Why did it ever had to be different for Firefox and PhantomJS? Anyway, do you have a PhantomJS in hand to give it a try? Will rebase it in a moment.

[mitchhentges]

Anyway, do you have a PhantomJS in hand to give it a try? Will rebase it in a moment.

I don't fully understand, sorry. Like, a version of phantomJS to test with, or a local installation? I don't currently have macOS installed on anything, so I can't test it. If you're feeling super awesome 💯/💯, it is downloadable with npm install -g phantomjs, IIRC. I don't believe that we're tied to a specific version

[mitchhentges]

Considering that this is an improvement either way, I'll merge this, release it, and we can improve phantomjs on macOS if there's any issues

[mitchhentges]

Marcelo, I've published release v1.2.7 to npm

[marcelofarias]

@mitchhentges ping - Skype?