174910 - Mutation events inside anchor elements are not propogated up in tree

Status: RESOLVED FIXED

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

Description

[Aaron Leventhal]

When a mutation event such as DOMAttrModified, DOMNodeInserted or DOMNodeRemoved
occurs somewhere inside an <a>, it is swallowed.

I think this has something to do with the event ending up in
nsGenericHTMLElement::HandleDOMEventForAnchors() and going no further.

Comment 1

[Aaron Leventhal]

Attachment: You can only listen for this doc's <div> mutations, not the <a> mutations

Comment 2

[Brian Ryner (not reading)]

joki is gone, -> me.

Aaron, is this blocking anything?

Comment 3

[Aaron Leventhal]

Yes, but I'm not sure what the priority should be. It would be nice to get it
fixed by January/February.

Comment 4

[Aaron Leventhal]

Now that the accessibility cache is implemented using mutation events, this is
an important bug. If we don't fix it, the accessibility APIs could report
incorrect data for some pages, and it would inevitably lead to crashes.

Comment 5

[Aaron Leventhal]

Attachment: Shouldn't event get propogated up DOM tree even when aPresContext argument == nsnull?

Comment 6

[Aaron Leventhal]

Other DOM events generated without a pres context would be swallowed by anchors
as well.

Comment 7

[Brian Ryner (not reading)]

Comment on attachment 124720 [details] [diff] [review]
Shouldn't event get propogated up DOM tree even when aPresContext argument == nsnull?

I'd suggest keeping the aEventStatus check as-is, and moving the aPresContext
check like you suggested.  However, I think it should be changed to an early
return and not use NS_ENSURE_ARG (that macro implies and unexpected condition
and spews to the console).

I'm also curious why this event doesn't have an aPresContext parameter.

Comment 8

[Aaron Leventhal]

Attachment: New patch with bryner's suggestions

There's no pres context because the event is generated by the code in
mozilla/content itself, for example nsGenericHTMLElement::SetAttr() or
::InsertChildAt()

Comment 9

[Aaron Leventhal]

Patch ready.

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 124799 [details] [diff] [review]
New patch with bryner's suggestions

sr=jst

Comment 11

[Aaron Leventhal]

this has been checked in

Comment 12

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381