Closed Bug 545268 Opened 15 years ago Closed 15 years ago

Event suppression doesn't work with child widgets removed

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: roc, Assigned: roc)

References

Details

Attachments

(1 file)

With the patches in bug 130078 I get test failures in bug 493251: 31838 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug493251.html | Wrong number events (19) - got 2, expected 1 31841 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug493251.html | Wrong number events (19) - got 3, expected 2 31842 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug493251.html | Wrong number events (20) - got 3, expected 2 31843 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug493251.html | Wrong number events (21) - got 3, expected 2 I think the problem is that the code in PresShell::HandleEvent looks at mDocument->EventHandlingSuppressed(). For mouse events, when there are no child widgets, mDocument is always the toplevel chrome document. Basically, this code doesn't handle situations where a subframe has events suppressed but the outer frame doesn't. (For key events I don't think this is a problem because key events are always dispatched to the presshell containing the focused element.)
I see that nsXMLHttpRequest blocks events for the "top" window. But that's only the topmost content window of course. I guess it makes sense to leave that there and have PresShell check EventHandlingSuppressed for all ancestor documents of the actual event target.
I see that nsDocument::(Un)SuppressEventHandling iterates through its subdocuments to suppress/unsupress them, so in theory we don't need to check ancestors. But that seems like it would break if there are DOM changes or frame navigations while events are suppressed. Right?
Attached patch fix (deleted) — Splinter Review
Anyway, this fixes the mouse bug and adds a test to confirm that suppressing in a parent document also suppresses the child. I think we could make the test fail by navigating the subframe in the right way.
Assignee: nobody → roc
Attachment #426138 - Flags: review?(Olli.Pettay)
I suggest not iterating through all descendants, and instead having nsIDocument::EventHandlingSuppressed check all ancestor documents. But I don't want to do that here.
(In reply to comment #2) > But that seems like it would break if there are DOM changes or frame > navigations while events are suppressed. Right? No. Subframe navigation checks if events are suppressed in the parent and suppressed them also in the subframe.
Attachment #426138 - Flags: review?(Olli.Pettay) → review+
OK, but it still seems like it would be simpler to just check the ancestors and not have to keep ensuring that the descendants are suppressed.
Whiteboard: [needs landing]
(In reply to comment #6) > OK, but it still seems like it would be simpler to just check the ancestors and > not have to keep ensuring that the descendants are suppressed. Yeah, probably. Would have to add ancestor check for timeout suspending too and then remove the code from docshell.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: