Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

encryptionScheme should be a DOMString #8

Closed
ddorwin opened this issue Nov 27, 2018 · 11 comments
Closed

encryptionScheme should be a DOMString #8

ddorwin opened this issue Nov 27, 2018 · 11 comments
Assignees

Comments

@ddorwin
Copy link

ddorwin commented Nov 27, 2018

Enum types within MediaKeySystemConfiguration cannot be safely extended because an unrecognized value will cause a TypeError exception to be thrown by requestMediaKeySystemAccess() rather than ignoring the unrecognized value and continuing to another capability. Thus, encryptionScheme should be a DOMString.

sessionTypes is aso DOMString for this reason. See w3c/encrypted-media#447 for how this can be handled.

@ddorwin
Copy link
Author

ddorwin commented Nov 27, 2018

The registry approach to addressing #9 may also require this value to be a DOMString.

SingingTree added a commit to SingingTree/encrypted-media-encryption-scheme that referenced this issue Mar 18, 2019
Using a DOMString instead of an enum for the encryptionScheme member
allows for safer extension of encryption schemes. Without using a
DOMString, invalid enum values will result in a TypeError.

An example of why this is problematic: if a new encryption scheme, fooo,
is added, and an application checks for support using the API, an older
User Agent may throw a TypeError as fooo is not a valid enum value.
Using a DOMString avoids this, and User Agents can ignore configurations
with unrecognized schemes. This allows the call to
requestMediaKeySystemAccess() to succeed, provided other configurations
with supported schemes are provided.
SingingTree added a commit to SingingTree/encrypted-media-encryption-scheme that referenced this issue Mar 18, 2019
Using a DOMString instead of an enum for the encryptionScheme member
allows for safer extension of encryption schemes. Without using a
DOMString, invalid enum values will result in a TypeError.

An example of why this is problematic: if a new encryption scheme, fooo,
is added, and an application checks for support using the API, an older
User Agent may throw a TypeError as fooo is not a valid enum value.
Using a DOMString avoids this, and User Agents can treat configurations
with unrecognized schemes as unsupported. This allows the call to
requestMediaKeySystemAccess() to succeed, provided other configurations
with supported schemes are provided.
@mounirlamouri
Copy link

I think encryptionScheme should stay an enum. The concerns of throwing a TypeError would actually help feature detection which is a small price to pay. It's actually the best way to also resolve #11 as websites could feature detect the feature before asking for a given scheme.

@chcunningham
Copy link

chcunningham commented Jun 6, 2019

The registry approach to addressing #9 may also require this value to be a DOMString.

@mounirlamouri, this may still block an enum. do you have a workaround?

@mounirlamouri
Copy link

Registries are a workaround to avoid updating a specification when a list is updated. In this case, if we expect to get regular encryptionScheme updates, it would make sense to use a registry but it doesn't seem likely AFAICT.

@joeyparrish
Copy link
Member

It's also worth note that having even two encryption schemes in common use, but not universally supported by UAs, is in some sense a partial failure of the goal of interoperability. While I don't think an enum should be the primary means of discouraging encryption scheme proliferation, it's not such a terrible thing if adding new schemes is challenging.

So I'm okay with an enum with two values, btoh from the point of view of an app author and from the point of view of Widevine.

@joeyparrish
Copy link
Member

If there are no further objections, I move we close this issue and PR #12.

@joeyparrish
Copy link
Member

Actually, it seems that this is dependent on issue #9. If a registry is called for by issue #9, then DOMString may be a technical requirement for that. So I spoke too soon on enum.

@SingingTree
Copy link
Contributor

I think encryptionScheme should stay an enum. The concerns of throwing a TypeError would actually help feature detection which is a small price to pay. It's actually the best way to also resolve #11 as websites could feature detect the feature before asking for a given scheme.

Shouldn't sites be able to feature detect based on the presence of the encryptionScheme member on the configuration object(s) they get back? I.e. if this is implemented that member will be present on the returned configs, otherwise it will not. I've made a test page to play with this and it appears to be the case.

This would allow sites that are aware of this feature to check for the presence and value of that member on any resolved access and act accordingly. I think this would also allow them to mitigate #11, unless I'm misreading that issue.

Provided my working above is correct, I prefer the usage of a DOMString for extensibility. It appears this would be useful for the approach being discussed in #9 also?

@joeyparrish
Copy link
Member

Yes, checking for the member on the config objects was my intent for polyfilling and feature detection.

@joeyparrish
Copy link
Member

After further consideration, I believe a DOMString would be best. The main reason for this is that an enum requires an additional error-handling paradigm from applications.

For example, if an app requests a bogus or unsupported codec string, the result is a rejected Promise. But if an app specifies an unsupported enum value (even if it exists in other browsers or in the spec), the result is an exception at the call site.

If we had designed EME with enums from the beginning, the addition of another would not create any new pain for applications, which would already have calls wrapped in try/catch. But requiring try/catch through the use of enum could break existing application logic and make this new feature more painful for developers.

Therefore I'm going to merge PR #12 and settle on DOMString instead of enum. We can still provide guidance in the spec on well-known values and their meaning.

@mounirlamouri
Copy link

SGTM 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants