Closed Bug 1651703 Opened 4 years ago Closed 4 years ago

Update AudioParamDescriptor webidl min and max values

Categories

(Core :: Web Audio, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox80 --- wontfix
firefox81 --- fixed

People

(Reporter: arnaud.bienner, Assigned: arnaud.bienner)

References

()

Details

Attachments

(1 file)

Follow-up of bug 1501709. This is about fixing the FIXME comment in AudioParamDescriptor.webidl. Now MSVC support has been dropped (bug 1512504), we can remove this workaround.

Assignee: nobody → arnaud.bienner
Status: NEW → ASSIGNED

Karl: I'm not sure which try jobs I should start. It looks like selecting jobs to run on try server is more complicated now than it used to be, and unfortunately "auto" (which looked promising) doesn't work.
Also, I'm not sure how to request this to be landed: I can't find the "checkin-needed" keyword, and it looks like I don't have access to Lando.

Flags: needinfo?(karlt)

This is the changeset that I add when ./mach try auto fails:
https://hg.mozilla.org/try/rev/096f332daa2c77188b9390482817412d9b8f9488

For landing, I think the process is
"Edit Revision" -> "Tags" -> type "Check-in Needed".

Flags: needinfo?(karlt)

Thanks Karl!
I missed the fact you can still use the try syntax.

I pushed to try, building on Linux macOS and Windows (since there was a compiler bug, I want to be sure all platforms behave correctly now, though I think all of them are using clang now): https://treeherder.mozilla.org/#/jobs?repo=try&revision=eba1ebbeed05cb58ff4351fb00913aad5a5d92d7
Almost green. I don't think the issue I see are related to this change, so I will request checkin.

Arnaud, land failed with:
"Details: hg error in cmd: hg push -r tip upstream: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote:
remote: ******************************* ERROR *******************************
remote: Changeset 775a97906327 alters WebIDL file(s) without DOM peer review:
remote: dom/webidl/AudioParamDescriptor.webidl
remote:
remote: Please, request review from the #webidl reviewer group or either:
remote: - Andrea Marchesini (:baku)
remote: - Andreas Farre (:farre)
remote: - Andrew McCreight (:mccr8)
remote: - Bobby Holley (:bholley)
remote: - Ehsan Akhgari (:ehsan)
remote: - Emilio Cobos (:emilio)
remote: - Henri Sivonen (:hsivonen)
remote: - Nika Layzell (:mystor)
remote: - Olli Pettay (:smaug)
remote: - Peter Van der Beken (:peterv)
remote: *********************************************************************
remote:
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.mozhooks hook failed
abort: push failed on remote"

Flags: needinfo?(arnaud.bienner)

I've asked Andrea to review the change, since he reviewed the original file when it was created.

Flags: needinfo?(arnaud.bienner)

Andrea, could you please review (and approve :)) this WebIDL change?
Looks like I need DOM peer review for this to be landed, and you reviewed this file when I created it two years ago (bug 1501709 https://hg.mozilla.org/mozilla-central/rev/fc6fa6ebad86)
This change is about updating the WebIDL file so it matches the spec.
We couldn't use the values used in the spec at that time because of a compiler error with MSVC, which is not supported anymore (more details above and on bug 1501709).

Flags: needinfo?(amarchesini)

Hi Arnaud. Sorry for the delay, I haven't seen your patch in my review queue. R+

Flags: needinfo?(amarchesini)

No worries :) Thanks for the review!

Pushed by ncsoregi@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5c5cd63fa29c fix min and max values in AudioParamDescriptor now we don't need this workaround to support MSVC. r=karlt,baku
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: