1382780 - Throw NotSupportedError DomException when playbackRate is unsupported.

Status: RESOLVED FIXED

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

Description

[Jennifer Apacible]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3162.0 Safari/537.36




Expected results:

Change to the standard:
https://github.com/whatwg/html/pull/2829

Rendered: search for "the playbackRate attribute gives" in https://html.spec.whatwg.org/multipage/media.html#playing-the-media-resource

Change to tests: https://github.com/w3c/web-platform-tests/pull/6522

Note: this is a change that all browsers need to make.

Comment 1

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Blake - this should be a small job. Can you find someone to do?

Comment 2

[Blake Wu [:bwu][:blakewu]]

Alastor, 
It should be a piece of cake to you. Interested?

Comment 3

[Alastor Wu [:alwu]]

Attachment: Bug 1382780 - part1 : return NS_ERROR_DOM_NOT_SUPPORTED_ERR for the negative playback rate.

According to [1], we should return NotSupportedError for the negative playback rate.

[1] https://github.com/w3c/web-platform-tests/pull/6522

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

Comment 4

[Alastor Wu [:alwu]]

Attachment: Bug 1382780 - part2 : separate the value of actual playback rate and media element's attribute.

According to [1], we should separate the value of actual playback rate and media elemenet's attribute.
But still keep a MAX and MIN threshold to control the highest and lowser playback rate for decoder.

[1] http://w3c-test.org/html/semantics/embedded-content/media-elements/playing-the-media-resource/playbackRate.html

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

Comment 5

[Alastor Wu [:alwu]]

Attachment: Bug 1382780 - part3 : remove the corresponding wpt .ini file.

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

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8900600 [details]
Bug 1382780 - part1 : return NS_ERROR_DOM_NOT_SUPPORTED_ERR for the negative playback rate.

https://reviewboard.mozilla.org/r/171998/#review177334

Comment 7

[Alastor Wu [:alwu]]

Attachment: Bug 1382780 - part4 : modify tests.

* test_playback_rate.html
In the patch2, we would honestly return the value of playback rate.

* test_info_leak.html/test_load.html

According to [1], we should only dispatch event "ratechange" when playbackRate or
defaultPlaybackRate attribute has just been updated. In these tests, they didn't
change the playbackRate or defaultPlaybackRate, we should not expect for the
"ratechange" event.

[1] https://html.spec.whatwg.org/multipage/media.html#event-media-ratechange

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

Comment 8

[Alastor Wu [:alwu]]

Comment on attachment 8900600 [details]
Bug 1382780 - part1 : return NS_ERROR_DOM_NOT_SUPPORTED_ERR for the negative playback rate.

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

Comment 9

[Alastor Wu [:alwu]]

Comment on attachment 8900601 [details]
Bug 1382780 - part2 : separate the value of actual playback rate and media element's attribute.

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

Comment 10

[Alastor Wu [:alwu]]

Comment on attachment 8900602 [details]
Bug 1382780 - part3 : remove the corresponding wpt .ini file.

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

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8900601 [details]
Bug 1382780 - part2 : separate the value of actual playback rate and media element's attribute.

https://reviewboard.mozilla.org/r/172000/#review177336

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8900602 [details]
Bug 1382780 - part3 : remove the corresponding wpt .ini file.

https://reviewboard.mozilla.org/r/172002/#review177338

Comment 13

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8900639 [details]
Bug 1382780 - part4 : modify tests.

https://reviewboard.mozilla.org/r/172044/#review177346

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12895c62f332
part1 : return NS_ERROR_DOM_NOT_SUPPORTED_ERR for the negative playback rate. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/7f230fbf1a5e
part2 : separate the value of actual playback rate and media element's attribute. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/1a458c014157
part3 : remove the corresponding wpt .ini file. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/7985d03d93db
part4 : modify tests. r=cpearce

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/12895c62f332
https://hg.mozilla.org/mozilla-central/rev/7f230fbf1a5e
https://hg.mozilla.org/mozilla-central/rev/1a458c014157
https://hg.mozilla.org/mozilla-central/rev/7985d03d93db