toy.raymond Reporter
	Description • 7 years ago

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

Steps to reproduce:

Consider this (using nightly build 59.0a1)

c = new AudioContext();
n = new GainNode(c);
n.gain.setValueCurveAtTime([3,4], 1, 1);


Actual results:

This produces an error:

TypeError: Argument 1 of AudioParam.setValueCurveAtTime does not implement interface Float32Array


Expected results:

This should have worked without throwing an error.  Curiously, this works:

n.gain.setValueCurveAtTime(Float32Array.from([3,4]), 1, 1)

It seems the error message is wrong as well?  The original interface for setValueAtTime only accepted Float32Array's but was extended a while ago to accept sequence<float>

	Alex Chronopoulos [:achronop]
	Comment 1 • 7 years ago

It takes some time for us to become spec compliant. It seems that the old interface has not been updated.

	toy.raymond Reporter
	Comment 2 • 7 years ago

No worries.  Just wanted to report the issue so someone might be able to fix it someday.

	Paul Adenot (:padenot) Assignee
	Comment 5 • 6 years ago

Baku, this is tested in web-platform-tests, but https://bugzilla.mozilla.org/show_bug.cgi?id=1472095#c4 needs to be addressed so that the behaviour is really tested properly.

	Andrea Marchesini [:baku]
	Comment 6 • 6 years ago
mozreview-review

Comment on attachment 8989458 [details]
Bug 1421091 - Update wpt expectations.

https://reviewboard.mozilla.org/r/254512/#review261334

	Andrea Marchesini [:baku]
	Comment 7 • 6 years ago

we should probably update some of our dom/media/tests.

	Boris Zbarsky [:bzbarsky]
	Comment 8 • 6 years ago
mozreview-review

Comment on attachment 8989458 [details]
Bug 1421091 - Update wpt expectations.

https://reviewboard.mozilla.org/r/254512/#review261348

Doesn't this need c++-side changed too?  I would expect this to not compile...

	Paul Adenot (:padenot) Assignee
	Comment 9 • 6 years ago

Yeah ok so mercurial hanged when I pushed the actual commit with all the changes, I found:

> remote: Connection to reviewboard-hg.mozilla.org closed by remote host.

at the bottom of my terminal this morning.

	Karl Tomlinson (:karlt)
	Comment 11 • 6 years ago
mozreview-review

Comment on attachment 8989458 [details]
Bug 1421091 - Update wpt expectations.

https://reviewboard.mozilla.org/r/254512/#review261700

c++ changes look good, thanks.

	Karl Tomlinson (:karlt)
	Comment 14 • 6 years ago
mozreview-review

Comment on attachment 8991603 [details]
Bug 1421091 - Fix duplicate name error.

https://reviewboard.mozilla.org/r/256532/#review263606

::: commit-message-2bc14:1
(Diff revision 1)
> +Bug 1421091 - Fix duplicate name error. r?karlt
> +
> +This makes WPT error out, because the if no name is passed in, it's synthetized
> +from the result, and the result is the same in those cases. It's unclear to me
> +why it wasn't an error before, but it's erroring out now and the fix is simple.

I assume you meant to push this to bug 1472095 because
https://reviewboard.mozilla.org/r/254224/diff/2 is the reason why this is
required.

Please either label with that bug number, so that "Fix duplicate name error"
makes sense, or perhaps better change the commit message to something like
"test exception type thrown by createDelay() and provide subtest names", or
both.

These changes to give the test an explicit name are good, thanks, regardless
of the issue that caused the duplicate names.  i.e. this fixes part of
https://github.com/web-platform-tests/wpt/issues/10201

I recommend this in .hgrc, so that I don't overwrite the wrong changesets:

[reviewboard]
autopublish = False

::: testing/web-platform/tests/webaudio/the-audio-api/the-delaynode-interface/delaynode-maxdelaylimit.html:42
(Diff revision 1)
> -            should(() => context.createDelay(-1)).throw();
> -            should(() => context.createDelay(NaN)).throw();
> -            ;
> +            should(() => context.createDelay(0)).throw("NotSupportedError",
> +              "Delay length cannot be 0");
> +            should(() => context.createDelay(-1)).throw("NotSupportedError",
> +              "Delay length cannot be negative");
> +            should(() => context.createDelay(NaN)).throw(TypeError,
> +              "Delay lenght cannot be a NaN");

s/lenght/length/

	Paul Adenot (:padenot) Assignee
	Comment 15 • 6 years ago

Right, that's what I missed, thanks.

	Pulsebot
	Comment 16 • 6 years ago

Pushed by paul@paul.cx:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d34c85e0ec1
Update AudioParam.setValueCurveAtTime to take sequence<float> instead of a Float32Array. r=baku,karlt
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c22cb391d72
Update wpt expectations. r=karlt

	Pulsebot
	Comment 17 • 6 years ago

Pushed by paul@paul.cx:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d0cce417e56
Remove a bit from a gtest that is tested directly in the bindings, on a CLOSED TREE.

	Cosmin Sabou [:CosminS]
	Comment 18 • 6 years ago

Backed out for gtest failures on AudioEventTimeline.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&fromchange=1f89fc574e6ccffbdf96ae74e1151c2a8442c7c3&tochange=9d4c8ab1fa61392f2c7449ed0cc5cafa091bd1cc&filter-searchStr=gtest&selectedJob=188035489

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=188035489&repo=mozilla-inbound&lineNumber=22821

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/9d4c8ab1fa61392f2c7449ed0cc5cafa091bd1cc

Please also take a look at these wpt failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&fromchange=1f89fc574e6ccffbdf96ae74e1151c2a8442c7c3&tochange=9d4c8ab1fa61392f2c7449ed0cc5cafa091bd1cc&filter-searchStr=wpt&selectedJob=188035510

https://treeherder.mozilla.org/logviewer.html#?job_id=188035510&repo=mozilla-inbound&lineNumber=8231

Notice that your latest push to fix the above failure had build bustages: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=5d0cce417e56935badf1e7a503f348079c8a9435&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&selectedJob=188037994

https://treeherder.mozilla.org/logviewer.html#?job_id=188037994&repo=mozilla-inbound&lineNumber=20124

	Andrea Marchesini [:baku]
	Comment 23 • 6 years ago
mozreview-review

Comment on attachment 8991899 [details]
Bug 1421091 - Update AudioParam.setValueCurveAtTime to take sequence<float> instead of a Float32Array.

https://reviewboard.mozilla.org/r/256816/#review263664

	Jean-Yves Avenard [:jya]
	Comment 24 • 6 years ago
mozreview-review

Comment on attachment 8991900 [details]
Bug 1421091 - Remove a bit from a gtest that is tested directly in the bindings.

https://reviewboard.mozilla.org/r/256818/#review263666

	Pulsebot
	Comment 25 • 6 years ago

Pushed by paul@paul.cx:
https://hg.mozilla.org/integration/autoland/rev/37d172c987e6
Update AudioParam.setValueCurveAtTime to take sequence<float> instead of a Float32Array. r=baku
https://hg.mozilla.org/integration/autoland/rev/65ca98d405c8
Update wpt expectations. r=baku,karlt
https://hg.mozilla.org/integration/autoland/rev/e1b695949d28
Remove a bit from a gtest that is tested directly in the bindings.  r=jya

	Karl Tomlinson (:karlt)
	Comment 26 • 6 years ago
mozreview-review

Comment on attachment 8991899 [details]
Bug 1421091 - Update AudioParam.setValueCurveAtTime to take sequence<float> instead of a Float32Array.

https://reviewboard.mozilla.org/r/256816/#review263808

	Noemi Erli[:noemi_erli]
	Comment 27 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/37d172c987e6
https://hg.mozilla.org/mozilla-central/rev/65ca98d405c8
https://hg.mozilla.org/mozilla-central/rev/e1b695949d28

	Eric Shepherd [:sheppy]
	Comment 28 • 6 years ago

Documentation updated:

https://developer.mozilla.org/en-US/docs/Web/API/AudioParam
https://developer.mozilla.org/en-US/docs/Web/API/AudioParam/setValueCurveAtTime

Also updated:

https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/63