Firefox WebExtension (and themeName support) #802

bvaughn · 2017-06-06T23:51:42Z

Resolves #705 and #524

Note that this change will mean dropping support for Firefox <= 53 with future releases of the extension. I think this is the right decision to make going forward. Firefox 54 has been released. ~~Firefox 54 is targeted for release on 2017-06-13. I think it's worth reviewing this diff now, but maybe we should hold off on merging it until the 13th.~~

tl;dr

Delete old Firefox add-ons directory
Move Chrome extension to new shells/webextension directory
Replace old build script with new Node script
- Can generate both Firefox and Chrome web extensions
- Runs via npm run build:extension
Add support for Firefox theme mirroring

Firefox themeName (#705)

The default Firefox theme will work as before for Firefox 54 (and earlier). As of the upcoming Firefox 55, DevTools will mimic Firefox's active theme. (This can currently be tested against the Firefox Nightly release.)

References

Change to WebExtensions (#524)

Firefox recently added a new API for browser.devtools.panels.themeName. After some testing it became apparent that this API is only available for WebExtensions and not add-ons (see here). Since Firefox will be dropping add-on support in the future anyway (by the end of the year) it seems the way forward is to switch over to using the WebExtensions build for Firefox as well.

To do this, we could fork the Chrome extension. However there's no differences other than the injected theme name that I'm aware of, so I don't think that's necessary (cc @clarkbw for confirmation). Instead I opted for updating the Firefox shell instructions to reference the Chrome shell instead. (WebExtensions support was first previewed in Firefox 42, in 2015, so hopefully this won't cause any backwards compatibility issues either?)

Another nice thing about switching over Firefox to use the WebExtension is that the DevTools dev-mode icon will work:

Here's the output of the new build script. (Isn't it pretty? 😅)

clarkbw · 2017-06-07T00:10:29Z

Looks good so far!

clarkbw · 2017-06-07T01:48:39Z

shells/firefox/panel/run.js

+// If available, mirror the current Firefox devtools theme:
+// https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/devtools.panels/themeName
+if (typeof devtools !== 'undefined' && devtools.panels) {
+  switch (devtools.panels.themeName) {


Should this be browser.devtools.panels.themeName?

I tried devtools.panels (shown on this page), browser.devtools.panels (shown on this page), and chrome.devtools.panels (shown on this page) but all were undefined. Since the docs mention all of them, I took a guess with devtools.panels since it seems to be mentioned more than the other 2 😅

I'm now wondering if this is because this code is an add-on sdk environment and that API is part of the WebExtensions. browser.devtools.panels.themeName should work if you load the Chrome WebExtension into Firefox.

I'm actually not sure what that means 😁

Edit I found this which, coupled with your comment above, makes me think that I should be using a WebExtensions SDK instead of the add-ons SDK. I'm not sure how that works but I'll read up. (I'm still pretty new to extensions.)

Edit 2 I also found this related issue.

Ah! I think I've figured it out. Seems like I need to mimic the structure in shells/chrome for firefox, and then run web-ext run --firefox=/Applications/FirefoxNightly.app/Contents/MacOS/firefox-bin from the directory. This is a bit more complicated a change than I anticipated. 😁

I looked away and you went all the way down the 🐰 hole! 😲

bvaughn · 2017-06-07T06:09:35Z

Score! Running the Chrome extension in Firefox, but switching on FF's theme names:

bvaughn · 2017-06-07T16:28:58Z

Hey @clarkbw 😁 I just noticed that this PR has shifted in such a way as to overlap with #524

Seems like there's some issue for non-nightly builds (mentioned in the description) but otherwise it seems like the Chrome shell works fine in Firefox. Maybe we could combine forces and finish this effort?

Probably worth renaming shells/chrome to something else at some point. But that could be done in a follow-up to avoid adding noise to this.

bvaughn · 2017-06-07T16:37:37Z

shells/chrome/src/panel.js

-const themeName = (chrome.devtools.panels : any).themeName === 'dark'
-  ? 'ChromeDark'
-  : 'ChromeDefault';
+const IS_CHROME = navigator.userAgent.indexOf('Firefox') < 0;


Is this too clunky? Should I try to inject this parameter via the build script (or similar)?

This is the same thing we did for the redux extension zalmoxisus/redux-devtools-extension@0cf6830

Oh, nice! 😁

clarkbw · 2017-06-07T17:57:58Z

Maybe we could combine forces and finish this effort?

💯

Probably worth renaming shells/chrome to something else at some point. But that could be done in a follow-up to avoid adding noise to this.

Agreed

bvaughn · 2017-06-07T18:07:31Z

Don't suppose you have any insight into what's going on in FF <= 53?

bvaughn · 2017-06-07T18:30:35Z

Ah, nevermind. I think it's b'c we have a reference to chrome.devtools.network elsewhere that isn't being guarded against.

clarkbw · 2017-06-07T18:31:18Z

Testing in Chrome and Firefox Nightly, this seems to work great. Older versions of Firefox I have locally (47, 53) don't quite work though so I still need to look into that

bug 1300590 only just landed in Firefox 55 and is required for features related to inspecting and console. And bug 1300587 landed in 54 which allows for creating the panel. We might need to set the min version at Fx 55

bvaughn · 2017-06-07T18:34:10Z

Hm. Yeah. That seems reasonable I guess. We have too many references we'd have to guard against otherwise.

clarkbw · 2017-06-07T18:47:31Z

Ah, nevermind. I think it's b'c we have a reference to chrome.devtools.network elsewhere that isn't being guarded against.

If it's chrome.devtools.network.onNavigated that should also be working in Firefox. I haven't tested it directly but according to the docs it is there and I haven't seen errors previously.

I have been seeing errors within the doublePipe function which I suspect is causing problems. It appears that one of the pipe references is being lost and I'm not sure why yet.

bvaughn · 2017-06-07T19:44:35Z

Ah, nevermind. I think it's b'c we have a reference to chrome.devtools.network elsewhere that isn't being guarded against.

If it's chrome.devtools.network.onNavigated that should also be working in Firefox. I haven't tested it directly but according to the docs it is there and I haven't seen errors previously.

chrome.devtools.network is undefined for FF 53. The docs show it as being added in 54. (I only have 47, 53, and Nightly local to test with.)

clarkbw · 2017-06-07T19:54:04Z

The docs show it as being added in 54.

Oh right! Fx 54 starts rolling out next week.

clarkbw · 2017-06-07T20:01:31Z

We'll need to figure out a way to use a separate manifest file for the Firefox version so we can set the min Fx version needed.

  "applications": {
    "gecko": {
      "strict_min_version": "55"
    }
  }

More details here and here. However, like you pointed out, that could be done with something like #632

I'd imagine the build steps are something like this:

Build Chrome extension using the default build directory and zip it up as chrome.zip
Build Firefox extension with the applicationattribute injected into the manifest and zip it up as firefox.zip

bvaughn · 2017-06-07T20:08:46Z

I was thinking it might make sense to restructure things:

.
└───┬ shells/
    ├───┬ chrome/
    │   ├─── build/
    │   ├─── manifest.json
    │   └─── README.md
    ├───┬ firefox/
    │   ├─── build/
    │   ├─── manifest.json
    │   └─── README.md
    └───┬ webextension/
        ├─── helpers/
        ├─── icons/
        ├─── popups/
        ├─── src/
        ├─── main.html
        ├─── panel.html
        ├─── README.md
        ├─── webpack.backend.js
        └─── webpack.config.js

Then use a build script (similar to the approach in #632) to package each of the extensions using their own manifests but the shared source. The resulting output could also be stored in the browser-specific directory (eg shells/firefox/build) to avoid confusion.

This way applicatinos.gecko.strict_min_version doesn't leak into the Chrome extension and minimum_chrome_version doesn't leak into Firefox.

bvaughn · 2017-06-07T22:08:25Z

I've got the new folder structure and process I described mostly working. (The one limitation is that you can't build both extensions at the same time. I could fix that though if we think it's important to support?)

Building Chrome

Building Firefox

Running the build script

Now Firefox and Chrome can both share the same source and build script while exporting separate Firefox and Chrome targets.

bvaughn · 2017-06-07T22:23:16Z

I'm going to restructure the code slightly to enable parallel builds by using a temp dir for each build. This will also avoid possibly cluttering the workspace with copied manifests if the build script dies in the middle. Should be easy to do though.

clarkbw · 2017-06-07T22:25:01Z

+1 on the use of chaulk 😄

clarkbw · 2017-06-07T22:26:15Z

shells/firefox/manifest.json

+  "applications": {
+    "gecko": {
+      "id": "extension@react.devtools",
+      "strict_min_version": "54.0"


I should check the versions here. Things mostly work in 54 and definitely work in 55

(Belated follow-up...) My own testing seemed to suggest that everything except the theme worked in 54, and that degrades gracefully to the light theme.

clarkbw · 2017-06-07T22:26:26Z

shells/firefox/manifest.json

+{
+  "manifest_version": 2,
+  "name": "React Developer Tools",
+  "description": "Adds React debugging tools to the Chrome Developer Tools.",


s/Chrome/Firefox/ 😄

Whoops 😅

Also supports building Firefox and Chrome in parallel.

bvaughn · 2017-06-08T03:18:21Z

Okay. The new build script feels pretty solid now. Based on my testing, it generates a useable extension for Chrome (58) and Firefox (54 and 55).

clarkbw · 2017-06-08T03:59:17Z

shells/firefox/build.js

  console.log('web-ext run --firefox=nightly');
+  console.log('web-ext run --firefox=/Applications/Firefox54.app/Contents/MacOS/firefox-bin');


Perhaps in the Fx README if we link to the Firefox releases and people download those then they can use --firefox=firefox or --firefox=beta or --firefox=nightly and it should just work. I think Firefox54.app might be a custom location.

Right now Firefox is 53, Beta is 54, Nightly is 55. Of course next week those numbers will all bump up by one.

Good suggestion. Thanks!

I updated the README as well as the on-screen instructions.

bvaughn · 2017-06-09T15:43:35Z

@gaearon Any thoughts on this one? 😄

bvaughn · 2017-06-13T18:13:58Z

Firefox 54 is rolling out today. Going to test this extension in a fresh install once the final release is available and then I'd like to merge this branch. 😄

bvaughn · 2017-06-16T13:38:22Z

I chatted a bit more with Bryan and it sounds like- after we merge this PR, if we decide to release an update for Firefox versions <=54, we could always release the old version, tag it with "strict_max_version": "53", and the add-ons store would should the right version to the user based on their browser. (To be fair, we probably want to avoid doing this because it sounds like a hassle- but at least it's an option.)

bvaughn · 2017-06-20T13:26:52Z

FYI I'm doing another round of testing this morning for this branch. Assuming I don't find any problems, I'm merging it.

jhen0409 and others added 4 commits June 2, 2017 23:18

[Electron] Add setBrowserName method

77e40a8

PreferencesPanel: Show divider in dropdown if browserName not empty

6ca9aae

Merge branch 'patch-8' of https://github.com/jhen0409/react-devtools

f7c6f8f

Merge branch 'master' of github.com:facebook/react-devtools

8d6ec8c

facebook-github-bot added GH Review: review-needed CLA Signed labels Jun 6, 2017

clarkbw reviewed Jun 7, 2017

View reviewed changes

Deprecated Firefox add-on in favor of WebExtension

b64fd01

bvaughn force-pushed the firefox-themename branch from 5dec92e to b64fd01 Compare June 7, 2017 16:08

bvaughn changed the title ~~Firefox themeName [WIP]~~ Firefox WebExtension [WIP] Jun 7, 2017

Updated Firefox shell README to simplify Nightly testing step

cb692ce

bvaughn commented Jun 7, 2017

View reviewed changes

Moved shells/chrome to shells/webextensions and updated build script

34d8625

Now Firefox and Chrome can both share the same source and build script while exporting separate Firefox and Chrome targets.

bvaughn force-pushed the firefox-themename branch from 4873dcd to 34d8625 Compare June 7, 2017 22:11

Brian Vaughn added 2 commits June 7, 2017 15:15

Updated shell READMEs

578f1cc

Renamed helper for clarity

19ac524

clarkbw reviewed Jun 7, 2017

View reviewed changes

Brian Vaughn added 2 commits June 7, 2017 20:05

Build script entirely Node based now

8b0ac1c

Also supports building Firefox and Chrome in parallel.

Removed nightly from FF build screen

1b2980d

Tidied up READMEs and added top-level node script

cef51c6

bvaughn changed the title ~~Firefox WebExtension [WIP]~~ Firefox WebExtension (and themeName support) Jun 8, 2017

bvaughn requested a review from gaearon June 8, 2017 03:28

Spinners and animation

012573f

clarkbw reviewed Jun 8, 2017

View reviewed changes

Expanded on Firefox testing instructions

209613f

bvaughn mentioned this pull request Jun 8, 2017

Themes follow up work #705

Closed

9 tasks

Merged master and resolved conflicts

7232268

bvaughn merged commit 2c17896 into master Jun 20, 2017

bvaughn deleted the firefox-themename branch June 20, 2017 17:50

This was referenced Jun 20, 2017

Refactor FireFox extension to use WebExtensions #524

Closed

npm script for bundling chrome extension (WIP) #632

Closed

clarkbw mentioned this pull request Jun 20, 2017

webpack command isn't found when following shell/chrome/readme instructions #647

Closed

2 tasks

		console.log('web-ext run --firefox=nightly');
		console.log('web-ext run --firefox=/Applications/Firefox54.app/Contents/MacOS/firefox-bin');

Firefox WebExtension (and themeName support) #802

Firefox WebExtension (and themeName support) #802

Conversation

bvaughn commented Jun 6, 2017 • edited Loading

tl;dr

Firefox themeName (#705)

References

Change to WebExtensions (#524)

clarkbw commented Jun 7, 2017

Choose a reason for hiding this comment

bvaughn Jun 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvaughn Jun 7, 2017 • edited Loading

Choose a reason for hiding this comment

bvaughn Jun 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvaughn commented Jun 7, 2017

bvaughn commented Jun 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clarkbw commented Jun 7, 2017

bvaughn commented Jun 7, 2017 • edited Loading

bvaughn commented Jun 7, 2017

clarkbw commented Jun 7, 2017

bvaughn commented Jun 7, 2017

clarkbw commented Jun 7, 2017

bvaughn commented Jun 7, 2017 • edited Loading

clarkbw commented Jun 7, 2017

clarkbw commented Jun 7, 2017

bvaughn commented Jun 7, 2017 • edited Loading

bvaughn commented Jun 7, 2017 • edited Loading

Building Chrome

Building Firefox

Running the build script

bvaughn commented Jun 7, 2017

clarkbw commented Jun 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvaughn commented Jun 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvaughn commented Jun 9, 2017

bvaughn commented Jun 13, 2017

bvaughn commented Jun 16, 2017

bvaughn commented Jun 20, 2017

bvaughn commented Jun 6, 2017 •

edited

Loading

bvaughn Jun 7, 2017 •

edited

Loading

bvaughn Jun 7, 2017 •

edited

Loading

bvaughn Jun 7, 2017 •

edited

Loading

bvaughn commented Jun 7, 2017 •

edited

Loading

bvaughn commented Jun 7, 2017 •

edited

Loading

bvaughn commented Jun 7, 2017 •

edited

Loading

bvaughn commented Jun 7, 2017 •

edited

Loading

bvaughn commented Jun 7, 2017 •

edited

Loading