1070015 - When we try pass to removeAttributeNode() attr object, which belongs to the different element, "NotFoundError" exception is not thrown.

Status: RESOLVED FIXED

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

Comment 1

[Boris Egorov]

Can I take this one? I created a test in content/base/test from a similar one in the same directory. It triggers a bug and fails in two places now:

1. When removeAttributeNode() not throws exception when it called with 'foreign' attr object as argument.
2. When node was deleted after such call.

Test checks removing of attribute with a same name in two elements. Would it be useful to check removing with different names?

I can't find sources for removeAttributeNode() for now. I used ack/ag since 'hg grep' search through all commits. There are idl files dom/interfaces/core/nsIDOMElement.idl and dom/webidl/Element.webidl with metioned function, but it is just interface descriptions. Where can I find it?

Comment 2

[Boris Egorov]

Attachment: patch to trigger bug

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

RemoveAttributeNode is implemented at http://mxr.mozilla.org/mozilla-central/source/content/base/src/Element.cpp#1027.

Comment 4

[Anuj Agarwal [:anujagarwal464]]

@Boris: Use http://dxr.mozilla.org/mozilla-central/source/ to search for source.

Comment 5

[Boris Egorov]

So I made some investigation.

Element::RemoveAttributeNode() takes Attr, use its NodeInfo to extract namespaceURI and LocalName, and pass this two nsAString's to nsDomAttributeMap::RemoveNamedItemNS(). Later calls do not use any other information about attribute except its name.
I can't found any way to check if specific attribute belongs to element. Is there any method for this?

Comment 6

[Boris Egorov]

Attachment: Patch to solve bug + mochitest

With help of the user Ms2ger in IRC I found needed method and a way to throw error. Looks like this patch fixes problem.

Comment 7

[Boris Egorov]

@bz is a mentor for this bug, so I probably should set him as a reviewer from the beginning..

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8554265 [details] [diff] [review]
Patch to solve bug + mochitest

This looks great, thanks!

Comment 9

[Boris Zbarsky [:bzbarsky]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7efccf4361b7

Comment 10

[Boris Zbarsky [:bzbarsky]]

Looks good, apart from things that were also broken on inbound on the parent of that push.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba0b46b1b3b5

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ba0b46b1b3b5