chore[DevTools]: make clipboardWrite optional for chromium #32262
Conversation
github-actions
bot
added
the
React Core Team
|
Comparing: 55b54b0...dfd27fb Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
hoxyq
force-pushed
the
react-devtools/make-clipboard-write-optional-for-chromium
branch
from
53719e0 to
cdf5c81
Compare
hoxyq
force-pushed
the
react-devtools/make-clipboard-write-optional-for-chromium
branch
from
cdf5c81 to
85d3d00
Compare
| return callback; | ||
| } else { | ||
| return async () => { | ||
| const alreadyContains = await chrome.permissions.contains(options); |
There was a problem hiding this comment.
Do we need this check? I'd expect that chrome.permissions.request does not prompt if you already granted permissions. At least that's how I interpret "displaying a prompt to the user if necessary." (https://developer.chrome.com/docs/extensions/reference/api/permissions#method-request)
There was a problem hiding this comment.
Yes, I believe when I was just requesting the permission, it was showing me the dialog window each time. I will double-check later today and follow-up on this with you.
There was a problem hiding this comment.
Checked that again, the request dialog is not shown for permissions that already granted, thanks for pointing out! Removed redundant check.
| onClick: withPermissionsCheck({permissions: ['clipboardWrite']}, () => | ||
| copy(flamechartStackFrame.scriptUrl), | ||
| ), |
There was a problem hiding this comment.
Do we currently have any copy that doesn't check permissions? Feels like every copy call should check those i.e. we should have the permission check built-into a custom copy function.
There was a problem hiding this comment.
No, I've double-checked every import of clipboard-js that we are using for copying to clipboard.
Ideally, we should stop using this deprecated npm package, and migrate to navigator.clipboard.writeText(...), but it will be a really tricky case for our current setup.
In short, React DevTools is portaling React roots to other containers, which are mounted in different document, because of the way browser sets up third-party panels for DevTools. This setup leaves the document, which handles user interactions (like click-to-copy), unfocused, and we can't perform navigator.clipboard.writeText(...) on unfocused documents.
hoxyq
force-pushed
the
react-devtools/make-clipboard-write-optional-for-chromium
branch
from
85d3d00 to
c16b8ba
Compare
hoxyq
deleted the
react-devtools/make-clipboard-write-optional-for-chromium
branch
…32262) Addresses facebook#32244. ### Chromium We will use [chrome.permissions](https://developer.chrome.com/docs/extensions/reference/api/permissions) for checking / requesting `clipboardWrite` permission before copying something to the clipboard. ### Firefox We will keep `clipboardWrite` as a required permission, because there is no reliable and working API for requesting optional permissions for extensions that are extending browser DevTools: - `chrome.permissions` is unavailable for devtools pages - https://bugzilla.mozilla.org/show_bug.cgi?id=1796933 - You can't call `chrome.permissions.request` from background, because this instruction has to be executed inside user-event callback, basically only initiated by user. I don't really want to come up with solutions like opening a new tab with a button that user has to click. DiffTrain build for [221f300](facebook@221f300)
…32262) Addresses facebook#32244. ### Chromium We will use [chrome.permissions](https://developer.chrome.com/docs/extensions/reference/api/permissions) for checking / requesting `clipboardWrite` permission before copying something to the clipboard. ### Firefox We will keep `clipboardWrite` as a required permission, because there is no reliable and working API for requesting optional permissions for extensions that are extending browser DevTools: - `chrome.permissions` is unavailable for devtools pages - https://bugzilla.mozilla.org/show_bug.cgi?id=1796933 - You can't call `chrome.permissions.request` from background, because this instruction has to be executed inside user-event callback, basically only initiated by user. I don't really want to come up with solutions like opening a new tab with a button that user has to click. DiffTrain build for [221f300](facebook@221f300)
We started using these globals in `react-devtools-shared/src/frontend` code, forward-fixing #32262.
Full list of changes: * DevTools: refactor NativeStyleEditor, don't use custom cache implementation ([hoxyq](https://github.com/hoxyq) in [#32298](#32298)) * fix[react-devtools-fusebox]: add extension globals to build ([hoxyq](https://github.com/hoxyq) in [#32297](#32297)) * DevTools: fix host component filter option title ([hoxyq](https://github.com/hoxyq) in [#32296](#32296)) * chore[DevTools]: make clipboardWrite optional for chromium ([hoxyq](https://github.com/hoxyq) in [#32262](#32262)) * DevTools: support useEffectEvent and forward-fix experimental prefix support ([hoxyq](https://github.com/hoxyq) in [#32106](#32106))
Addresses #32244.
Chromium
We will use chrome.permissions for checking / requesting
clipboardWritepermission before copying something to the clipboard.Firefox
We will keep
clipboardWriteas a required permission, because there is no reliable and working API for requesting optional permissions for extensions that are extending browser DevTools:chrome.permissionsis unavailable for devtools pages - https://bugzilla.mozilla.org/show_bug.cgi?id=1796933chrome.permissions.requestfrom background, because this instruction has to be executed inside user-event callback, basically only initiated by user.I don't really want to come up with solutions like opening a new tab with a button that user has to click.