1291997 - Add new DOM exceptions for implementing promise-based HTMLMediaElement::play() API

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Tzuhao Kuo [:kaku]]

In bug 1244768, we are going to implement the promise-based HTMLMediaElement::play() API which requires a new DOM exception, NotAllowedError.

https://html.spec.whatwg.org/multipage/embedded-content.html#dom-media-play, this is where the NotAllowedError is used in the spec.

Comment 1

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1291997 - add media related dom exceptions;

Review commit: https://reviewboard.mozilla.org/r/69182/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69182/

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8777658 [details]
Bug 1291997 - add media related dom exceptions;

https://reviewboard.mozilla.org/r/69182/#review66474

::: xpcom/base/ErrorList.h:526
(Diff revision 1)
>    /* WebCrypto API errors from http://www.w3.org/TR/WebCryptoAPI/ */
>    ERROR(NS_ERROR_DOM_UNKNOWN_ERR,                  FAILURE(30)),
>    ERROR(NS_ERROR_DOM_DATA_ERR,                     FAILURE(31)),
>    ERROR(NS_ERROR_DOM_OPERATION_ERR,                FAILURE(32)),
> +  /* HTMLMediaElement API errors from https://html.spec.whatwg.org/multipage/embedded-content.html#media-elements */
> +  ERROR(NS_ERROR_DOM_NOT_ALLOWED_ERR,              FAILURE(33)),

You need to map this to NotAllowedError in domerr.msg, right?  Without that, it won't do reasonable things if it makes its way out to content.

Note that depending on your use cases, you may want a less-generic nsresult name (e.g. see NS_ERROR_DOM_PUSH_DENIED) that maps to NotAllowedError.

Comment 3

[Tzuhao Kuo [:kaku]]

https://reviewboard.mozilla.org/r/69182/#review66474

> You need to map this to NotAllowedError in domerr.msg, right?  Without that, it won't do reasonable things if it makes its way out to content.
> 
> Note that depending on your use cases, you may want a less-generic nsresult name (e.g. see NS_ERROR_DOM_PUSH_DENIED) that maps to NotAllowedError.

Should I open a new module in the ErrorList.h? Maybe call it NS_ERROR_MODULE_MEDIA?

Comment 4

[Boris Zbarsky [:bzbarsky]]

I don't have a strong feeling about that.

Comment 5

[Tzuhao Kuo [:kaku]]

According the discussion here: https://github.com/whatwg/html/issues/505, the community is going to replace the MediaErrors with DOMExceptions in long tern. So, I will then create a new NS_ERROR_MODULE_MEDIA module and put the related DOM exceptions in it with the NS_ERROR_DOM_MEDIA_ prefix.

Comment 6

[Tzuhao Kuo [:kaku]]

Comment on attachment 8777658 [details]
Bug 1291997 - add media related dom exceptions;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69182/diff/1-2/

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8777658 [details]
Bug 1291997 - add media related dom exceptions;

https://reviewboard.mozilla.org/r/69182/#review67296

Comment 8

[Tzuhao Kuo [:kaku]]

Thanks for the review!
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=15f515e1dc42
Building on all platforms are ok. 
There are no logic changes in this patch, so we don't need to care tests so much.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7d6101e0937
add media related dom exceptions; r=bz

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a7d6101e0937