search

w3c / encrypted-media

Encrypted Media Extensions
https://w3c.github.io/encrypted-media/
Other
180 stars 80 forks source link

Fix #364: Integrate EME with Feature Policy #432

Closed raymeskhoury closed 4 years ago

raymeskhoury commented 6 years ago

This adds a section defining the feature name and default allowlist, as well as integration with the requestMediaKeySystemAccess algorithm.

Preview | Diff

raymeskhoury commented 6 years ago

@clelland we still need to fix the "allowed to use" monkey-patch to make it take a policy-controlled feature. Is that something you could do?

@ddorwin ptal - I'm sure I've violated some style conventions but help would be appreciated in how to do things right - I'm not very experienced with this stuff!

I'm also in the process of getting a W3C account linked...

raymeskhoury commented 6 years ago

@ddorwin ping :)

raymeskhoury commented 6 years ago

Thanks - please let me know how to update index.html.

I'm happy for you to do the merge if it's easier.

ddorwin commented 6 years ago

See https://github.com/w3c/encrypted-media/blob/master/TEAM.md#updating-indexhtml for how to update index.html.

raymeskhoury commented 6 years ago

Updated index.html. Note that it changed massively. I'm not sure if I did something wrong or if it is a new version of respec or something. But it changed just as much when I produced a snapshot from top-of-tree. Let me know if I'm missing something!

plehegar commented 6 years ago

(to answer David's question)

plehegar commented 6 years ago

Feature Policy is still being developed so isn't yet suitable as a normative reference in a W3C Recommendation, so we wouldn't be able to push a new Edited Recommendation with it. Once it becomes more stable, it's only adding a restriction so, from an IPR perspective, that's fine (cc @wseltzer ). As long as the change gets properly reviewed, we'd be good to go.

Malvoz commented 5 years ago

Friendly bump, other W3C recommendations such as https://www.w3.org/TR/payment-request/#feature-policy are now integrating this.

joeyparrish commented 4 years ago

Any objections to my merging this now (with an update to index.html)?

ddorwin commented 4 years ago

It looks like there is an open question of NotSupportedError vs SecurityError in the inline comments.

joeyparrish commented 4 years ago

It looks like there is an open question of NotSupportedError vs SecurityError in the inline comments.

The latest from that discussion was you saying you were fine with SecurityError and foolip saying that WPT expects it. So I assumed that was settled.

joeyparrish commented 4 years ago

JFYI, the "as defined by" part was being rejected by respec as it couldn't resolve the reference to "Feature Policy". I couldn't figure out the syntax to fix it, but I realized it was redundant, since the sentence already links to the definition in the Feature Policy spec. So with that removed and index.html regenerated, this merges cleanly.

I'll leave this open for another day to see if anybody has further comment. If not, I'll merge it. If I merge it and someone comes along later and says I missed something, we'll fix it.

Thanks!

joeyparrish commented 4 years ago

Updated link: https://github.com/w3c/webappsec-feature-policy/blob/master/integration.md#referencing-the-feature-policy-spec

Thanks! I've updated my local copy to reflect all of this, including the switch to SecurityError.

joeyparrish commented 4 years ago

Since my local diff against master is fairly small and easy to verify against the latest referencing guidelines, I'll go ahead and merge it now. I'd be happy to follow-up on any changes I missed, or review another PR if someone wants to make additional changes for this.

Thank you @raymeskhoury for your contribution!