Closed Bug 847699 Opened 12 years ago Closed 2 years ago

Crash extending the selection after Find [@ nsRange::UnregisterCommonAncestor]

Categories

(Core :: DOM: Selection, defect)

defect
Not set
critical

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: jruderman, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, crash, testcase, Whiteboard: [fuzzblocker])

Crash Data

Attachments

(2 files)

Attached file manual testcase (deleted) —
1. Load the testcase 2. Press ⌘F 3. Press x 4. Press Esc. (The 'x' should now be selected.) 5. Press Shift+Down twice ###!!! ASSERTION: unexpected disconnected nodes: 'aDisconnected', file content/base/src/nsContentUtils.cpp, line 2002 Assertion failure: mCommonParent && startParent && endParent, at content/base/src/nsContentIterator.cpp:1199 Or crash [@ nsRange::UnregisterCommonAncestor] with a null deref
Requires https://www.squarefree.com/extensions/domFuzzLite3.xpi (Use this testcase if you want to bisect or something.)
Crash Signature: [@ nsRange::UnregisterCommonAncestor(nsINode*) ]
1. nsFind creates a range with "SetMaySpanAnonymousSubtrees(true)" http://hg.mozilla.org/mozilla-central/annotate/3d81fc67e6ed/embedding/components/find/src/nsFind.cpp#l1261 2. nsFrameSelection::MoveCaret decides the anonymous div inside the <meter> is the target to extend the selection to 3. Selection::Extend accepts the anon-div as a range end point due to 1. and adds the range to the selection which triggers... 4. a call to MarkDescendants http://hg.mozilla.org/mozilla-central/annotate/3d81fc67e6ed/content/base/src/nsRange.cpp#l288 which use "nsINode::GetNextNode" to traverse the tree, which doesn't include anon content so the range end point isn't marked. 2. seems like a bug and fixing it would avoid the crash in this case I think 4. is maybe a bug too - it should mark all descendants 1. is a fix for bug 384706; it's the only use of SetMaySpanAnonymousSubtrees AFAICT
OS: Mac OS X → All
Hardware: x86_64 → All
4. is somewhat hard to fix. Would need to traverse frames to find anonymous content.
Would nsIContent::GetChildren(eAllChildren) do the job?
Does mutation observers get notified for anon-content changes in the same way as normal content? In the "manual testcase" we have: <div>x<meter/></div> The range has startParent="x" endParent=anon-div-in-meter and the common ancestor the range is observing is the (non-anonymous) <div>. It appears that the style change (document.documentElement.style.content="blah") causes the anon-div-in-meter to UnbindFromTree without any notification to the range, afaict from debugging.
Actually, the node we're observing is nsRange::mRoot which I guess is the document. Observing only the common ancestor was just wishful thinking on my part :-)
The range isn't notified because ParentChainChanged doesn't "bubble up" like the other mutations do, it's just called for each node in the affected sub-tree, not all the ancestors. So we need to observe the start/end nodes too just to see ParentChainChanged on those, but that will also lead to the range seeing the other types of mutations more than once? Might be simpler to implement a new mutation type, ParentChainChangedForSubtree or some such, that would be notified on the root of the UnboundFromTree sub-tree and its ancestors.
When adding new mutation types, be very careful with performance. Mutation notifications show up in the profiles pretty badly.
One option would be to have separate mutation observer for start/end nodes.
Summary: Crash extending the selection [@ nsRange::UnregisterCommonAncestor] → Crash extending the selection after Find [@ nsRange::UnregisterCommonAncestor]
Whiteboard: [fuzzblocker]
Blocks: fuzz-keys
Crash Signature: [@ nsRange::UnregisterCommonAncestor(nsINode*) ] → [@ nsRange::UnregisterCommonAncestor(nsINode*) ] [@ nsRange::UnregisterCommonAncestor ]

I can no longer reproduce the assertion using the testcase from comment 0. Closing.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: