-
Notifications
You must be signed in to change notification settings - Fork 887
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reverts Chromium's [Clipboard API] Remove user gesture requirement fo… #14901
Conversation
Building now to give it a try 😄 |
|
||
#define BRAVE_CLIPBOARD_PROMISE_REQUEST_PERMISSION \ | ||
false); \ | ||
if( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, but imo quite unclear change for a pretty critical issue.
maybe just add our own block to prevent any clipboard interaction without user gesture? (and add test?)
if (!has_transient_user_activation) {
script_promise_resolver_->Reject(MakeGarbageCollected<DOMException>(
DOMExceptionCode::kSecurityError,
"Must be handling a user gesture to use clipboard"));
return;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@goodov agree that we may want to add our check, but I'd prefer to limit this fix to just reverting the Chromium's temporary(?) relaxation of the gesture requirement. @ShivanKaul do we need a follow up issue to consider adding our own restriction here as @goodov suggesting?
NOTE: we'll want to update https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove) when we merge |
2650ae9
to
77aa52e
Compare
…r read/writeText. We should not relax the gesture requirement. Chromium change: https://chromium.googlesource.com/chromium/src/+/4d7b74b051abfe5945f418601fdc2ffc8ce3072c commit 4d7b74b051abfe5945f418601fdc2ffc8ce3072c Author: Anupam Snigdha <snianu@microsoft.com> Date: Tue Jun 7 16:36:28 2022 +0000 [Clipboard API] Remove user gesture requirement for read/writeText. Adding user gesture requirement for readText and writeText APIs breaks NTP doodle sharing. We are relaxing this check for now, but we should fix this for sites to not rely on these APIs to be called without a user gesture. See NewTabPageDoodleShareDialogFocusTest.All test for more details. Bug: 106449, 1334203
77aa52e
to
d353be9
Compare
Force pushed the fix for the lint error. |
Reproduced the issue on
Went through the STR/Cases outlined via #14901 (comment) and reproduced the original issue as per the following: Verification PASSED on
Went through the STR/Cases outlined via #14901 (comment) and ensured that a
|
…r read/writeText.
We should not relax the gesture requirement.
Chromium change:
https://chromium.googlesource.com/chromium/src/+/4d7b74b051abfe5945f418601fdc2ffc8ce3072c
commit 4d7b74b051abfe5945f418601fdc2ffc8ce3072c
Author: Anupam Snigdha snianu@microsoft.com
Date: Tue Jun 7 16:36:28 2022 +0000
Resolves brave/brave-brwoser#16890
(It doesn't strictly address the original issue, because Chromium's initial implementation before the above change does require a gesture already.)
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
,npm run lint
,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: