-
Notifications
You must be signed in to change notification settings - Fork 14
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
Make ContentFrame allow cross-origin clipboard-write permission #6003
Conversation
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.
The situation with regard to the clipboard-write
permission is confusing. w3c/clipboard-apis#164 suggests that it was removed from the spec. However my understanding is that from our collective testing, Chrome still requires it so we'll have to implement it.
LGTM but can you update the comment to describe which applications need the permission, not which pieces of functionality use it, as that is going to get out of date.
// "fullscreen" - Enables full-screen button in player | ||
allow="autoplay; clipboard-write; fullscreen" | ||
allow="autoplay; clipboard-write *; fullscreen" |
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.
I wondered what the value was if the origin is not explicitly specified. According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Permissions_Policy for the allow
attribute it is 'src'
(ie. same origin as the URL in the iframe's src
attribute).
In the context of Via's video player, the origin of the video player itself matches 'src'
, which explains why this worked. However the 'src'
origin doesn't match the client's sidebar, which is why we're having to add the wildcard here.
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.
Thanks! I tried to find a piece of documentation explaining that and couldn't find it for some reason.
@@ -17,9 +17,9 @@ export default function ContentFrame({ url, iframeRef }: ContentFrameProps) { | |||
// too). | |||
// | |||
// "autoplay" - Enables Play button to work without first clicking on video | |||
// "clipboard-write" - Used by "Copy transcript" button | |||
// "clipboard-write *" - Used to copy YouTube transcripts and exported annotations, cross-origin |
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.
Rather than spelling out the individual features that use the permission, which is going to get outdated, it would be easier to spell out which applications need it: Via's video player, the Hypothesis client sidebar.
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.
Makes sense
68ed803
to
348ec1b
Compare
This PR ensures the
iframe
wrapped byContentFrame
allowsclipboard-write
from any origin.This is required to make copying exported annotations work when proxying a site with via inside an LMS assignment.