Closed
Bug 1443224
Opened 6 years ago
Closed 6 years ago
ChannelMerger should throw errors on invalid values of count and mode
Categories
(Core :: Web Audio, defect, P2)
Core
Web Audio
Tracking
()
RESOLVED
FIXED
mozilla61
People
(Reporter: toy.raymond, Assigned: padenot)
References
Details
Attachments
(2 files, 1 obsolete file)
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.106 Safari/537.36 Steps to reproduce: Run this in the console: c = new AudioContext() n = new ChannelMergerNode(c) n.channelCount = 3 n.channelCountMode = "max" Actual results: The channelCount is set to 3 and channelCountMode is set to "max". Expected results: In both cases, an error MUST be thrown because these attributes are fixed. See https://webaudio.github.io/web-audio-api/#dfn-channelcount-constraints and https://webaudio.github.io/web-audio-api/#dfn-channelcountmode-constraints
Comment 1•6 years ago
|
||
Tested on Mac OS X 10.12 with FF Nightly 60.0a1(2018-03-07) and FF 58 and I can reproduce it.
Status: UNCONFIRMED → NEW
Has STR: --- → yes
status-firefox58:
--- → affected
status-firefox59:
--- → affected
status-firefox60:
--- → affected
Component: Untriaged → Web Audio
Ever confirmed: true
Product: Firefox → Core
Version: 57 Branch → Trunk
Comment hidden (mozreview-request) |
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8957588 [details] Bug 1443224 - Throw correct exceptions when trying to set the channelCount, the channelCountMode and the channelInterpretation on a ChannelSplitterNode. https://reviewboard.mozilla.org/r/226440/#review232404 ::: dom/media/webaudio/AudioNode.cpp:94 (Diff revision 1) > return; > } > } > > if (aOptions.mChannelInterpretation.WasPassed()) { > - SetChannelInterpretationValue(aOptions.mChannelInterpretation.Value()); > + SetChannelInterpretationValue(aOptions.mChannelInterpretation.Value(), aRv); Just in case we add something more in this method, what about if we add here: if (NS_WARN_IF(aRv.Failed())) { return; } ::: dom/media/webaudio/ChannelSplitterNode.h:34 (Diff revision 1) > const ChannelSplitterOptions& aOptions, ErrorResult& aRv) > { > return Create(aAudioContext, aOptions, aRv); > } > > + virtual void SetChannelCount(uint32_t aChannelCount, ErrorResult& aRv) This method is final. No virtual here.
Attachment #8957588 -
Flags: review?(amarchesini) → review+
Updated•6 years ago
|
Assignee: nobody → padenot
Rank: 15
Priority: -- → P2
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8958045 -
Attachment is obsolete: true
Assignee | ||
Comment 6•6 years ago
|
||
mozreview-review |
Comment on attachment 8957588 [details] Bug 1443224 - Throw correct exceptions when trying to set the channelCount, the channelCountMode and the channelInterpretation on a ChannelSplitterNode. https://reviewboard.mozilla.org/r/226440/#review232712
Comment hidden (mozreview-request) |
Pushed by paul@paul.cx: https://hg.mozilla.org/integration/autoland/rev/7386deb98f9a Throw correct exceptions when trying to set the channelCount, the channelCountMode and the channelInterpretation on a ChannelSplitterNode. r=baku
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•6 years ago
|
||
mozreview-review |
Comment on attachment 8958050 [details] Bug 1443224 - Fix bustage by adding override on the definition. https://reviewboard.mozilla.org/r/226988/#review232722
Attachment #8958050 -
Flags: review+
Comment 11•6 years ago
|
||
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
Comment 12•6 years ago
|
||
Backed out for build bustages on dom/ChannelSplitterNode.h Treeherder push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7386deb98f9a909d8c6f4dea701e351bbd266e94&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&selectedJob=167363685 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=167363685&repo=autoland&lineNumber=16323 Backout link: https://hg.mozilla.org/integration/autoland/rev/fe6bc4234078855432012d333b7e2ccd9a31abd8
Flags: needinfo?(padenot)
Comment 13•6 years ago
|
||
Pushed by paul@paul.cx: https://hg.mozilla.org/integration/mozilla-inbound/rev/6d9ebda1ad07 Throw correct exceptions when trying to set the channelCount, the channelCountMode and the channelInterpretation on a ChannelSplitterNode. r=baku
Comment 14•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6d9ebda1ad07
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Updated•6 years ago
|
Updated•6 years ago
|
Comment 15•6 years ago
|
||
I tested this on Mac OS X 10.12 with FF Nightly 61.0a1(2018-04-25) and the issue is still reproducible.
You need to log in
before you can comment on or make changes to this bug.
Description
•