Closed
Bug 847699
Opened 12 years ago
Closed 2 years ago
Crash extending the selection after Find [@ nsRange::UnregisterCommonAncestor]
Categories
(Core :: DOM: Selection, defect)
Core
DOM: Selection
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: jruderman, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: assertion, crash, testcase, Whiteboard: [fuzzblocker])
Crash Data
Attachments
(2 files)
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
Reporter | ||
Comment 1•12 years ago
|
||
Requires https://www.squarefree.com/extensions/domFuzzLite3.xpi
(Use this testcase if you want to bisect or something.)
Reporter | ||
Comment 2•12 years ago
|
||
Updated•12 years ago
|
Crash Signature: [@ nsRange::UnregisterCommonAncestor(nsINode*) ]
Comment 3•12 years ago
|
||
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
Comment 4•12 years ago
|
||
4. is somewhat hard to fix. Would need to traverse frames to find anonymous content.
Comment 5•12 years ago
|
||
Would nsIContent::GetChildren(eAllChildren) do the job?
Comment 6•12 years ago
|
||
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.
Comment 7•12 years ago
|
||
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 :-)
Comment 8•12 years ago
|
||
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.
Comment 9•12 years ago
|
||
When adding new mutation types, be very careful with performance.
Mutation notifications show up in the profiles pretty badly.
Comment 10•12 years ago
|
||
One option would be to have separate mutation observer for start/end nodes.
Reporter | ||
Updated•12 years ago
|
Summary: Crash extending the selection [@ nsRange::UnregisterCommonAncestor] → Crash extending the selection after Find [@ nsRange::UnregisterCommonAncestor]
Whiteboard: [fuzzblocker]
Updated•9 years ago
|
Crash Signature: [@ nsRange::UnregisterCommonAncestor(nsINode*) ] → [@ nsRange::UnregisterCommonAncestor(nsINode*) ]
[@ nsRange::UnregisterCommonAncestor ]
Comment 11•2 years ago
|
||
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.
Description
•