Closed Bug 621618 Opened 9 years ago Closed 6 years ago

"ASSERTION: Removing id entry that doesn't exist" with <svg:use>


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

Not set



Tracking Status
blocking2.0 --- .x+


(Reporter: jruderman, Assigned: dbaron)


(Blocks 1 open bug)


(Keywords: assertion, regression, testcase, Whiteboard: [mentor=bz][lang=c++])


(6 files)

1. Load the testcase.
2. Close the tab or window or quit Firefox.

###!!! ASSERTION: Removing id entry that doesn't exist: '!aElement->GetOwnerDoc() || !aElement->GetOwnerDoc()->IsHTML() || mIdContentList.IndexOf(aElement) >= 0', file content/base/src/nsDocument.cpp, line 425

(Same assertion as in bug 593302, which had a very different testcase.)
Attached file stack trace
Could be bad, but I need to investigate. Definitely a regression since id handling is much more sensitive now.
Assignee: nobody → jonas
blocking2.0: --- → ?
Keywords: regression
I think we can ship 2.0 w/o this, but if this is more severe than we think we'll reconsider.
blocking2.0: ? → .x
This can lead to additional badness, such as

ASSERTION: No Document Request!: 'mDocumentRequest', file uriloader/base/nsDocLoader.cpp

ASSERTION: Firing OnStateChange(...) notification with a NULL request!: 'aRequest', file uriloader/base/nsDocLoader.cpp
Duplicate of this bug: 703549
This is being hit by dom/plugins/test/mochitest/test_bug813906.html
The basic issue here is that things only add themselves to the id table if they're not anonymous, but they remove unconditionally....
Whiteboard: [mentor=bz][lang=c++]
The patch in bug 741295 would fix this I'm pretty sure.
Depends on: 741295
Hmm.... on second thought, maybe it wouldn't.
No longer depends on: 741295
This is the most common assertion I see, since it shows up all the time on the iD editor.
Assignee: jonas → dbaron
I think this could have been done as part of Bug 700981 part 2, which
moved AddToIdTable and RemoveFromIdTable calls from nsStyledElement to
Attachment #8384428 - Flags: review?(bzbarsky)
The patch in bug 741295 probably also fixes this one.
Well, it certainly makes patches 1 and 2 irrelevant.

I'm not sure about patch 4, which is actually the part most relevant to the cases for which we have testcases for the assertion firing.  It seems to make that inconsistency worse.
Comment on attachment 8384428 [details] [diff] [review]
patch 1:  Remove BindToTree overrides in nsXMLElement that exist only to make AddToIdTable/RemoveFromIdTable calls already in the base class (Element)

Attachment #8384428 - Flags: review?(bzbarsky) → review+
Comment on attachment 8384429 [details] [diff] [review]
patch 2:  Make nsXMLElement reuse Element::AddToIdTable and Element::RemoveFromIdTable thoroughly

Attachment #8384429 - Flags: review?(bzbarsky) → review+
Comment on attachment 8384430 [details] [diff] [review]
patch 3:  Add FIXMEs in nsXULDocument for ID handling

Attachment #8384430 - Flags: review?(bzbarsky) → review+
Comment on attachment 8384431 [details] [diff] [review]
patch 4:  Make conditions in Element::RemoveFromIdTable match those in AddToIdTable, to avoid asymmetric calls to the document's functions

Attachment #8384431 - Flags: review?(bzbarsky) → review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.