[DevTools] Browser dev tools should open default browser #510

iamdustan · 2015-03-30T20:25:48Z

This changes the browser dev tools tunneling to open the default browser using *nix’s open command instead of applescript.

Benefits:

leave browser tooling decisions to the user.
linux support

Limitations:

Doesn’t detect whether the tab is already open and focus if so (like the Chrome applescript implementation did).

I think that limitation needs to be tackled before merging. I’m not really sure where to start to tackle that. I suspect this is low priority to your team, but is this something you’d be interested in?

Closes #492

frantic · 2015-03-30T21:17:02Z

One possible solution is to make the websocket-proxy smarter. Instead of just broadcasting incoming messages it should be able to detect browser connection and simulator connection and pair them. If there is incoming simulator connection without existing browser counterpart, proxy should open a new browser tab.

iamdustan · 2015-03-30T21:50:10Z

Great idea, @frantic. I’ll explore that over the next day or two.

iamdustan · 2015-04-03T21:05:16Z

@frantic this was a bit of a hack, but there is already a comment saying “this is hacky, refresh the browser window to reattach”.

Originally the chrome already open or not is happened in AppleScript. This just sets a local variable on initial browser connection and then bails out in the future.

I've tested it in chrome and fx dev edition on my bus ride and it was working well for me.

frantic · 2015-04-03T21:23:41Z

packager/packager.js

+      else {
+        var debuggerURL = 'http://localhost:' + options.port + '/debugger-ui';
+        console.log('Launching Dev Tools in default browser...');
+        connected = true;


this means it will open the tab only once per server session.

Yeah, but that’s same as it is currently (as far as I could tell).

E.g. Once you open and attempt to close or refresh the browser tab it says, “if you do this you’ll break the bridge”

But when you close the tab and disable debugging on simulator (via Cmd+Ctrl+Z) or restart the app in Xcode, it's important that the tool can connect to the debugger again.

So the two use cases I’m currently missing are:

Cmd+Ctrl+Z switches back to running js on the device. This should reset the connected state so future Cmd+D can reconnect to a browser tab.

Restarting the simulator from XCode without closing the window should be able to reconnect.

Am I missing anything else?

nicklockwood · 2015-05-05T22:23:31Z

This seems problematic because we use different executor implementations for Chrome/Safari (and don't support Firefox) - unless I'm missing something obvious?

iamdustan · 2015-05-06T00:34:54Z

@nicklockwood is this something that has changed over the past month since I opened this? At the time simply having it open my default browser worked fine.

Still need to work in some time to come revisit this and finish it.

Ignore the [GH Review: review-needed] label and set it back to needs-revision. I’ve only rebased master, haven’t done any additional work yet.

brentvatne · 2015-06-01T19:56:32Z

@iamdustan - any interest in continuing work on this or should we close?

iamdustan · 2015-06-08T15:21:52Z

@brentvatne I am interested, but am currently on 3 projects at work so my time is a bit scarce. :/

Feel free to close for now to get it off the backlog and someday I’ll make it back around to this.

sahrens · 2015-06-11T20:44:15Z

closing for now - feel free to re-open when it's ready for review - we'll need to make sure our full tool stack actually works in chrome, safari, and firebox before we enable something like this, though, otherwise it's just going to open in a broken environment.

This allows... 1. launching Chrome on platforms other than OS X 2. users to launch their own instance of Chrome (e.g. via command line) rather than being forced to use the default instance (i.e. `tell application "Chrome"` always used the default instance) `isClientConnected()` addresses the problem in facebook#510 where the dev tools would only open once per server session.

This allows... 1. launching Chrome on platforms other than OS X 2. users to launch their own instance of Chrome (e.g. via command line) rather than being forced to use the default instance (i.e. `tell application "Chrome"` always used the default instance) `isDebuggerConnected()` addresses the problem in facebook#510 where the dev tools would only open once per server session.

This allows... 1. launching Chrome on platforms other than OS X 2. users to launch their own instance of Chrome (e.g. via command line) rather than being forced to use the default instance (i.e. `tell application "Chrome"` always used the default instance) `isDebuggerConnected()` addresses the problem in facebook#510 where the dev tools would only open once per server session. Add a '--dangerouslyDisableChromeDebuggerWebSecurity' flag to packager.js to enable Chrome '--disable-web-security' flag. This allows users to inspect network requests in Chrome by commenting the xhr polyfill in InitializeJavaScriptAppEngine.js: facebook#934 (comment) Usage: node packager.js --dangerouslyDisableChromeDebuggerWebSecurity or: packager.sh --dangerouslyDisableChromeDebuggerWebSecurity

This allows... 1. launching Chrome on platforms other than OS X 2. users to launch their own instance of Chrome (e.g. via command line) rather than being forced to use the default instance (i.e. `tell application "Chrome"` always used the default instance) `isDebuggerConnected()` addresses the problem in facebook#510 where the dev tools would only open once per server session. Add a '--dangerouslyDisableChromeDebuggerWebSecurity' flag to packager.js to enable Chrome '--disable-web-security' flag. This allows users to inspect network requests in Chrome by commenting the xhr polyfill in InitializeJavaScriptAppEngine.js Usage: node packager.js --dangerouslyDisableChromeDebuggerWebSecurity or: packager.sh --dangerouslyDisableChromeDebuggerWebSecurity

This allows... 1. launching Chrome on platforms other than OS X 2. users to launch their own instance of Chrome (e.g. via command line) rather than being forced to use the default instance (i.e. `tell application "Chrome"` always used the default instance) `isDebuggerConnected()` addresses the problem in facebook#510 where the dev tools would only open once per server session. Add a '--dangerouslyDisableChromeDebuggerWebSecurity' flag to packager.js to enable Chrome '--disable-web-security' flag. This allows users to inspect network requests in Chrome by commenting the xhr polyfill in InitializeJavaScriptAppEngine.js: facebook#934 (comment) Usage: node packager.js --dangerouslyDisableChromeDebuggerWebSecurity or: packager.sh --dangerouslyDisableChromeDebuggerWebSecurity

This allows opening the Chrome debugger on OS X, Linux, and Windows. `isDebuggerConnected()` addresses the problem in facebook#510 where the dev tools would only open once per server session.

…tch (facebook#510) * Add unsafeAddProp (like ReactDOMRe.Style) & unsafeTransform as escape hatch for fancy platforms (and move array/list(Option) at the top - without huge comments) This escape hatch are a necessity when using react-native-web (eg: position: fixed on the web - as you cannot always have 60fps sticky header with interpolation+translateY... Also RNW allows `translateZ`etc...) * Replace with unsafeAddStyle, unsafeStyle & unsafeTransform * Zero cost unsafeAddStyle 🤯

vjeux added the GH review: needs-revision label Apr 3, 2015

iamdustan force-pushed the default-browser-dev-tools branch from b9f4966 to f0d2d0d Compare April 3, 2015 21:00

frantic reviewed Apr 3, 2015
View reviewed changes

iamdustan mentioned this pull request Apr 6, 2015

[Debugger] Chrome popping up on every reload (Already closed the debugging session) #689

Closed

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 7, 2015

iamdustan added 2 commits May 5, 2015 20:32

Browser dev tools should open default browser

8afecff

hacky "hey I already connected" approach to multi-browser support

509a97e

iamdustan force-pushed the default-browser-dev-tools branch from f0d2d0d to 509a97e Compare May 6, 2015 00:33

reactjs-bot added GH Review: review-needed and removed GH Review: needs-revision labels May 6, 2015

brentvatne changed the title ~~(wip) Browser dev tools should open default browser~~ [DevTools] Browser dev tools should open default browser Jun 1, 2015

sahrens closed this Jun 11, 2015

elliottsj mentioned this pull request Aug 22, 2015

Replace applescript with browser-launcher2 #2406

Closed

mganandraj pushed a commit to mganandraj/react-native that referenced this pull request Jul 21, 2020

Manually bump version to .83. (facebook#510)

b845fef

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DevTools] Browser dev tools should open default browser #510

[DevTools] Browser dev tools should open default browser #510

iamdustan commented Mar 30, 2015

frantic commented Mar 30, 2015

iamdustan commented Mar 30, 2015

iamdustan commented Apr 3, 2015

frantic Apr 3, 2015

iamdustan Apr 3, 2015

frantic Apr 3, 2015

iamdustan Apr 3, 2015

nicklockwood commented May 5, 2015

iamdustan commented May 6, 2015

brentvatne commented Jun 1, 2015

iamdustan commented Jun 8, 2015

sahrens commented Jun 11, 2015

[DevTools] Browser dev tools should open default browser #510

[DevTools] Browser dev tools should open default browser #510

Conversation

iamdustan commented Mar 30, 2015

frantic commented Mar 30, 2015

iamdustan commented Mar 30, 2015

iamdustan commented Apr 3, 2015

frantic Apr 3, 2015

Choose a reason for hiding this comment

iamdustan Apr 3, 2015

Choose a reason for hiding this comment

frantic Apr 3, 2015

Choose a reason for hiding this comment

iamdustan Apr 3, 2015

Choose a reason for hiding this comment

nicklockwood commented May 5, 2015

iamdustan commented May 6, 2015

brentvatne commented Jun 1, 2015

iamdustan commented Jun 8, 2015

sahrens commented Jun 11, 2015