1879579 - CompareTreePosition uses flat tree order for DOM operations, which seems unexpected.

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

So bug 1837540 introduced nsContentUtils::CompareTreePosition which reuses nsLayoutUtils::CompareTreePosition. However I don't think these should do the same.

nsLayoutUtils::CompareTreePosition uses the flat tree, mostly because it needs to for quotes and counters.

However it's weird that nsContentUtils::CompareTreePosition does that. It doesn't match the semantics of nsContentUtils::PositionIsBefore, and it seems likely that it's causing subtle correctness issues in presence of shadow DOM.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Maybe we should have a single CompareTreePosition that takes a template parameter or something enum class TreeKind { Light, Flat } or so.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Yeah, I think we should be always clear on what kind of tree we're considering.
When nsContentUtils::CompareTreePosition was created it just moved some existing nsLayoutUtils::CompareTreePosition usage inside it.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1837540

:avandolder, since you are the author of the regressor, bug 1837540, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

It should be easy to construct a test-case that causes incorrect behavior (pre-existing) with something like:

<form>
  <some-component-with-two-slots>
    <input slot="second-slot">
    <input slot="first-slot">
  </some-component-with-two-slots>
</form>

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Err, wrong field, similar bug # :)

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1879579 - Clean up CompareTreePosition and related code. r=smaug

Make TreeOrderedArray support what the form controls stuff needs, and
use it instead of custom nsContentUtils stuff.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: test-case

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1acc95286189
Clean up CompareTreePosition and related code. r=smaug

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/44533 for changes under testing/web-platform/tests

Comment 10

[Noemi Erli[:noemi_erli]]

Backed out changeset 1acc95286189 (Bug 1879579) for causing mass failures CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=H8d-ZShWTji-vSSfDdRT_Q.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=1acc952861899da04fe4d070552c2f9974db27ac

Log: https://treeherder.mozilla.org/logviewer?job_id=446920333&repo=autoland&lineNumber=2502
https://treeherder.mozilla.org/logviewer?job_id=446920213&repo=autoland&lineNumber=7864
https://treeherder.mozilla.org/logviewer?job_id=446920191&repo=autoland&lineNumber=14127
https://treeherder.mozilla.org/logviewer?job_id=446920184&repo=autoland&lineNumber=5585
https://treeherder.mozilla.org/logviewer?job_id=446920535&repo=autoland&lineNumber=10268
and many more

Backout: https://hg.mozilla.org/integration/autoland/rev/36e8bac7c6f3c93cfbe1b2a209663b8e293bed9a

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/37ec3180a6cb
Clean up CompareTreePosition and related code. r=smaug

Comment 13

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/37ec3180a6cb

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot