102133 - Implement phase="target" in XBL

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[[e-mail]]

Many XBL event handlers compare event.target to this, to ensure that the event
was targeted at the XBL widget. Surely that's the point of phase="target"? If
not, then feel free to resolve this bug as invalid. But if it is, then I'll come
up with a patch to change all appropriate handlers.

Comment 1

[David Hyatt]

Hmmm.  I'm not sure phase="target" is supported.  I also don't think that's what
it means.  These values were copying the XHTML events spec.  you should go over
there and see what target means.

Comment 2

[[e-mail]]

Well I had a look at the above URL and it seems to suggest the following:

An event first propagates from the document root to the event target. Capturing
event listeners on the event target or its ancestors will receive the event
during this phase.
An event reaches the event target. This is the target phase. Target event
listeners on the event target will receive the event during this phase.
An event propagates from the event target to the document root. Bubbling event
listeners on the event target or its ancestores will receive the event during
this phase.

Diagrammatically: [root capture] -> [ancestor capture] -> [parent capture] ->
[target capture] -> [target target] -> [target bubble] -> [parent bubble] ->
[ancestor bubble] -> [root bubble]
XBL event handlers appear to default to the bubble phase, so that they receive
events on all their children. For those events for which this is appropriate I
believe the phase should be specified as target.

Comment 3

[David Hyatt]

Ok, so phase="target" should (in our system) be registered as a non-capturing
event listener that requires event.target to be the same as the element to which
the <handler> is bound?

Comment 4

[David Hyatt]

This is not implemented in XBL.  I'll have to add it.

Comment 5

[[e-mail]]

Dave Hyatt wrote:

> Ok, so phase="target" should (in our system) be registered as a non-capturing
> event listener that requires event.target to be the same as the element to which
> the <handler> is bound?

It should be registered as an event listener which fires after all capturing
event listeners and before all bubbling event listeners. It should only fire on
the target, but I'm not exactly sure how this fits in with anonymous content.
Nor can I read C++ well enough to see how events currently propagate. Do you
suppose that anyone would want to register 3 event handlers for the same event :-)

Comment 6

[David Hyatt]

That's basically what I said... rememebr that the DOM API addEventLIstener only
gives you two options, to be a capturer or bubbler, so I'll get the same result
by making the event either a capturer or bubbler and just restricting the target.

Comment 7

[[e-mail]]

I was worried about the order in which events were processed: while capturing
events clearly occur before bubbling events I couldn't determine any
restrictions on event handlers defined on the event target, as the terms
capturing and bubbling are only strictly defined for event handlers on an
ancestor of the target.

Additional note: The doc also suggests that phase="target" should be the default.

Comment 8

[John Morrison]

Just poking my head in, not having been following this too closely, but if 
the above was a question, then ... there are explictly no guarantees about the 
order in which events fire for a given node:

http://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-flow-basic
  Although all EventListeners on the EventTarget are guaranteed to be
  triggered by any event which is received by that EventTarget, no
  specification is made as to the order in which they will receive the
  event with regards to the other EventListeners on the EventTarget.

Comment 9

[[e-mail]]

You're only confusing me further...

Just use the condition event.eventPhase = event.AT_TARGET and I'll be happy :-)

Comment 10

[kill this account]

*** Bug 108950 has been marked as a duplicate of this bug. ***

Comment 11

[bobbell]

Bug 108950 was incorrectly marked as a duplicate of this bug.  It was actually 
a duplicate of bug 102113.  This has been fixed.

Comment 12

[David Hyatt]

--> default component owner

Comment 13

[[e-mail]]

Attachment: Proposed patch

Comment 14

[jag (Peter Annema)]

I'll let hyatt take a look at this first.

Comment 15

[Jan Varga [:janv]]

I think this patch makes sense.

Comment 16

[David Hyatt]

Comment on attachment 109819 [details] [diff] [review]
Proposed patch

Looks good, and I'm glad you left the default value of mPhase alone.  

Bubbling should remain the default for XBL handlers.  There would IMO be
extraordinary confusion if XBL handlers used "target" as the default phase, and
that wouldn't be consistent with the idea of handlers being
"filtered/restricted" via attributes.

r=hyatt

Comment 17

[jag (Peter Annema)]

Comment on attachment 109819 [details] [diff] [review]
Proposed patch

sr=jag

Comment 18

[[e-mail]]

Checked in a few days ago.