Skip to content

Add Clipboard Feature Policy#120

Open
dway123 wants to merge 4 commits into
w3c:mainfrom
dway123:feature-policy-add-2
Open

Add Clipboard Feature Policy#120
dway123 wants to merge 4 commits into
w3c:mainfrom
dway123:feature-policy-add-2

Conversation

@dway123

@dway123 dway123 commented Jun 13, 2020

Copy link
Copy Markdown

Closes #106

The following tasks have been completed:

  • Confirmed there are no ReSpec/BikeShed errors or warnings.
  • Modified Web platform tests

Implementation commitment:


Preview | Diff

Comment thread index.bs Outdated
Comment thread index.bs Outdated
@marcoscaceres

Copy link
Copy Markdown
Member

again, it's kinda annoying that we ended up with "clipboard-read" and "clipboard-write" as the feature name :( ... would have been nice to have a single name, as we are again potentially setting a bad precedence.

@dway123

dway123 commented Jun 17, 2020

Copy link
Copy Markdown
Author

Thanks for the review! I've updated the change a bit now per the comments.

Yeah, I agree that a single feature name would have been nice as well, but am just trying to match the clipboard spec's permission names with the feature policy, as feature policy and permissions are merging since this change.

@marcoscaceres

Copy link
Copy Markdown
Member

Filed bug on the Gecko side too.

Comment thread index.bs
Comment on lines 627 to +632
1. Let |p| be a new [=Promise=].

1. If the [=current settings object's=] responsible document is not
[=allowed to use=] the "clipboard-write" feature, then reject |p|
with a "NotAllowedError" DOMException.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This current algorithm currently continues with the rejected promise, instead of returning it. Let's do:

Suggested change
1. Let |p| be a new [=Promise=].
1. If the [=current settings object's=] responsible document is not
[=allowed to use=] the "clipboard-write" feature, then reject |p|
with a "NotAllowedError" DOMException.
1. If the [=current settings object's=] responsible document is not
[=allowed to use=] the "clipboard-write" feature, then
return a promise rejected with a "NotAllowedError" DOMException.
1. Let |p| be a new [=Promise=].

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nits... I think you can link [=responsible document=] and [=a promise rejected with=] ... please confirm by looking up the terms at https://respec.org/xref/ ... you can probably link {{DOMException}} too.

Comment thread index.bs

1. Let |p| be a new [=Promise=].

1. If the [=current settings object's=] responsible document is not

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above... immediately return the rejected promise, otherwise continue in parallel.

Comment thread index.bs

1. Let |p| be a new [=Promise=].

1. If the [=current settings object's=] responsible document is not

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above here too...

Comment thread index.bs

1. Let |p| be a new [=Promise=].

1. If the [=current settings object's=] responsible document is not

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same... 😊

@marcoscaceres marcoscaceres left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super close... what might be interesting now is to rebase this given the transient activation text is in the spec... we might be able to also do the transient activation checks early.

@saschanaz

saschanaz commented Jun 18, 2020

Copy link
Copy Markdown
Member

again, it's kinda annoying that we ended up with "clipboard-read" and "clipboard-write" as the feature name :(

Are we too late to rename them? It seems currently Chrome has the only implementation, should we discuss about renaming? (I think I saw somewhere that WebKit has no intent to implement them. Edit: Yes, #101 (comment))

@marcoscaceres

Copy link
Copy Markdown
Member

Yes, if only Firefox and Chrome are supporting these, we should discuss renaming them... but we would need to commit to implementing them too.

@saschanaz

Copy link
Copy Markdown
Member

Firefox has no implementation, only Chrome does.

@marcoscaceres

Copy link
Copy Markdown
Member

Firefox has no implementation, only Chrome does.

Understood, but we should figure out if we will implement it in the future (and figure out if what's in the spec is palatable to us, and remove what Gecko is not willing to support to match reality).

Base automatically changed from master to main February 3, 2021 04:30
@wandyezj

Copy link
Copy Markdown

What is really annoying is that browsers have gone ahead with their own implementation for a critical feature. This ends up forcing developers to use commands to read and write to the clipboard to work around this nonsense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restrict Clipboard API to top-level origin

4 participants