ContentSearch sometimes leaks event listeners and their closures

RESOLVED FIXED in Firefox 33

Status

()

Firefox
Search
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: adw, Assigned: adw)

Tracking

Trunk
Firefox 33
Points:
1
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Created attachment 8445370 [details] [diff] [review]
patch

Whoops.  msg is added as an event listener to target A, msg receives a SwapDocShells event and changes its target to B, msg is done processing and is incorrectly removed as an event listener from target B.
Attachment #8445370 - Flags: review?(ttaubert)
Comment on attachment 8445370 [details] [diff] [review]
patch

Review of attachment 8445370 [details] [diff] [review]:
-----------------------------------------------------------------

Oh :| Good catch!
Attachment #8445370 - Flags: review?(ttaubert) → review+
https://hg.mozilla.org/integration/fx-team/rev/7d9d619896bf
Iteration: --- → 33.2
Points: --- → 1
QA Whiteboard: [qa-]
Flags: firefox-backlog+
It just occurred to me that we should probably rather remove the event listener in handleEvent() and add a new one to the new target. If the docShell is swapped twice before the event is processed we might still fail. We wouldn't need .originalTarget that way either.
Created attachment 8445390 [details] [diff] [review]
follow-up patch

You're right.
Attachment #8445390 - Flags: review?(ttaubert)
Comment on attachment 8445390 [details] [diff] [review]
follow-up patch

Review of attachment 8445390 [details] [diff] [review]:
-----------------------------------------------------------------

Cool, thanks.
Attachment #8445390 - Flags: review?(ttaubert) → review+
https://hg.mozilla.org/mozilla-central/rev/7d9d619896bf
https://hg.mozilla.org/mozilla-central/rev/348cffe90344
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
You need to log in before you can comment on or make changes to this bug.