Closed Bug 847699 Opened 11 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
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: