263240 - Lots of events not going through system event group dispatch

Status: RESOLVED WORKSFORME

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Brian Ryner (not reading)]

So, recently I found that blur events (dispatched from nsEventStateManager)
aren't going through the system event group.  It turns out that pretty much
anyone who creates an nsEvent and calls HandleDOMEvent on the target needs to do
this explicitly, and only a couple of callers (PresShell::HandleEventInternal
and nsEventStateManager::DispatchNewEvent) are.

I think we need to centralize this so that every caller doesn't need to deal
with it.  Here are a couple of ideas:

1. Add a new method, DispatchDOMEvent, on nsIContent, nsIDocument, and
nsIScriptGlobalObject which takes the same parameters as HandleDOMEvent.  It
sets the event target to the current object (that's another thing we're
inconsistent about, some callers do it up front, some get it in
nsEventListenerManager::HandleEventSubType, and others miss it entirely, if an
event only ever goes through DispatchToInterface in nsEventListenerManager), and
then the event is dispatched through the default event group, the stop-dispatch
flag is cleared, and it's dispatched through the system event group.

2. Modify the existing nsGenericElement::HandleDOMEvent to do this automatically
if NS_EVENT_FLAG_INIT is set when the event is passed in.  We know this is the
event's entry point into the DOM.  It could happen near the end of
HandleDOMEvent where the comment says "We're leaving the DOM event loop"

thoughts, anyone?

Comment 1

[Boris Zbarsky [:bzbarsky]]

I like the first idea more, I think (as you say, it lets us fix the current
event  target too).

Comment 2

[Brian Ryner (not reading)]

We could also easily set the event target via option #2, at the start of
HandleDOMEvent where it checks for NS_EVENT_FLAG_INIT.

Comment 3

[Brian Ryner (not reading)]

Setting the event target as I described is kind of tricky due to our lazy
creation of DOM events.  We actually use a global table mapping nsEvents to
their targets, but that lives on the presshell, and only works on events that
are dispatched through the presshell.

So, I think we should fix that problem separately, preferably by getting rid of
the lazy DOM event creation and setting the target up-front (what a concept!).

Comment 4

[Boris Zbarsky [:bzbarsky]]

I've always wondered why we have the lazy DOMEvent thing.... it just seems to
complicate the code unnecessarily.

Comment 5

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

This was fixed in bug 234455.