2010730 - `RangeBoundaryBase::AsRangeBoundaryInFlatTree` needs to use proper parent node for the pointing child

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/19-9/27)]

Currently, RangeBoundaryBase::AsRangeBoundaryInFlatTree() use the parent node as-is and the offset. However, the pointing child may be moved into a <slot> in the flat tree. Therefore, it should compute parent node from pointing child.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/19-9/27)]

Attachment: Bug 2010730 - Rewrite `RangeBoundaryBase::AsRangeBoundaryInFlatTree()` r=jjaschke!,smaug!,#dom-core

Currently, the method may return unset boundary. However, we need to
map a DOM tree point to a flat tree point.

The wrong point of current implementation is, the method respect the
container and offset. However, that may be wrong if the pointing child
node is moved into a <slot> or a ShadowRoot. Instead, this patch
computes proper parent from pointing child node if there is.

(I think there should be nsINode::ComputeFlatTreeIndexOfForSelection
or something, but I'd like to put off doing that later because it'd
affect to wider scope of our code.)

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/34e2538a7173
https://hg.mozilla.org/integration/autoland/rev/89ecc0ba319a
Rewrite `RangeBoundaryBase::AsRangeBoundaryInFlatTree()` r=jjaschke,smaug,dom-core

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/96800d940e01
https://hg.mozilla.org/integration/autoland/rev/381c8dfd5d28
Revert "Bug 2010730 - Rewrite `RangeBoundaryBase::AsRangeBoundaryInFlatTree()` r=jjaschke,smaug,dom-core" for causing wpt failures @setBaseAndExtent-start-or-end-in-anonymous-shadow-container.html.

Comment 4

[agoloman]

Backed out for causing wpt failures @setBaseAndExtent-start-or-end-in-anonymous-shadow-container.html.

• Backout link
• Push with failures
• Failure Log

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/19-9/27)]

It's odd, that was not reproduced on Linux before landing... Although the crash itself looks like a possible case and explicitly a bug of GetPromotedEndPoint().

Now, I'm waiting the test result on tryserver.
https://treeherder.mozilla.org/jobs?repo=try&revision=3a4f57bba5aa759979f51dbf9a5d1506e6697d5c

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/19-9/27)]

Hmm, only on macOS, we try to get the latest selection at repainting the Selection.
https://searchfox.org/firefox-main/rev/3eaf7e2acf8186eb7aa579561eaa1312cb89132b/layout/generic/nsFrameSelection.cpp#1743

This may run while there are some pending notifications so that nsDocumentEncoder and RangeBoundaryBase may fail to convert a DOM point to a flat tree point. We need to guarantee that nsFrameSelection::RepaintSelection() call it after flushing the pending things.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/19-9/27)]

Ah, I misunderstand about the shadow DOM construction, but I still don't understand why the result becomes different if I run the path even on Windows. Anyway, the assertion failure cases on macOS are caused by 2 trivial bugs of the patch.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f880cd0ef82c
https://hg.mozilla.org/integration/autoland/rev/83ccd8953f46
Rewrite `RangeBoundaryBase::AsRangeBoundaryInFlatTree()` r=jjaschke,smaug,dom-core

Comment 9

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/83ccd8953f46

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/19-9/27)]

FYI: I guess the fuzzing team will file some assertion failures related to this series of patches. Especially related to nsContentUtils::ComparePoints. However, I won't work on them immediately because now I'm writing patches to fix the known issues of the flattened tree handling.