545268 - Event suppression doesn't work with child widgets removed

Status: RESOLVED FIXED

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

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.)

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

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.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

(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.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

(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.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/e423964db71c