364413 - Related node on DOMAttrModified listener isn't the same as attr node set by SetAttributeNodeNS

Status: RESOLVED FIXED

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

Description

[Alex Vincent [:WeirdAl]]

Similar problem to bug 364092.  When I set an attribute via setAttributeNodeNS, the event listener on the owner element gets an event with a related node similar to, but not exactly, the node being set.

Testcase coming up as an attachment.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061219 SeaMonkey/1.5a

Comment 1

[Alex Vincent [:WeirdAl]]

Attachment: testcase

Comment 2

[Alex Vincent [:WeirdAl]]

Attachment: relevant stack trace

In this case, nsDOMAttributeMap::SetNamedItemInternal() calls on nsGenericElement::SetAttr(), but does not pass in the original node.  Later on, we have a call to GetNamedItemNS(), which tries to find the new attribute.  No attribute has been set yet, so we create a new one.  Ergo, we fail the test.

Comment 3

[Alex Vincent [:WeirdAl]]

Attachment: patch, v1

Thanks to smaug for inspiring this patch with his patch for the removeAttributeNode case.  It took me a couple hours to see this myself.

Comment 4

[Alex Vincent [:WeirdAl]]

Comment on attachment 249217 [details] [diff] [review]
patch, v1

Uh, oops.  We lost the new value of the attribute with this patch.

Comment 5

[Alex Vincent [:WeirdAl]]

Attachment: testcase with attribute value shown

Comment 6

[Alex Vincent [:WeirdAl]]

Attachment: patch, v1.1

This patch gets the attribute value, and the event has the correct prevValue and newValue settings (according to Venkman).

Comment 7

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

Won't event.relatedNode.ownerElement be wrong with this patch? I.e. don't you want to call SetMap before calling SetAttr and solve the value problem some other way (I'm not sure I fully understand the comment in the patch)

Comment 8

[Alex Vincent [:WeirdAl]]

Comment on attachment 249222 [details] [diff] [review]
patch, v1.1

Nuts, you're right.  As Mike Rowe would call it, "rookie mistake!"

smaug, do you want to take this on and do it right?

Comment 9

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

(In reply to comment #8)
> (From update of attachment 249222 [details] [diff] [review] [edit])
> Nuts, you're right.  As Mike Rowe would call it, "rookie mistake!"
> 
> smaug, do you want to take this on and do it right?
> 

Ok :)

Comment 10

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

Attachment: proposed patch

Comment 11

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

I'll add the .ownerElement test to the testcase and add that to mochitest.