Closed
Bug 1444073
Opened 6 years ago
Closed 6 years ago
Remove old-event-emitter usage from webaudioeditor
Categories
(DevTools Graveyard :: Web Audio Editor, enhancement)
DevTools Graveyard
Web Audio Editor
Tracking
(firefox60 fixed)
RESOLVED
FIXED
Firefox 60
| Tracking | Status | |
|---|---|---|
| firefox60 | --- | fixed |
People
(Reporter: nchevobbe, Assigned: nchevobbe)
References
Details
Attachments
(1 file)
No description provided.| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 3•6 years ago
|
||
| mozreview-review | ||
Comment on attachment 8957143 [details] Bug 1444073 - Remove old-event-emitter usage from webAudioEditor; . https://reviewboard.mozilla.org/r/226080/#review232028 ::: devtools/client/webaudioeditor/test/head.js:219 (Diff revision 2) > + win.once(win.EVENTS.UI_SET_PARAM, onParamSetSuccess); > + win.once(win.EVENTS.UI_SET_PARAM_ERROR, onParamSetError); Since you use `once` here, no need to do `off` in the handlers. Right?
Attachment #8957143 -
Flags: review?(pbrosset) → review+
|
Assignee | |
Comment 4•6 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8957143 [details] Bug 1444073 - Remove old-event-emitter usage from webAudioEditor; . https://reviewboard.mozilla.org/r/226080/#review232028 > Since you use `once` here, no need to do `off` in the handlers. Right? If UI_SET_PARAM_ERROR is emitted, the listener on UI_SET_PARAM is still up (and vice-versa). Tis way we are sure to clean things up. Or am I missing something in "once" implementation ?
|
|
||
Comment 5•6 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8957143 [details] Bug 1444073 - Remove old-event-emitter usage from webAudioEditor; . https://reviewboard.mozilla.org/r/226080/#review232028 > If UI_SET_PARAM_ERROR is emitted, the listener on UI_SET_PARAM is still up (and vice-versa). > Tis way we are sure to clean things up. > Or am I missing something in "once" implementation ? Oh I see what you're doing here. I have no idea how/when these events are fired. But you are right, if we expect one to be fired, then we need to unlisten from the other one when it happens. However here you do `off` both times on the same event.
|
|
||
Updated•6 years ago
|
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 7•6 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8957143 [details] Bug 1444073 - Remove old-event-emitter usage from webAudioEditor; . https://reviewboard.mozilla.org/r/226080/#review232028 > Oh I see what you're doing here. I have no idea how/when these events are fired. But you are right, if we expect one to be fired, then we need to unlisten from the other one when it happens. However here you do `off` both times on the same event. ah, good catch !
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a5579339a6dd Remove old-event-emitter usage from webAudioEditor; r=pbro.
|
||
Comment 9•6 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/a5579339a6dd
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
|
||
Updated•6 years ago
|
Product: Firefox → DevTools
|
|
||
Updated•5 years ago
|
Product: DevTools → DevTools Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•