Assertion failure: !aRoot || aNotInsertedYet || (nsContentUtils::ContentIsDescendantOf(aStart.Container(), aRoot) && ... (Wrong root), at src/dom/base/nsRange.cpp:976
Categories
(Core :: DOM: Selection, defect, P2)
Tracking
()
People
(Reporter: tsmith, Assigned: saschanaz)
References
(Blocks 2 open bugs)
Details
(Keywords: assertion, testcase)
Attachments
(3 files)
|
||
Updated•7 years ago
|
|
Assignee | |
Comment 1•4 years ago
|
||
Somehow I couldn't repro bug 1617278 but can repro this one. Continuing from https://bugzilla.mozilla.org/show_bug.cgi?id=1617278#c14:
I think this is more about <details> itself. Selection::mAnchorFocusRange incorrectly includes text node from disconnected internal <summary> element, which is why the assertion fails. It seems the disconnection happens without correctly updating selection range. FlushPendingNotifications does not help this time.
Emilio, do you have a quick idea about selection update in tree change?
|
||
Comment 2•4 years ago
|
||
Not off the top of my head... How does selection usually get updated when an element is unbound from the tree? The anonymous summary should be unbound when being removed from the frame tree via nsIFrame::DestroyAnonymousContent.
|
|
Assignee | |
Comment 3•4 years ago
•
|
||
A smaller repro. (Click the element.)
|
|
Assignee | |
Comment 4•4 years ago
|
||
While it seems nsRange::ContentRemoved can do the work, why does <details> need to destruct and create new <summary> every time it gets a click? Could we just cache it?
|
|
||
Comment 5•4 years ago
|
||
Well that would solve the click case but not the display: none case. Bug 1308080 would fix both I guess, but it seems there's a bunch of other elements that would have the same issue.
|
|
Assignee | |
Comment 6•4 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)
Well that would solve the click case but not the
display: nonecase. Bug 1308080 would fix both I guess, but it seems there's a bunch of other elements that would have the same issue.
Do you have an example where anonymous contents are selectable?
BTW I found nsINode::IsMaybeSelected returns false for internal summary element even when it is selectable, is this expected or should I consider it as a bug?
|
|
Assignee | |
Updated•4 years ago
|
|
|
||
Comment 7•4 years ago
|
||
(In reply to Kagami :saschanaz from comment #6)
(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)
Well that would solve the click case but not the
display: nonecase. Bug 1308080 would fix both I guess, but it seems there's a bunch of other elements that would have the same issue.Do you have an example where anonymous contents are selectable
<input type=text>? Though those have their own selection so maybe not a concern.
Other than that i don't know, we don't have that many nsIAnonymousContentCreator implementations.
BTW I found nsINode::IsMaybeSelected returns false for internal summary element even when it is selectable, is this expected or should I consider it as a bug?
Not sure, in general the interaction of selection with shadow dom / anon content is pretty messy... I suspect we clear those flags on the dom and not properly clear them for anonymous kids.
Maybe we should just make the anonymous content not selectable for now, if we believe <details> is the only element hitting this issue. So something like details > summary:-moz-native-anonymous { user-select: none } around here.
|
|
Assignee | |
Comment 8•4 years ago
|
||
It seems user-select: none is the way to go for now and ultimately shadow DOM. It already has a weird selection behavior, the anonymous summary can be selected but not together with other text node, virtually making it unselectable.
|
|
Assignee | |
Comment 9•4 years ago
|
||
Just for the record, this was my intended patch:
void nsIFrame::DestroyAnonymousContent(
nsPresContext* aPresContext, already_AddRefed<nsIContent>&& aContent) {
if (nsCOMPtr<nsIContent> content = aContent) {
// XXX(krosylight): Why can't we use content->IsMaybeSelected()?
// if (content->IsAnyOfHTMLElements(nsGkAtoms::summary)) {
// if (Selection* selection = aPresContext->PresShell()->GetSelection(nsISelectionController::SELECTION_NORMAL)) {
// for (uint32_t i = 0; i < selection->RangeCount(); i++) {
// selection->GetRangeAt(i)->ContentRemoved(content, content->GetPreviousSibling());
// }
// }
// }
aPresContext->EventStateManager()->NativeAnonymousContentRemoved(content);
aPresContext->PresShell()->NativeAnonymousContentRemoved(content);
content->UnbindFromTree();
}
}
|
|
Assignee | |
Comment 10•4 years ago
|
||
It has already been impossible to select it together with other nodes.
|
||
Comment 11•4 years ago
|
||
|
||
Comment 12•4 years ago
|
||
Backed out for causing selection wpt failures
backout: https://hg.mozilla.org/integration/autoland/rev/0a11625a91651e6507d8b3e4de1e395d77179f88
push where it appeared: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=6b9978d2a957cb6ec9d6bd0e0155d16db887a186&searchStr=wpt&test_paths=%2Fselection
failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=319147433&repo=autoland&lineNumber=8735
[task 2020-10-20T16:13:41.819Z] 16:13:41 INFO - TEST-START | /selection/select-internal.html
[task 2020-10-20T16:13:41.819Z] 16:13:41 INFO - Closing window 108
[task 2020-10-20T16:13:42.315Z] 16:13:42 INFO - {'actions': [{u'type': u'none', u'id': u'0', u'actions': [{u'duration': 16, u'type': u'pause'}, {u'duration': 16, u'type': u'pause'}, {u'duration': 16, u'type': u'pause'}, {u'duration': 16, u'type': u'pause'}]}, {u'type': u'pointer', u'actions': [{u'y': 5, u'x': 5, u'type': u'pointerMove', u'origin': {'element-6066-11e4-a52e-4f735466cecf': u'9468cd2f-838f-42c2-849a-3a3a741cf9d8', 'chromeelement-9fc5-4b51-a3c8-01716eedeb04': u'9468cd2f-838f-42c2-849a-3a3a741cf9d8'}}, {u'button': 0, u'type': u'pointerDown'}, {u'y': 50, u'x': 50, u'type': u'pointerMove', u'origin': u'viewport'}, {u'button': 0, u'type': u'pointerUp'}], u'parameters': {u'pointerType': u'mouse'}, u'id': u'1'}]}
[task 2020-10-20T16:13:42.371Z] 16:13:42 INFO - PID 4015 | [Parent 4015, Main Thread] WARNING: NS_ENSURE_TRUE(rootFrame) failed: file /builds/worker/checkouts/gecko/dom/base/nsGlobalWindowOuter.cpp:4284
[task 2020-10-20T16:13:42.378Z] 16:13:42 INFO - PID 4015 | console.warn: SearchSettings: "get: No settings file exists, new profile?" (new Error("", "(unknown module)"))
[task 2020-10-20T16:13:42.455Z] 16:13:42 INFO - PID 1194 | [Child 3964, Main Thread] WARNING: NS_ENSURE_TRUE(info) failed: file /builds/worker/checkouts/gecko/extensions/permissions/PermissionDelegateHandler.cpp:348
[task 2020-10-20T16:13:42.455Z] 16:13:42 INFO - PID 1194 | [Child 1469, Main Thread] WARNING: NS_ENSURE_TRUE(info) failed: file /builds/worker/checkouts/gecko/extensions/permissions/PermissionDelegateHandler.cpp:348
[task 2020-10-20T16:13:42.515Z] 16:13:42 INFO - PID 4015 | [Parent 4015, Main Thread] WARNING: Failed to retarget HTML data delivery to the parser thread.: file /builds/worker/checkouts/gecko/parser/html/nsHtml5StreamParser.cpp:1132
[task 2020-10-20T16:13:42.591Z] 16:13:42 INFO -
[task 2020-10-20T16:13:42.591Z] 16:13:42 INFO - TEST-UNEXPECTED-FAIL | /selection/select-internal.html | Selecting the default summary of <details> should report a DOM-visible ancestor - promise_test: Unhandled rejection with value: object "TypeError: can't access property "constructor", selection.anchorNode is null"
[task 2020-10-20T16:13:42.591Z] 16:13:42 INFO - TEST-OK | /selection/select-internal.html | took 774ms
|
|
Assignee | |
Comment 13•4 years ago
|
||
Ah, so it conflicts with my earlier test. Well, sure it does, and I have no other way to test it 🤔. Maybe we should disable it for now...
|
|
||
Comment 14•4 years ago
|
||
|
||
Comment 16•4 years ago
|
||
| bugherder | ||
|
||
Updated•4 years ago
|
Description
•