Closed Bug 1413284 Opened 2 years ago Closed Last year

ConstantSourceNode.start(-1) should throw RangeError

Categories

(Core :: Web Audio, defect, P3)

57 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: toy.raymond, Assigned: maxime.langlade)

Details

(Keywords: dev-doc-complete, Whiteboard: [need info padenot 2017-11-01])

Attachments

(4 files)

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
Component: Untriaged → Web Audio
Product: Firefox → Core
Paul, again could you please asses and priorities this bug report?
Flags: needinfo?(padenot)
Whiteboard: [need info padenot 2017-11-01]
I'm going to mark this P3 because there is no clear/immediate user impact.
Priority: -- → P3
Clearing NI.

Théo, do you want to take this one? Or maybe someone else?
Flags: needinfo?(padenot) → needinfo?(theo.rabut)
I think Max will take it.
Flags: needinfo?(theo.rabut) → needinfo?(maxime.langlade)
Yes, I start working on it.
Flags: needinfo?(maxime.langlade)
Assignee: nobody → maxime.langlade
Rank: 25
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Ok, i found it. I’m not sure if range error is only for start(-1) or for every non-valid time ? doc-needed ?
Reading [0]:

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

[0]: https://webaudio.github.io/web-audio-api/#dom-audioscheduledsourcenode-start
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.
Attachment #8931860 - Flags: review?(padenot) → review-
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
Attachment #8931860 - Flags: review?(padenot) → review+
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.
Attachment #8993076 - Flags: review?(padenot) → review+
Comment on attachment 8993077 [details]
Bug 1413284 - Remove constant-source-basic.html.ini, it now passes.

https://reviewboard.mozilla.org/r/257898/#review264886
Attachment #8993077 - Flags: review?(karlt) → review+
Comment on attachment 8993319 [details]
Bug 1413284 - Remove test-constantsourcenode.html.ini, it's now passing.

https://reviewboard.mozilla.org/r/258104/#review265274
Attachment #8993319 - Flags: review?(karlt) → review+
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)
Pushed by paul@paul.cx:
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
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.
You need to log in before you can comment on or make changes to this bug.