fix[devtools/extension]: added a workaround for proxy content script injection in firefox #27375

hoxyq · 2023-09-14T18:16:04Z

Changes:

[Firefox-only] For some reason, Firefox might try to inject dynamically registered content script in pages like about:blank. I couldn't find a way to change this behaviour, about: is not a valid scheme, so we can't exclude it and match_about_blank flag is not supported in chrome.scripting.registerContentScripts.
While navigating the history in Firefox, some content scripts might not be re-injected and still be alive. To handle this, we are now patching window with __REACT_DEVTOOLS_PROXY_INJECTED__ flag, to make sure that proxy is injected and only once. This flag is cleared on pagehide event.

Biki-das · 2023-09-15T08:06:17Z

is this related to the issue where react devtools extension when running on firefox and if we search discord.com, the devtools extension won't light up. though discord.com runs react and chrome and edge works fine in this case

Might be solving this one since this is solely due to the proxy injection thing!

hoxyq · 2023-09-15T10:25:15Z

Changes:

[Firefox-only] For some reason, Firefox might try to inject dynamically registered content script in pages like about:blank. I couldn't find a way to change this behaviour, about: is not a valid scheme, so we can't exclude it and match_about_blank flag is not supported in chrome.scripting.registerContentScripts.

While navigating the history in Firefox, some content scripts might not be re-injected and still be alive. To handle this, we are now patching window with __REACT_DEVTOOLS_PROXY_INJECTED__ flag, to make sure that proxy is injected and only once. This flag is cleared on unload event.

is this related to the issue where react devtools extension when running on firefox and if we search discord.com, the devtools extension won't light up. though discord.com runs react and chrome and edge works fine in this case
Might be solving this one since this is solely due to the proxy injection thing!

No, this should not be related, but I will check if current version of the extension for Firefox works on discord.com

hoxyq · 2023-09-15T10:34:34Z

Discord blocks script injection via XMLHttpRequest, which we do here.

We do this only for Firefox, because it doesn't support ExecutionWorld.MAIN yet. The only other option for us is to use chrome.devtools.eval, but we need to inject this script before React has loaded on the page, using chrome.devtools.eval can't guarantee it, unless we would also reload the whole document again.

This is also the reason why we haven't migrated Firefox extension to manifests v3 yet.

gsathya · 2023-09-18T14:58:13Z

packages/react-devtools-extensions/src/contentScripts/proxy.js

@@ -2,23 +2,17 @@

 'use strict';

+window.addEventListener('unload', function ({target}) {


Is this going to break the bf-cache for pages even when the devtools is not open? https://developer.mozilla.org/en-US/docs/Web/API/Window/unload_event#usage_notes

Yes, but only this content script won't be put in bfcache. This is our goal, because if content scripts in Firefox are not unloaded / loaded properly again, extension might not receive some important events that are necessary to start

Are you sure that it's just the script not ending up in bf-cache?

MDN says:

To combat this, some browsers (such as Firefox) will not place pages in the bfcache if they have unload listeners, and this is bad for performance.

Yeah, I interpret

will not place pages in the bfcache if they have unload listeners, and this is bad for performance.

as "if page has unload listeners, it won't be in bfcache", this doesn't mean that every page won't be in bfcache.

Content scripts are loaded separately from "page", and I think for Firefox it has a distinct page, so it won't be cached, but page, which runs react application, will. I will double-check this though.

lgtm once you double check

Screen.Recording.2023-09-20.at.14.42.57.mov

Looks like it really breaks bfcache for Firefox. I've moved to pagehide / pageshow events, they are bfcache-compatible. Thanks again, re-requesting review.

…injection in firefox

hoxyq · 2023-09-22T18:35:53Z

Merging this, since we've discussed it in person, cc @gsathya

* refactor[devtools/extension]: refactored messaging logic across different parts of the extension ([hoxyq](https://github.com/hoxyq) in [#27417](#27417)) * fix[devtools/extension]: added a workaround for proxy content script injection in firefox ([hoxyq](https://github.com/hoxyq) in [#27375](#27375)) * fix[devtools/useTransition]: don't check for dispatch property when determining if hook is stateful ([hoxyq](https://github.com/hoxyq) in [#27365](#27365)) * feat[devtools/extension]: show disclaimer when page doesnt run react and refactor react polling logic ([hoxyq](https://github.com/hoxyq) in [#27373](#27373)) * feat:-Added a delete all filters action and added title to the add filter a… ([Biki-das](https://github.com/Biki-das) in [#27332](#27332)) * fix[devtools/extension]: unregister dynamically injected content scripts instead of filtering ([hoxyq](https://github.com/hoxyq) in [#27369](#27369)) * refactor[devtools/extension]: more stable element updates polling to avoid timed out errors ([hoxyq](https://github.com/hoxyq) in [#27357](#27357)) * feat[devtools/extension]: add dark theme for popup ([rakleed](https://github.com/rakleed) in [#27330](#27330))

### React upstream changes - facebook/react#27417 - facebook/react#27408 - facebook/react#27409 - facebook/react#27405 - facebook/react#27375 - facebook/react#27407 - facebook/react#27365 - facebook/react#27399 - facebook/react#27395 - facebook/react#27394 - facebook/react#27397 - facebook/react#26992 - facebook/react#27388 - facebook/react#27373 - facebook/react#27332

…injection in firefox (facebook#27375) Changes: 1. [Firefox-only] For some reason, Firefox might try to inject dynamically registered content script in pages like `about:blank`. I couldn't find a way to change this behaviour, `about:` is not a valid scheme, so we can't exclude it and `match_about_blank` flag is not supported in `chrome.scripting.registerContentScripts`. 2. While navigating the history in Firefox, some content scripts might not be re-injected and still be alive. To handle this, we are now patching `window` with `__REACT_DEVTOOLS_PROXY_INJECTED__` flag, to make sure that proxy is injected and only once. This flag is cleared on `pagehide` event.

* refactor[devtools/extension]: refactored messaging logic across different parts of the extension ([hoxyq](https://github.com/hoxyq) in [facebook#27417](facebook#27417)) * fix[devtools/extension]: added a workaround for proxy content script injection in firefox ([hoxyq](https://github.com/hoxyq) in [facebook#27375](facebook#27375)) * fix[devtools/useTransition]: don't check for dispatch property when determining if hook is stateful ([hoxyq](https://github.com/hoxyq) in [facebook#27365](facebook#27365)) * feat[devtools/extension]: show disclaimer when page doesnt run react and refactor react polling logic ([hoxyq](https://github.com/hoxyq) in [facebook#27373](facebook#27373)) * feat:-Added a delete all filters action and added title to the add filter a… ([Biki-das](https://github.com/Biki-das) in [facebook#27332](facebook#27332)) * fix[devtools/extension]: unregister dynamically injected content scripts instead of filtering ([hoxyq](https://github.com/hoxyq) in [facebook#27369](facebook#27369)) * refactor[devtools/extension]: more stable element updates polling to avoid timed out errors ([hoxyq](https://github.com/hoxyq) in [facebook#27357](facebook#27357)) * feat[devtools/extension]: add dark theme for popup ([rakleed](https://github.com/rakleed) in [facebook#27330](facebook#27330))

…injection in firefox (#27375) Changes: 1. [Firefox-only] For some reason, Firefox might try to inject dynamically registered content script in pages like `about:blank`. I couldn't find a way to change this behaviour, `about:` is not a valid scheme, so we can't exclude it and `match_about_blank` flag is not supported in `chrome.scripting.registerContentScripts`. 2. While navigating the history in Firefox, some content scripts might not be re-injected and still be alive. To handle this, we are now patching `window` with `__REACT_DEVTOOLS_PROXY_INJECTED__` flag, to make sure that proxy is injected and only once. This flag is cleared on `pagehide` event. DiffTrain build for commit f9d75e3.

hoxyq requested review from gsathya and motiz88 September 14, 2023 18:16

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Sep 14, 2023

hoxyq force-pushed the devtools/firefox-workaround-for-proxy-content-script-injection branch from d16301e to 5a6feba Compare September 15, 2023 10:35

gsathya reviewed Sep 18, 2023

View reviewed changes

gsathya approved these changes Sep 20, 2023

View reviewed changes

fix[devtools/extension]: added a workaround for proxy content script …

45142d6

…injection in firefox

hoxyq force-pushed the devtools/firefox-workaround-for-proxy-content-script-injection branch from 5a6feba to 45142d6 Compare September 20, 2023 13:48

hoxyq requested a review from gsathya September 20, 2023 13:51

hoxyq merged commit f9d75e3 into facebook:main Sep 22, 2023
36 checks passed

hoxyq deleted the devtools/firefox-workaround-for-proxy-content-script-injection branch September 22, 2023 18:36

hoxyq mentioned this pull request Sep 25, 2023

React DevTools 4.28.3 -> 4.28.4 #27419

Merged

shuding mentioned this pull request Sep 26, 2023

Update vendored React vercel/next.js#56012

Merged

saurabharch mentioned this pull request Nov 28, 2023

[Snyk] Security upgrade react-devtools-core from 4.26.1 to 4.28.4 saurabharch/react#394

Open

leftrightleft mentioned this pull request Nov 29, 2023

[Snyk] Security upgrade react-devtools-core from 4.24.1 to 4.28.4 leftrightleft/react#56

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix[devtools/extension]: added a workaround for proxy content script injection in firefox #27375

fix[devtools/extension]: added a workaround for proxy content script injection in firefox #27375

hoxyq commented Sep 14, 2023 •

edited

Loading

Biki-das commented Sep 15, 2023 •

edited

Loading

hoxyq commented Sep 15, 2023

hoxyq commented Sep 15, 2023 •

edited

Loading

gsathya Sep 18, 2023

hoxyq Sep 20, 2023

gsathya Sep 20, 2023

hoxyq Sep 20, 2023 •

edited

Loading

gsathya Sep 20, 2023

hoxyq Sep 20, 2023 •

edited

Loading

hoxyq commented Sep 22, 2023

		@@ -2,23 +2,17 @@

		'use strict';

		window.addEventListener('unload', function ({target}) {

fix[devtools/extension]: added a workaround for proxy content script injection in firefox #27375

fix[devtools/extension]: added a workaround for proxy content script injection in firefox #27375

Conversation

hoxyq commented Sep 14, 2023 • edited Loading

Biki-das commented Sep 15, 2023 • edited Loading

hoxyq commented Sep 15, 2023

hoxyq commented Sep 15, 2023 • edited Loading

gsathya Sep 18, 2023

Choose a reason for hiding this comment

hoxyq Sep 20, 2023

Choose a reason for hiding this comment

gsathya Sep 20, 2023

Choose a reason for hiding this comment

hoxyq Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

gsathya Sep 20, 2023

Choose a reason for hiding this comment

hoxyq Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

hoxyq commented Sep 22, 2023

hoxyq commented Sep 14, 2023 •

edited

Loading

Biki-das commented Sep 15, 2023 •

edited

Loading

hoxyq commented Sep 15, 2023 •

edited

Loading

hoxyq Sep 20, 2023 •

edited

Loading

hoxyq Sep 20, 2023 •

edited

Loading