273798 - Dispatching Mutation Event DOMAttrModified is implemented in several places

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

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

At the moment dispatching DOMAttrModified is implemented in 6 places,
twice in GenericElement, twice in XULElement and once in HTMLElement and
SVGElement. There could be one (or perhaps two, REMOVAL may need its own)
static method in nsGenericElement to implement it once and properly.

Bug 231676 is another solution for this, but it needs changes to
document observer.

Comment 1

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

Attachment: untested patch

Something like this.

Comment 2

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

Attachment: v1

This adds (static) nsGenericElement::DispatchDOMAttrModified.
It is used in every place, where DOMAttrModied is created.

This also fixes few bugs. REMOVAL is now dispatched after the attribute has
been
removed and the relatedNode is the correct nsIDOMAttr - now also the namespaced
attributes should work.

Comment 3

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

As Jonas said in Bug 232009
"Currently the attribute-node can be compleatly unaware that
it no longer is an attribute on the element."

Have to fix that too, but it is probably separate bug.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The absolutly best solution would be to fix bug 231676. However this is a step
in the right direction.

Comment 5

[Peter Van der Beken [:peterv]]

Should the patch be reviewed?

Comment 6

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

(In reply to comment #5)
> Should the patch be reviewed?
> 

Ah, no. This is old. Way too old.

Comment 7

[Boris Zbarsky [:bzbarsky]]

At this point we're down to three dispatchers of DOMAttrModified:

nsXULElement::UnsetAttr
nsGenericElement::SetAttrAndNotify
nsGenericElement::UnsetAttr

I believe we wanted to merge the latter two by having an AfterSetAttr method or something.  At least sicking's mentioned something like that.  nsXULElement::UnsetAttr should just call up into the superclass more somehow.

Comment 8

[Boris Zbarsky [:bzbarsky]]

The merging comment 7 mentions is bug 314286.

Comment 9

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

this was fixed long ago.