1246842 - once function removes incorrect event listener

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

While looking through the dom/media/test/manifest.js file I think I may have found an issue with the once function (https://hg.mozilla.org/mozilla-central/file/tip/dom/media/test/manifest.js#l1358).

It appears the function adds an event listener and then removes a different event listener. I believe the event listener removed should be the same one as added.

Comment 1

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: MozReview Request: Bug 1246842 - Correct once function to remove correct event listener r?cpearce

The once function should add and then remove an event listener after it is
invoked, but it appears to remove another event listener instead. This commit
adjusts that behaviour. Note that the dom/media/mediasource/test/mediasource.js
file has the same pattern and has been updated. This commit reflects using the
same changes to remedy the once function here.

Review commit: https://reviewboard.mozilla.org/r/34123/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/34123/

Comment 2

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=378e19db3734

Comment 3

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b8e8551c319b

Comment 4

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Been thinking about this, and I'm trying a version that uses named functions to avoid using arguments.callee as it seems to be a no no as of ES5.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8717308 [details]
MozReview Request: Bug 1246842 - Correct once function to remove correct event listener r?cpearce

https://reviewboard.mozilla.org/r/34123/#review30833

Comment 6

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: MozReview Request: Bug 1246842 - Rework previous commit to not used deprecated arguments.callee r?cpearce

Review commit: https://reviewboard.mozilla.org/r/34161/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/34161/

Comment 7

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Comment on attachment 8717366 [details]
MozReview Request: Bug 1246842 - Rework previous commit to not used deprecated arguments.callee r?cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/34161/diff/1-2/

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8717366 [details]
MozReview Request: Bug 1246842 - Rework previous commit to not used deprecated arguments.callee r?cpearce

https://reviewboard.mozilla.org/r/34161/#review30897

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/01d335a5c88d
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a38b481c111

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/01d335a5c88d
https://hg.mozilla.org/mozilla-central/rev/4a38b481c111