`nsINode::GetFlattenedTreeParentNodeForSelection()` and `nsContentUtils::ComparePoints<TreeKind::FlatForSelection>()` work with nodes node in the flattened tree
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox154 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
Attachments
(4 files)
It returns the parent node of a child even if the child is a child of shadow host.
https://searchfox.org/firefox-main/rev/133582f487ff8291ec10bd524db52db0b8ed363e/dom/base/nsIContentInlines.h#52,57,81-83,86,94-97
I guess this solved wrong selection/range handling issue, but this should not be required after fixing bug 2012637.
| Assignee | ||
Comment 1•5 months ago
|
||
The original report including the summary may be wrong.
From Selection API point of view, nodes which are not part of the flattened tree are valid range boundaries since Selection API does not aware of shadow DOM. However, currently, we set the may-cross-shadow-boundary-range if and only if the boundaries can be converted to boundaries in the flattened tree. Otherwise, we fail to set the range.
On the other hand, we convert the points into the ShadowRoot in such case. However, this may cause wrong comparing result with another range boundary.
So, I think GetFlattenedTreeParentNodeForSelection() solves this issue with returning the shadow host element in such case even though it's wrong from the flattened tree point of view, but it must be the right thing.
I think we should explain this in the comment and add some tentative tests for the cases.
| Assignee | ||
Updated•5 months ago
|
| Assignee | ||
Updated•4 months ago
|
| Assignee | ||
Comment 2•2 months ago
|
||
Selection API accepts any DOM node in the non-flattened tree as a
container of a range boundary. Therefore, even if a node is not
formatted in the flattened tree, e.g., inclusive descendants of
unassigned children of a shadow host element or fallback content of
<slot> which has some assigned nodes, it can be a valid container of
selection range boundaries and valid argument of Selection API and
Range API.
Therefore, when climbing up the flattened tree for selection, the
internal APIs should treat the non-formatted nodes are also connected
to the flattened tree. In this patch, the non-formatted children are
connected after the attached shadow root or assigned nodes. The former
is defined as so in the shadow-including tree order [1]. However, I've
not found the definition for the latter. Therefore, I align the behavior
to the former behavior for making the code simpler.
When comparing the range boundaries in the flattened tree for selection
with the non-formatted nodes, we may need to use the position in the end
of the formatted node. However, currently, setting it as
nsRange::mCrossShadowBoundaryRange does not work well with some tests.
Therefore, this patch does not touch around that.
On the other hand, Chrome returns false from Selection.containsNode
if its node parameter is set to a non-formatted node. The spec [2]
defines with a fuzzy word, "visually". It's possible to understand that
non-formatted nodes never visually appear. And I tried to make it
compare with treating the positions of non-formatted nodes as end of
formatted ancestor element. However, it needs complicated code and
causes regressions in some tests, and anyway it's incompatible with
Chrome. Therefore, this patch makes Selection::ContainsNode returns
false if the given node is not formatted in the flattened tree.
| Assignee | ||
Comment 3•2 months ago
|
||
According to testing with simple cases, the intent of the shadow DOM
boundary crossing mode of ContentSubtreeIterator is to list up only
the formatted nodes in the flattened tree. I.e., neither unassigned
children of a shadow host element nor unused fallback content of a
<slot> should be listed up. However, the boundaries of
nsRange::mCrossShadowBoundaryRange may be in unformatted node. Then,
the comparison of the start or end container with the range in the
flattened tree will fail due to treated as disconnected. Thus, various
assertions will fail.
Therefore, this patch makes ContentSubtreeIterator stores the range
which starts from and end at formatted nodes. If the original range
boundaries are not in formatted nodes, it treats the range boundary as
collapsed at end of the ShadowRoot or <slot> which causes the node
is unformatted.
Additionally, ContentSubtreeIterator uses nsINode::GetFirstChild()
etc, which are for TreeKind::DOM. So, it may dive into unformatted
nodes. This patch makes the utility methods templated with TreeKind
and use the templated APIs or ChildIteratorBase to access the DOM
with valid APIs.
| Assignee | ||
Comment 4•2 months ago
|
||
Fixing ContentSubtreeIterator bugs makes stop failing in some cases.
Then, AbstractRange::CollectClientRectsAndText() is changed and
Highlight::RangesAtPoint() starts appending ranges in a shadow
which is covered by an assigned node of a <slot> in the shadow since
selecting a <slot> succeeds the hit-testing in its assigned nodes.
| Assignee | ||
Comment 5•1 month ago
|
||
It may converts a TreeKind::DOM point to a point whose container is
a non-flattened node which is a descendant of a shadow host child or
a <slot> in TreeKind::DOM. Therefore, the name should explain it
may return non-flattened node point.
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/61016 for changes under testing/web-platform/tests
Comment 8•1 month ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/874f68ac15dc
https://hg.mozilla.org/mozilla-central/rev/a4e05af4890e
https://hg.mozilla.org/mozilla-central/rev/373123e888ab
https://hg.mozilla.org/mozilla-central/rev/cc4cdd329058
Upstream PR merged by moz-wptsync-bot
Comment 10•1 month ago
|
||
Comment 11•1 month ago
|
||
Backed out for causing Bug 2052026
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/61028 for changes under testing/web-platform/tests
Upstream PR merged by moz-wptsync-bot
| Assignee | ||
Updated•1 month ago
|
Comment 14•1 month ago
|
||
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/61078 for changes under testing/web-platform/tests
Comment 16•1 month ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/1404f5a62772
https://hg.mozilla.org/mozilla-central/rev/cb00e004db6f
https://hg.mozilla.org/mozilla-central/rev/bc906b1d5fd1
https://hg.mozilla.org/mozilla-central/rev/9bc0b43fe364
Upstream PR merged by moz-wptsync-bot
Updated•1 month ago
|
Description
•