-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
FF122 Clipboard API #31137
FF122 Clipboard API #31137
Conversation
d0d4b56
to
537436c
Compare
@@ -148,12 +148,7 @@ | |||
"interfaces": ["Clipboard", "ClipboardEvent", "ClipboardItem"], | |||
"methods": [], | |||
"properties": ["Navigator.clipboard"], | |||
"events": [ | |||
"Window: clipboardchange", |
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.
Window: clipboardchange
is not implemented anywhere.
This is ready for review. There is more I could do, and a little more I might need to do, but I think it is good for now. |
The stuff on permissions seems to be a complicated mess, so we should take care to explain it as well as we can. As I understand it:
Does that cover it? Do you think we need to say something like this? |
@wbamberg Your summary in #31137 (comment) is almost right.
This is correct
The distinction is that things are even more messy than you think.
Upshot, the spec is so confused now it is IMO unimplementable, and so what the spec requires is now a matter of opinion. Given the notes in the spec and comments in the PR I made the decision that we should treat permissions as removed from the spec. So what I did say in concepts section was treat permissions as a thing that a browser can choose to do: I've tried to make this even more clear by adding a new note at the end of the concepts section to capture "the facts". Does this do what you need? |
PS, and I removed the clipboard API from the list of APIs that have permissions in Permission API. I see this as reasonable because the Permissions API spec does not mention the permissions (whether that was premature or not, it is what the spec says). |
Co-authored-by: wbamberg <[email protected]>
Co-authored-by: wbamberg <[email protected]>
47444d1
to
e0d3b01
Compare
@wbamberg I've accepted all the nit fixes. Now trying to work out what is going on with the non-rendering image. Frankly I am stumped, and not sure what to do next.
Any suggestions? I think this is not a bug in my code. I'd like to merge and see if it is something to do with the PR reviewer. |
I don't think this is a bug in your code either. My suggestion:
|
|
||
- Even though the butterfly image is a JPG file, when read from the clipboard it is a PNG. | ||
- If prompted, you will need to grant permission in order to paste the image. | ||
- This may not work on chromium browsers as the sample frame is not granted the [Permissions-Policy](/en-US/docs/Web/HTTP/Headers/Permissions-Policy) `clipboard-read` and `clipboard-write` permissions ([required by Chromium browsers](/en-US/docs/Web/API/Clipboard_API#security_considerations)). |
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.
@wbamberg As discussed, I couldn't get the second image to display - created an issue in mdn/yari#10287
This also logs an error on Chromium due to the permissions policy not being granted to the live sample frame. I tried a LiveSampleLink
because I figured that might work, but on PR preview that page gives a "Not found".
Anyway, think this is good for a re-review.
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.
👍 Looks great, thank you for this @hamishwillee !
* FF122 Clipboard API - basic layout, style subedit, secure_context * copy event clarification * Remove Window: clipboardchange event from GroupData.json - not implemented * Add Element cut/copy/paste events to overview * Security considerations * Events are 'of the Clipboard API' * Clipboard.write() requires transient activation * Clipboard.read() - update example and remove permissions * Clipboard.read*/write* - remove permission mentions * Clipboard interface - remove permissions details * Clipboard API - make concepts a little less abstract * Navigator.clipboard - remove the text on requiring permissions * Minor tweaks * Permissions API- remove clipboard permissions * Add note about Chromium behaviour * read() example handles the permissions handled/or not handled * Merge specific suggestions from code review Co-authored-by: wbamberg <[email protected]> * Fix example as per feedback * read - add example for more reading of data * Remove guide on interacting with web extensions * Security requirements * Add security considerations to the other methods * Fix typos. Update readText * Make examples async * Remove that write one at a time comment * Update files/en-us/web/api/clipboard_api/index.md * typo * Apply suggestions from code review Co-authored-by: wbamberg <[email protected]> * Update files/en-us/web/api/clipboard/read/index.md * Test image source has effect on rendering * Remove clock (not rendering). Try sample link for chromium * Remove the live sample link --------- Co-authored-by: wbamberg <[email protected]>
It only looks this way because of your help and patience. Thank you. |
* FF122 Clipboard API - basic layout, style subedit, secure_context * copy event clarification * Remove Window: clipboardchange event from GroupData.json - not implemented * Add Element cut/copy/paste events to overview * Security considerations * Events are 'of the Clipboard API' * Clipboard.write() requires transient activation * Clipboard.read() - update example and remove permissions * Clipboard.read*/write* - remove permission mentions * Clipboard interface - remove permissions details * Clipboard API - make concepts a little less abstract * Navigator.clipboard - remove the text on requiring permissions * Minor tweaks * Permissions API- remove clipboard permissions * Add note about Chromium behaviour * read() example handles the permissions handled/or not handled * Merge specific suggestions from code review Co-authored-by: wbamberg <[email protected]> * Fix example as per feedback * read - add example for more reading of data * Remove guide on interacting with web extensions * Security requirements * Add security considerations to the other methods * Fix typos. Update readText * Make examples async * Remove that write one at a time comment * Update files/en-us/web/api/clipboard_api/index.md * typo * Apply suggestions from code review Co-authored-by: wbamberg <[email protected]> * Update files/en-us/web/api/clipboard/read/index.md * Test image source has effect on rendering * Remove clock (not rendering). Try sample link for chromium * Remove the live sample link --------- Co-authored-by: wbamberg <[email protected]>
FF122 adds support for the async clipboard API. This PR updates the docs to spec states.
The first commit can largely be ignored as a "subedit for current templates/patterns".
The remainder of this was to remove Permissions API information, which was partially done in #12066, and to add some exceptions information. Also fix up an example.
The permissions stuff is a little hard to resolve because they have been sloppily removed from spec - so we know that the permissions aren't used, but it's hard to work out what actually is done. They are still used in Chrome too. I have "resolved" this primarily by removing mention of permissions but keeping a generic exception for
NotAllowed
. I've also noted that browsers might add additional restrictions on the top level API doc (which captures both the permissions and also that FF asks for a prompt on cross-origin content read.Could also update the top level Clipboard API to have additional examples. But possibly as a follow on.
Related docs project can be tracked in #31104
I probably should add permission info to BCD about permissions too.