Re-enable context menu options in Firefox

[jasonLaster]

#17668 disabled support for "copy to clipboard" and "go to definition" context menu items in Firefox.

Use cases

• copy to clipboard (Devtools renable copy attr context menu for firefox #17740)
• jump to element node (blocked by Firefox bugs 1605597, 1609671, 1609677)
• jump to function definition (blocked by Firefox bugs 1605597, 1609671, 1609677)

[jasonLaster]

@rpl and I looked into why, jump to element did not work.

We found that highlightNativeElement failed to find the correct node to inspect on Firefox link. The likely culprit was findNativeNodesForFiberID.

When we paused on line 122 and manually overrode the node, inspect worked.

One surprising thing we found was that highlightNativeElement seemed to be called twice.

[jasonLaster]

We think the clipboard apis might not be working because they're called from the page. CC @rpl

[jasonLaster]

Luca was curious about $attribute in the inspect case.

[bvaughn]

Ah, I tried to explain the way we were using $attribute here:

The way this works currently is the frontend sends a postMessage command saying to inspect an attribute at a specific path (e.g. some-fiber-id -> props -> foo -> bar). The "renderer" (which is injected to run in the page in the same context as React itself) stores the value at the specified path in a global, window.$attribute. Then the extension itself asks the browser to inspect() it.

The extension is involved because JavaScript running in the document itself can't call inspect() since it's part of the DevTools API and not a standard JavaScript feature. In Firefox though, it seems to just silently fail when the extension calls inspect().

Does this help clear things up any? Or is some part of it unclear?

[bvaughn]

We found that highlightNativeElement failed to find the correct node to inspect on Firefox link. The likely culprit was findNativeNodesForFiberID.

This sounds...unexpected to me. Maybe it failed for a specific component, b'c that didn't have a DOM node, but in general it should work (and it should work the same for both browsers)

[rpl]

@jasonLaster For the "jump to function definition" part, I just filed on bugzilla Bug 1605597 - DevTools API inspect binding called on a function should show it in the debugger panel and attached a patch to it.

would you mind to edit the issue description and link the upstream bug there?

[rpl]

I took a deeper look into the issue with the "copy to clipboard" action from the context menu (the one disabled in #17668 because it does trigger an error when used on Firefox) and it looks that my initial guess was right:

as I mentioned in #17668 (comment) , on Firefox writing in the clipboard from outside of a "user handling" callback is possible but it requires the extension to ask for the clipboardWrite permission and also that the clipboard-related method (either document.execCommand("copy") or navigator.clipboard.writeText) is being called from the extension js code (on the contrary it does still fails if the caller is actually the webpage, because the clipboardWrite doesn't have any effect for the webpage js code) and the [copyToClipboard code is defined in packages/react-devtools-shared/src/backend/utils.js] and called from the webpage js code.

This is the expected behavior on Firefox, and so we would need to fix it on the extension side.

I've been able to workaround the issue and make it to work as expected by using the exportFunction helper from the injectGlobalHook.js content script (exportFunction usage from content script is also documented on MDN here) to inject a clipboardWriteText helper function into the webpage and then by using that helper function from copyToClipboard (if available, otherwise it uses the clipboard-js polyfill as it does right now).

@jasonLaster @bvaughn Follows the diff of the small changes I applied locally to make it work as expected on Firefox:

diff --git a/packages/react-devtools-extensions/firefox/manifest.json b/packages/react-devtools-extensions/firefox/manifest.json
index 498e04425..1918ab740 100644
--- a/packages/react-devtools-extensions/firefox/manifest.json
+++ b/packages/react-devtools-extensions/firefox/manifest.json
@@ -44,7 +44,7 @@
     "scripts": ["build/background.js"]
   },
 
-  "permissions": ["file:///*", "http://*/*", "https://*/*"],
+  "permissions": ["file:///*", "http://*/*", "https://*/*", "clipboardWrite"],
 
   "content_scripts": [
     {
diff --git a/packages/react-devtools-extensions/src/injectGlobalHook.js b/packages/react-devtools-extensions/src/injectGlobalHook.js
index ea41a6c96..e60f875f3 100644
--- a/packages/react-devtools-extensions/src/injectGlobalHook.js
+++ b/packages/react-devtools-extensions/src/injectGlobalHook.js
@@ -91,3 +91,18 @@ if (sessionStorageGetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY) === 'true') {
 injectCode(
   ';(' + installHook.toString() + '(window))' + saveNativeValues + detectReact,
 );
+
+if (typeof exportFunction === 'function') {
+  // eslint-disable-next-line no-undef
+  exportFunction(
+    (text) => {
+      // Call clipboard.writeText from the extension content script
+      // (as it has the clipboardWrite permission) and return a Promise
+      // accessible to the webpage js code.
+      return new window.Promise((resolve, reject) =>
+        window.navigator.clipboard.writeText(text).then(resolve, reject));
+    },
+    window.wrappedJSObject.__REACT_DEVTOOLS_GLOBAL_HOOK__,
+    {defineAs: 'clipboardCopyText'}
+  );
+}
diff --git a/packages/react-devtools-shared/src/backend/utils.js b/packages/react-devtools-shared/src/backend/utils.js
index 47355f207..b0ce3e033 100644
--- a/packages/react-devtools-shared/src/backend/utils.js
+++ b/packages/react-devtools-shared/src/backend/utils.js
@@ -40,7 +40,19 @@ export function cleanForBridge(
 
 export function copyToClipboard(value: any): void {
   const safeToCopy = serializeToString(value);
-  copy(safeToCopy === undefined ? 'undefined' : safeToCopy);
+  const text = safeToCopy === undefined ? 'undefined' : safeToCopy;
+  const {clipboardCopyText} = window.__REACT_DEVTOOLS_GLOBAL_HOOK__;
+
+  // On Firefox navigator.clipboard.writeText has to be called from
+  // the content script js code (because it requires the clipboardWrite
+  // permission to be allowed out of a "user handling" callback),
+  // clipboardCopyText is an helper injected into the page from.
+  // injectGlobalHook.
+  if (clipboardCopyText) {
+    clipboardCopyText(text).catch(err => {});
+  } else {
+    copy(text).catch(err => {});
+  }
 }
 
 export function copyWithSet(

[bvaughn]

Fantastic! Thanks @rpl!

[bvaughn]

FYI v4.4 (released today) re-enables the "copy" menu!

[rpl]

@bvaughn @jasonLaster Bug 1605597 has been landed on Firefox 74 and so it should now be available in the Firefox nightly builds (and the "jump to definition" should work as expected once re-enabled).

(@jasonLaster would you mind to update the TODO list in this issue's description comment accordingly? we can also remove or strikeout jump to element node (blocked by Firefox Bug 1605597), as we verified in our zoom call with Brian that part was actually working as expected even before Bug 1605597).

[bvaughn]

Nice! Thanks for the update @rpl.

[jasonLaster]

Thanks @rpl!

[bvaughn]

@rpl I tried re-enabling locally and testing against version 74.0a1 (2020-01-14) (64-bit)

Overall it seems to be working now which is nice 👍

In many cases though, it points to the first line of a mangled file, so that's not ideal.

There are still some cases in which I'm seeing the following error though:

TypeError: objectGrip.location is undefined

For example:

1. Visit https://reactjs.org/
2. Select the Anonymous component inside of the "Tutorial" menu link (page header).
3. Click to inspect innRef bound function.

I'll attach a build here if you'd like to reproduce it yourself!
ReactDevTools.zip

[rpl]

In many cases though, it points to the first line of a mangled file, so that's not ideal.

There are still some cases in which I'm seeing the following error though:

TypeError: objectGrip.location is undefined

For example:

1. Visit https://reactjs.org/

2. Select the `Anonymous` component inside of the "Tutorial" menu link (page header).

3. Click to inspect `innRef` bound function.

I took a quick look into this, starting from double-checking what is the behavior when inspect is called from the webconsole on that innRef bound function (e.g. by using the "Store as a global variable" action from the context menu and then call inspect($reactTemp1) from the webconsole panel) and I can definitely trigger the same issue from the webconsole:

It looks that the debugger server has been unable to identify where that function has been defined, I guess that something didn't worked as expected during the "source discovery phase".

This issue doesn't look like a bug specific to the WebExtensions devtools API, but a more generic issue in the Firefox debugger internals, we should file a bug to look more deeply into it.

@jasonLaster is the issue described above something that we may have already filed on bugzilla? would you mind to file it if we dont'?

@bvaughn about the functions that are being pointed to the mangled file, do you remember how I could trigger one?
I suspect that like for the other issue the same would happen if inspect is being called directly from the webconsole, but I would like to give it a look to double-check if I can spot any difference with the other issue.

[bvaughn]

@bvaughn about the functions that are being pointed to the mangled file, do you remember how I could trigger one?

You can just "store as global variable" (also using the context menu) and then interact with them. (Am I misunderstanding what you're asking?)

[rpl]

You can just "store as global variable" (also using the context menu) and then interact with them. (Am I misunderstanding what you're asking?)

@bvaughn hehe, sorry, I wasn't very clear :-)

I was asking if there is a particular component that I could use to trigger the issue where "the debugger panel points it to a mangled file"
(Select the "Anonymous" component inside of the "Tutorial" menu link is how I reproduced the "objectGrip.location is undefined" TypeError locally, and so I was wondering if you could point me to another component that would trigger the other issue locally and dig into it a bit)

[bvaughn]

@rpl Gotcha.

1. Open reactjs.org.
2. Select the Anonymous (ForwardRef) component that renders the "Docs" navigation button.
3. Click "<> Go to definition" for the onClick prop.

In Chrome, this jumps you to the specific line (with pretty formatted source):

In Firefox nightly, this jumps you to a single line of (non pretty formatted) source:

[rpl]

@bvaughn 👍 Thanks! The STR was really helpful to track it down quickly!

@jasonLaster @bvaughn it looks that I may have tracked down both these new "inspect binding" issues, I've just filed the following new bugzilla issues:

• Bug 1609671 - toolbox viewSourceInDebugger should try to get the sourcemapped location, this one covers the issue where the mangled file is selected in the debugger panel
• Bug 1609677 - Inspecting a bound function should select the dbgObj.boundTargetFunction in the debugger panel, this one covers the missing location on the bound innerRef function

I've also attached 2 patches on them, which are the changes I've applied locally to fix these two issues (they are not yet on review, mostly because I'm going to push them to the CI to double-check if I may have broken some assumptions and related existing tests and there are a couple of additional test case to add).

@jasonLaster would you mind to link them in this issue description in the meantime?

[bvaughn]

Excellent. Thanks for the update @rpl!

I've updated the description for you to link to the new issues.

[rpl]

@bvaughn both Bug 1609671 and Bug 1609677 have been landed on Firefox 74 and should be available in the Nightly builds.

[bvaughn]

Thanks for the update, @rpl! I think it should be safe to close this issue then 😄

Pleasure working with both of you.