Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wider Chromium support for openBrowser #8367

Merged
merged 3 commits into from Jan 31, 2020

Conversation

handeyeco
Copy link

@handeyeco handeyeco commented Jan 23, 2020

Hey there! The Contribution Guidelines say to ask before submitting a PR, but it doesn't say how to ask. I don't have a lot of experience contributing to OSS, so please let me know if I missed something.

This PR allows for CRA to control tabs on other Chromium-based browsers besides Google Chrome. How the code works:

  1. Checks if AppleScript is applicable. Logic isn't changed, but I renamed the variable to shouldTryOpenChromiumWithAppleScript.
  2. Runs through a list of supported browsers:
    • Chrome Canary (has to come before Chrome because of grep)
    • Chrome
    • Edge
    • Brave
    • Vivaldi
    • Chromium
  3. For each browser, it greps to see if that browser is a process
  4. If it finds a match, it passes the browser name to the AppleScript
  5. AppleScript uses the Google Chrome dictionary to control the provided browser

Other than that, the logic is essentially the same. Most people shouldn't see a difference in behavior except for those using a browser in the list above that's not Chrome; for those people, CRA will reuse the existing tab rather than a new tab.

I noticed that Safari has its own dictionary for AppleScripts. If it benefits anyone, I could look into implementing similar logic for Safari users. Unfortunately for Firefox users like myself, there seems to be no hope.

Apologies if I left something out, but I tried my best. Let me know what needs to be changed, would be proud to have my code in CRA.

Closes #8264

Adjust openChrome.applescript to allow manipulation of
other Chromium-based browsers (defaulting to Chrome).
Requires list of compatible browsers to try in openBrowser.js
@handeyeco
Copy link
Author

Was able to run this with OSX/Node 8 and OSX/Node 10. Not sure why this would be failing. Any suggestions would be welcome.

@favna
Copy link
Contributor

favna commented Jan 27, 2020

From what I can tell from the CI logs it's something in the Azure Pipelines config or flakyness in its container. It, for some unknown reason, is unable to symlink npm when running npm i -g npm@latest.

@andriijas andriijas added this to the 3.4 milestone Jan 31, 2020
@andriijas andriijas merged commit d2de54b into facebook:master Jan 31, 2020
@andriijas
Copy link
Contributor

Thanks for making the web browser space more inclusive and diverse!

@iamandrewluca
Copy link
Contributor

@handeyeco

Not sure why this would be failing

This will fix mac build failures #8402

@lock lock bot locked and limited conversation to collaborators Feb 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Vivaldi & Brave browsers for "openChrome.applescript"
6 participants