If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in mozilla38



DOM: Core & HTML
3 years ago
3 years ago


(Reporter: anujagarwal464, Assigned: Boris Egorov, Mentored)


34 Branch

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [lang=c++])


(1 attachment, 1 obsolete attachment)

Comment hidden (empty)

Comment 1

3 years ago
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

3 years ago
Created attachment 8496439 [details] [diff] [review]
patch to trigger bug
RemoveAttributeNode is implemented at http://mxr.mozilla.org/mozilla-central/source/content/base/src/Element.cpp#1027.


3 years ago
Depends on: 1060938

Comment 4

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

Comment 5

3 years ago
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

3 years ago
Created attachment 8554265 [details] [diff] [review]
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.
Attachment #8496439 - Attachment is obsolete: true
Attachment #8554265 - Flags: review?(jst)


3 years ago
Attachment #8554265 - Flags: review?(jst) → review?(bzbarsky)

Comment 7

3 years ago
@bz is a mentor for this bug, so I probably should set him as a reviewer from the beginning..
Comment on attachment 8554265 [details] [diff] [review]
Patch to solve bug + mochitest

This looks great, thanks!
Attachment #8554265 - Flags: review?(bzbarsky) → review+
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7efccf4361b7
Looks good, apart from things that were also broken on inbound on the parent of that push.
Keywords: checkin-needed
Assignee: nobody → egorov
Keywords: checkin-needed
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.