1413284 - ConstantSourceNode.start(-1) should throw RangeError

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P3)

Description

[toy.raymond]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.18 Safari/537.36

Steps to reproduce:

Create a ConstantSourceNode and call start(-1)


Actual results:

A NotSupportedError is thrown.


Expected results:

A RangeError should be thrown.

See https://webaudio.github.io/web-audio-api/#dom-audioscheduledsourcenode-start

Comment 1

[Nils Ohlmeier [:drno]]

Paul, again could you please asses and priorities this bug report?

Comment 2

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

I'm going to mark this P3 because there is no clear/immediate user impact.

Comment 3

[Paul Adenot (:padenot)]

Clearing NI.

Théo, do you want to take this one? Or maybe someone else?

Comment 4

[theo.rabut]

I think Max will take it.

Comment 5

[maxime.langlade]

Yes, I start working on it.

Comment 6

[maxime.langlade]

Ok, i found it. I’m not sure if range error is only for start(-1) or for every non-valid time ? doc-needed ?

Comment 7

[Paul Adenot (:padenot)]

Reading [0]:

> A RangeError exception MUST be thrown if when is negative.

[0]: https://webaudio.github.io/web-audio-api/#dom-audioscheduledsourcenode-start

Comment 8

[maxime.langlade]

Attachment: Bug 1413284 - A RangeError exception is thrown if start/stop is called with a out of range param

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

Comment 9

[Paul Adenot (:padenot)]

Comment on attachment 8931860 [details]
Bug 1413284 - A RangeError exception is thrown if start/stop is called with a out of range param

https://reviewboard.mozilla.org/r/202962/#review208860

::: dom/media/webaudio/ConstantSourceNode.cpp:217
(Diff revision 1)
>    if (!WebAudioUtils::IsTimeValid(aWhen)) {
> +    if (aWhen < 0) {
> +      aRv.Throw(NS_ERROR_RANGE_ERR);
> +    } else {
> -    aRv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
> +      aRv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
> +    }

Please always throw a range error. We do have an upper limit, but it's also a range error, so it makes more sense.

Comment 10

[maxime.langlade]

Comment on attachment 8931860 [details]
Bug 1413284 - A RangeError exception is thrown if start/stop is called with a out of range param

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

Comment 11

[Paul Adenot (:padenot)]

Comment on attachment 8931860 [details]
Bug 1413284 - A RangeError exception is thrown if start/stop is called with a out of range param

https://reviewboard.mozilla.org/r/202962/#review211494

Comment 12

[Paul Adenot (:padenot)]

Attachment: Bug 1413284 - A RangeError exception is thrown if start/stop is called with a out of range param

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

Comment 13

[Paul Adenot (:padenot)]

Attachment: Bug 1413284 - Remove constant-source-basic.html.ini, it now passes.

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

Comment 14

[Paul Adenot (:padenot)]

Comment on attachment 8993076 [details]
Bug 1413284 - A RangeError exception is thrown if start/stop is called with a out of range param

https://reviewboard.mozilla.org/r/257896/#review264822

This is Maxime's patch, I've removed the mochitest because wpt covers this. I had r+ed this earlier.

Comment 15

[Karl Tomlinson (:karlt)]

Comment on attachment 8993077 [details]
Bug 1413284 - Remove constant-source-basic.html.ini, it now passes.

https://reviewboard.mozilla.org/r/257898/#review264886

Comment 16

[Paul Adenot (:padenot)]

Attachment: Bug 1413284 - Remove test-constantsourcenode.html.ini, it's now passing.

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

Comment 17

[Paul Adenot (:padenot)]

Comment on attachment 8993076 [details]
Bug 1413284 - A RangeError exception is thrown if start/stop is called with a out of range param

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

Comment 18

[Paul Adenot (:padenot)]

Comment on attachment 8993077 [details]
Bug 1413284 - Remove constant-source-basic.html.ini, it now passes.

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

Comment 19

[Karl Tomlinson (:karlt)]

Comment on attachment 8993319 [details]
Bug 1413284 - Remove test-constantsourcenode.html.ini, it's now passing.

https://reviewboard.mozilla.org/r/258104/#review265274

Comment 20

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 6bd898b2d57a1b62095f8d34f3332dffc19b943e -d 1ae4395e7cf7: rebasing 473660:6bd898b2d57a "Bug 1413284 - A RangeError exception is thrown if start/stop is called with a out of range param r=padenot"
rebasing 473661:2429d18486ef "Bug 1413284 - Remove constant-source-basic.html.ini, it now passes. r=karlt"
local [dest] changed testing/web-platform/meta/webaudio/the-audio-api/the-constantsourcenode-interface/constant-source-basic.html.ini which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 21

[Paul Adenot (:padenot)]

Comment on attachment 8993076 [details]
Bug 1413284 - A RangeError exception is thrown if start/stop is called with a out of range param

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

Comment 22

[Paul Adenot (:padenot)]

Comment on attachment 8993077 [details]
Bug 1413284 - Remove constant-source-basic.html.ini, it now passes.

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

Comment 23

[Paul Adenot (:padenot)]

Comment on attachment 8993319 [details]
Bug 1413284 - Remove test-constantsourcenode.html.ini, it's now passing.

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

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a6d4d1b11fd
A RangeError exception is thrown if start/stop is called with a out of range param r=padenot
https://hg.mozilla.org/integration/autoland/rev/40195349a292
Remove constant-source-basic.html.ini, it now passes. r=karlt
https://hg.mozilla.org/integration/autoland/rev/666dc3b74320
Remove test-constantsourcenode.html.ini, it's now passing. r=karlt

Comment 25

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/4a6d4d1b11fd
https://hg.mozilla.org/mozilla-central/rev/40195349a292
https://hg.mozilla.org/mozilla-central/rev/666dc3b74320

Comment 26

[Eric Shepherd [:sheppy]]

Documentation updated:

https://developer.mozilla.org/en-US/docs/Web/API/AudioScheduledSourceNode/start — Rewrote the Exceptions list to be complete and up to date, incorporating this fix among others. Added an example with a description. Updated some formatting.

https://developer.mozilla.org/en-US/docs/Web/API/AudioScheduledSourceNode/stop — Updated the Exceptions list. Revised the example a bit. Added example description. Fixed some formatting.

Also added a mention to Firefox 63 for developers.