454746 - "ASSERTION: Start parent and end parent give different root!" with XBL, ranges

Status: RESOLVED WONTFIX

(Core :: XBL, defect, P3)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: Start parent and end parent give different root!: 'newRoot == IsValidBoundary(mEndParent)', file /Users/jruderman/central/content/base/src/nsRange.cpp, line 352

###!!! ASSERTION: Wrong root: '!aRoot || (nsContentUtils::ContentIsDescendantOf(aStartN, aRoot) && nsContentUtils::ContentIsDescendantOf(aEndN, aRoot) && aRoot == IsValidBoundary(aStartN) && aRoot == IsValidBoundary(aEndN))', file /Users/jruderman/central/content/base/src/nsRange.cpp, line 443

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is fun.  The basic issue is that we issue no notification (like say ContentRemoved) when removing the XBL anonymous content on binding teardown.  So the range has no idea that the <h:span> node there got removed, and doesn't update its endpoints.  Then when we mess around with the DOM it doesn't know (because it's observing the HTMLElement, which doesn't get those notifications from the disconnected subtree), and gets all confused.

Comment 2

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

Ugh, tricky. I don't really know a good solution. A solution (though complicated) would be to force ranges never to go into anonymous trees. So if you for example select into a binding we'd create two ranges, one inside the binding and one outside...

For extra credit, apply to nested bindings :(

Comment 3

[Boris Zbarsky [:bzbarsky]]

Or we could notify in some way when the anonymous content of a binding is removed from the tree, right?  Note that in this case the range is entirely inside the binding's anonymous contet to start with, so your proposed solution wouldn't help anyway.

Comment 4

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

Good point. That seems like a simpler solution

Comment 5

[Johnny Stenback (:jst)]

Not a blocker unless we think this can result in exploitable crashes.

Comment 6

[Jesse Ruderman]

I think this can also lead to:

###!!! ASSERTION: Bad root: '!aRoot || (aStartN->IsNodeOfType(nsINode::eCONTENT) && aEndN->IsNodeOfType(nsINode::eCONTENT) && aRoot == static_cast<nsIContent*>(aStartN)->GetBindingParent() && aRoot == static_cast<nsIContent*>(aEndN)->GetBindingParent()) || (!aRoot->GetNodeParent() && (aRoot->IsNodeOfType(nsINode::eDOCUMENT) || aRoot->IsNodeOfType(nsINode::eATTRIBUTE) || aRoot->IsNodeOfType(nsINode::eDOCUMENT_FRAGMENT) || aRoot->IsNodeOfType(nsINode::eCONTENT)))', file /Users/jruderman/central/content/base/src/nsRange.cpp, line 454

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 8

[Boris Zbarsky [:bzbarsky]]

XBL is now disabled in Firefox (Bug 1583314) and is in the process of being removed from Gecko (Bug 1566221), so closing bugs requesting changes to its implementation as wontfix.