Closed Bug 872870 Opened 6 years ago Closed 6 years ago
Assertions and crash with undo
Manager, removing document .document Element
With: user_pref("dom.undo_manager.enabled", true); ###!!! ASSERTION: Inserting node that already has parent: '!aKid->GetParentNode()', file content/base/src/nsINode.cpp, line 1326 ###!!! ASSERTION: aChild with prev sibling?: '!aChild->GetPreviousSibling()', file content/base/src/nsAttrAndChildArray.cpp, line 822 ###!!! ASSERTION: Already have a document. Unbind first!: '!GetCurrentDoc()', file content/base/src/Element.cpp, line 1016 ###!!! ASSERTION: Shouldn't be here!: '!(IsNodeOfType(eCONTENT) && IsInDoc())', file nsINode.h, line 1495 ###!!! ASSERTION: Bound to wrong document: 'aDocument == GetCurrentDoc()', file content/base/src/Element.cpp, line 1200 ###!!! ASSERTION: No style context found!: 'mStyleContext', file nsStyleStructList.h, line 61 Crash [@ nsGfxScrollFrameInner::IsLTR() const]
Related to bug 840877?
On Windows: bp-4158e848-e25d-47b5-9aea-b6a952130516.
Crash Signature: [@ NS_DebugBreak | nsINode::doInsertChildAt(nsIContent*, unsigned int, bool, nsAttrAndChildArray&) ] [@ nsCachedStyleData::GetStyleVisibility() ]
OS: Mac OS X → All
Hardware: x86_64 → All
Could you look at this, William? Thanks.
Assignee: nobody → wchen
Marking sec-high because the assertions sound kind of bad.
Comment on attachment 761022 [details] [diff] [review] Use GetParentNode() instead of GetParent() in UndoManager. [Security approval request comment] How easily could an exploit be constructed based on the patch? Test case included could be used to reproduce the bug. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. Which older supported branches are affected by this flaw? 20+ If not all supported branches, which bug introduced the flaw? 617532 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? No backports. The patch will likely apply cleanly on to previous branches. How likely is this patch to cause regressions; how much testing does it need? Not likely to cause regression. Existing tests is likely sufficient. It should also be noted that this feature is hidden behind a pref and disabled by default.
Attachment #761022 - Flags: sec-approval?
Comment on attachment 761022 [details] [diff] [review] Use GetParentNode() instead of GetParent() in UndoManager. sec-approval+ since this is pref'd off but the test needs to be in a separate patch and only checked in after we ship the fix.
Attachment #761022 - Flags: sec-approval? → sec-approval+
You need to log in before you can comment on or make changes to this bug.