Fix incorrect recursion for dir=auto special cases. #10561

dbaron · 2024-08-15T16:43:24Z

dir=auto has special-case behavior for both (1) auto-directionality form-associated elements and (2) slots with assigned nodes. In these special cases dir=auto is intended to have special behavior when used on the element, but this special behavior was not intended to apply when dir=auto was used on an ancestor.

The wording in the HTML spec for this behavior got this incorrect for at least one case, where the handling of the slot case needs recursion over the assigned nodes as though they were descendants. This was uncovered by the Gecko implementor adding a test for this case as described in https://bugzilla.mozilla.org/show_bug.cgi?id=1876163#c13 . I believe this was a mistake in the spec and is fixed in this change by splitting "contained text auto directionality" into a separate algorithm and invoking that, rather than the complete "auto directionality" algorithm, when handling an Element assigned to a slot.

This special case handling also incorrectly missed applying exclusions of descendants (for a different set of special cases) to the special traversal for computing dir=auto on a slot element, which traverses its assigned nodes (in order) rather than its descendants. This change also fixes that exclusion by adding a canExcludeRoot boolean to the "contained text auto directionality" algorithm.

This is a followup to PR #9796 (fixing #3699). Fixes #10488.

At least two implementers are interested (and none opposed):
- Chromium
- Mozilla (per Fix incorrect recursion for dir=auto special cases. #10561 (review) )
Tests are written and can be reviewed and commented upon at:
Implementation bugs are filed:
- Chromium: already fixed in crrev.com/c/5796604, crrev.com/c/5798082, and crrev.com/c/5798880
- Gecko: bug 1914062
- WebKit: existing bug 267951 should be sufficient since it hasn't been fixed yet
MDN issue is filed: existing document inheritance of HTML directionality and language and its relationship to shadow DOM mdn/mdn#455 should be sufficient
The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

Cc @vinhill who implemented this in Gecko and @nt1m who is implementing in WebKit.

Note that https://github.com/web-platform-tests/wpt/blob/b6bca1a1aa8042d6651dc8ac9a0541cd506377b2/html/dom/elements/global-attributes/dir-auto-dynamic-changes.window.js#L381 is the test that is testing the behavior that I believe was a mistake in the spec.

/dom.html ( diff )

dir=auto has special-case behavior for both (1) auto-directionality form-associated elements and (2) slots with assigned nodes. In these special cases dir=auto is intended to have special behavior when used on the element, but this special behavior was not intended to apply when dir=auto was used on an ancestor. The wording in the HTML spec for this behavior got this incorrect for at least one case, where the handling of the slot case needs recursion over the assigned nodes as though they were descendants. This was uncovered by the Gecko implementor adding a test for this case as described in https://bugzilla.mozilla.org/show_bug.cgi?id=1876163#c13 . I believe this was a mistake in the spec and should be fixed.

nt1m · 2024-08-15T17:00:47Z

cc @shallawa who's been taking over this work in WebKit

vinhill · 2024-08-15T17:19:05Z

I described more such cases in #10488 and my initial understanding is that this PR would fix these too. This sounds reasonable to me, but I'm no longer employed at Mozilla.

Therefore this PR impacts these tests that are currently passing in Chromium, so Chromiums implementation might not be fully compliant with this spec change.

dbaron · 2024-08-15T17:28:31Z

Ah, I'd forgotten about that. This doesn't fix all of the cases in #10488, since it doesn't apply the skipping rules in 3.1.

dbaron · 2024-08-15T17:56:15Z

I think I've updated this to fully fix #10488.

source

annevk

Thanks, looks good now.

dbaron · 2024-08-19T14:09:19Z

One further note here: there are effectively two sub-parts to the change:

skipping the excluded elements (bdi, script, style, textarea, element with dir attribute) when they're assigned nodes in a slot
not applying the special rules intended to affect dir=auto only on an element itself but not on ancestors (for auto-directionality form-associated elements and for slots) on the assigned nodes in a slot when computing dir=auto for the slot

Change (1) requires code updates in both Gecko and Chromium; change (2) matches current Chromium code but requires code updates in Gecko (and one WPT update).

I just wrote the Chromium code change for (1), so after I compile it I'll figure out what WPT updates are needed. Then I'll separately prepare the WPT updates for change (2) as well (which as far as current tests affect just one test where Gecko and Chromium disagree).

dbaron · 2024-08-19T17:12:11Z

I posted the first half and second half of the changes I mentioned; WPT PRs will be created automatically once the changes are reviewed. However, before that happens, I need to figure out how to fix an assertion failure that I triggered with one of the new tests I wrote.

smaug---- · 2024-08-19T17:29:51Z

source

+
+      <ul class="brief">
+       <li><var>descendant</var></li>
+       <li>any ancestor element of <var>descendant</var> that is a descendant of <var>element</var></li>


This is still very much a mouthful :) But I don't have good ideas how to make this easier to read.

This fixes Chromium code and web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in both spec and Chromium implementation) subtrees that were intended to be excluded from dir=auto traversal were not being excluded if they were an assigned node in a slot with dir=auto (which has a special traversal to traverse in the order of the assigned nodes, despite the general traversal being on the dom tree and not the flat tree). This fix is behind the DirAutoFixSlotExclusions flag in case we need to disable it. This also updates the test for bdi elements to only check for bdi elements in the HTML namespace. Bug: 576815, 352191350 Change-Id: I58a3bef05d27ba83a7e6b8e34d7bba74a8e0a482

This fixes Chromium code and web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in both spec and Chromium implementation) subtrees that were intended to be excluded from dir=auto traversal were not being excluded if they were an assigned node in a slot with dir=auto (which has a special traversal to traverse in the order of the assigned nodes, despite the general traversal being on the dom tree and not the flat tree). This fix is behind the DirAutoFixSlotExclusions flag in case we need to disable it. This also updates the test for bdi elements to only check for bdi elements in the HTML namespace. Bug: 576815, 352191350 Change-Id: I58a3bef05d27ba83a7e6b8e34d7bba74a8e0a482 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5796604 Reviewed-by: Di Zhang <dizhangg@chromium.org> Commit-Queue: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1343841}

…cestors This fixes web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in spec and Gecko, although not in the Chromium code) the special rules for dir=auto traversals that were intended to apply only to dir=auto on an element and not to dir=auto on its ancestors (relating to auto-directionality form-associated elements and to slots) were being applied in one ancestor case, where the special element was an assigned node of a slot. This makes the test changes corresponding to the fix to the HTML spec. Bug: 576815, 352191350 Change-Id: I6ffa8e22acc6f71777df1165655eaa1c8891867e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798082 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1343842}

This fixes Chromium code and web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in both spec and Chromium implementation) subtrees that were intended to be excluded from dir=auto traversal were not being excluded if they were an assigned node in a slot with dir=auto (which has a special traversal to traverse in the order of the assigned nodes, despite the general traversal being on the dom tree and not the flat tree). This fix is behind the DirAutoFixSlotExclusions flag in case we need to disable it. This also updates the test for bdi elements to only check for bdi elements in the HTML namespace. Bug: 576815, 352191350 Change-Id: I58a3bef05d27ba83a7e6b8e34d7bba74a8e0a482 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5796604 Reviewed-by: Di Zhang <dizhangg@chromium.org> Commit-Queue: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1343841}

…cestors This fixes web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in spec and Gecko, although not in the Chromium code) the special rules for dir=auto traversals that were intended to apply only to dir=auto on an element and not to dir=auto on its ancestors (relating to auto-directionality form-associated elements and to slots) were being applied in one ancestor case, where the special element was an assigned node of a slot. This makes the test changes corresponding to the fix to the HTML spec. Bug: 576815, 352191350 Change-Id: I6ffa8e22acc6f71777df1165655eaa1c8891867e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798082 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1343842}

This fixes Chromium code to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . I previously thought the code was fine, but I realize a small update is needed. In particular, the code could apply element-only rules for dir=auto to elements (in particular, button-type inputs) that are auto-directionality form-associated elements but were not in the list of exclusions. This also fixes the list of exclusions to exclude specifically textarea, so that any actual text content of input elements (but not the value) is traversed as required by the spec. This also removes the no-longer needed is_deferred argument to ResolveAutoDirectionality. Bug: 576815, 352191350 Change-Id: I7c13b9661d23f8904727c8298a8dca6360d6d732

dbaron · 2024-08-20T21:13:26Z

I think this is now ready to land. I've already merged most of the test changes and the rest should auto-merge within a few hours.

This fixes Chromium code to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . I previously thought the code was fine, but I realize a small update is needed. In particular, the code could apply element-only rules for dir=auto to elements (in particular, button-type inputs) that are auto-directionality form-associated elements but were not in the list of exclusions. This also fixes the list of exclusions to exclude specifically textarea, so that any actual text content of input elements (but not the value) is traversed as required by the spec. This also removes the no-longer needed is_deferred argument to ResolveAutoDirectionality. Bug: 576815, 352191350 Change-Id: I7c13b9661d23f8904727c8298a8dca6360d6d732 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798880 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Commit-Queue: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1344402}

This fixes Chromium code and web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in both spec and Chromium implementation) subtrees that were intended to be excluded from dir=auto traversal were not being excluded if they were an assigned node in a slot with dir=auto (which has a special traversal to traverse in the order of the assigned nodes, despite the general traversal being on the dom tree and not the flat tree). This fix is behind the DirAutoFixSlotExclusions flag in case we need to disable it. This also updates the test for bdi elements to only check for bdi elements in the HTML namespace. Bug: 576815, 352191350 Change-Id: I58a3bef05d27ba83a7e6b8e34d7bba74a8e0a482 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5796604 Reviewed-by: Di Zhang <dizhangg@chromium.org> Commit-Queue: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1343841}

…cestors This fixes web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in spec and Gecko, although not in the Chromium code) the special rules for dir=auto traversals that were intended to apply only to dir=auto on an element and not to dir=auto on its ancestors (relating to auto-directionality form-associated elements and to slots) were being applied in one ancestor case, where the special element was an assigned node of a slot. This makes the test changes corresponding to the fix to the HTML spec. Bug: 576815, 352191350 Change-Id: I6ffa8e22acc6f71777df1165655eaa1c8891867e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798082 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1343842}

This fixes Chromium code to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . I previously thought the code was fine, but I realize a small update is needed. In particular, the code could apply element-only rules for dir=auto to elements (in particular, button-type inputs) that are auto-directionality form-associated elements but were not in the list of exclusions. This also fixes the list of exclusions to exclude specifically textarea, so that any actual text content of input elements (but not the value) is traversed as required by the spec. This also removes the no-longer needed is_deferred argument to ResolveAutoDirectionality. Bug: 576815, 352191350 Change-Id: I7c13b9661d23f8904727c8298a8dca6360d6d732 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798880 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Commit-Queue: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1344402}

…ly to assigned nodes in a slot., a=testonly Automatic update from web-platform-tests Fix dir=auto traversal exclusions to apply to assigned nodes in a slot. This fixes Chromium code and web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in both spec and Chromium implementation) subtrees that were intended to be excluded from dir=auto traversal were not being excluded if they were an assigned node in a slot with dir=auto (which has a special traversal to traverse in the order of the assigned nodes, despite the general traversal being on the dom tree and not the flat tree). This fix is behind the DirAutoFixSlotExclusions flag in case we need to disable it. This also updates the test for bdi elements to only check for bdi elements in the HTML namespace. Bug: 576815, 352191350 Change-Id: I58a3bef05d27ba83a7e6b8e34d7bba74a8e0a482 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5796604 Reviewed-by: Di Zhang <dizhangg@chromium.org> Commit-Queue: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1343841} -- wpt-commits: 24bea5f38d235db99296614c07bcab105825b6f8 wpt-pr: 47670

…pply element-only rules to ancestors, a=testonly Automatic update from web-platform-tests Update dir=auto traversal tests to not apply element-only rules to ancestors This fixes web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in spec and Gecko, although not in the Chromium code) the special rules for dir=auto traversals that were intended to apply only to dir=auto on an element and not to dir=auto on its ancestors (relating to auto-directionality form-associated elements and to slots) were being applied in one ancestor case, where the special element was an assigned node of a slot. This makes the test changes corresponding to the fix to the HTML spec. Bug: 576815, 352191350 Change-Id: I6ffa8e22acc6f71777df1165655eaa1c8891867e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798082 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1343842} -- wpt-commits: fddc4be423e3243af4b3b4fb410361579ea97193 wpt-pr: 47678

…ent-only rules to ancestors, a=testonly Automatic update from web-platform-tests Fix dir=auto traversal to not apply element-only rules to ancestors This fixes Chromium code to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . I previously thought the code was fine, but I realize a small update is needed. In particular, the code could apply element-only rules for dir=auto to elements (in particular, button-type inputs) that are auto-directionality form-associated elements but were not in the list of exclusions. This also fixes the list of exclusions to exclude specifically textarea, so that any actual text content of input elements (but not the value) is traversed as required by the spec. This also removes the no-longer needed is_deferred argument to ResolveAutoDirectionality. Bug: 576815, 352191350 Change-Id: I7c13b9661d23f8904727c8298a8dca6360d6d732 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798880 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Commit-Queue: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1344402} -- wpt-commits: 5fd1516871dc70888b9895f887109c39713db03e wpt-pr: 47697

…ly to assigned nodes in a slot., a=testonly Automatic update from web-platform-tests Fix dir=auto traversal exclusions to apply to assigned nodes in a slot. This fixes Chromium code and web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in both spec and Chromium implementation) subtrees that were intended to be excluded from dir=auto traversal were not being excluded if they were an assigned node in a slot with dir=auto (which has a special traversal to traverse in the order of the assigned nodes, despite the general traversal being on the dom tree and not the flat tree). This fix is behind the DirAutoFixSlotExclusions flag in case we need to disable it. This also updates the test for bdi elements to only check for bdi elements in the HTML namespace. Bug: 576815, 352191350 Change-Id: I58a3bef05d27ba83a7e6b8e34d7bba74a8e0a482 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5796604 Reviewed-by: Di Zhang <dizhanggchromium.org> Commit-Queue: David Baron <dbaronchromium.org> Cr-Commit-Position: refs/heads/main{#1343841} -- wpt-commits: 24bea5f38d235db99296614c07bcab105825b6f8 wpt-pr: 47670 UltraBlame original commit: 5a235bc5aaa335bfffff109a441b242acdd3192c

…pply element-only rules to ancestors, a=testonly Automatic update from web-platform-tests Update dir=auto traversal tests to not apply element-only rules to ancestors This fixes web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in spec and Gecko, although not in the Chromium code) the special rules for dir=auto traversals that were intended to apply only to dir=auto on an element and not to dir=auto on its ancestors (relating to auto-directionality form-associated elements and to slots) were being applied in one ancestor case, where the special element was an assigned node of a slot. This makes the test changes corresponding to the fix to the HTML spec. Bug: 576815, 352191350 Change-Id: I6ffa8e22acc6f71777df1165655eaa1c8891867e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798082 Commit-Queue: David Baron <dbaronchromium.org> Reviewed-by: Di Zhang <dizhanggchromium.org> Cr-Commit-Position: refs/heads/main{#1343842} -- wpt-commits: fddc4be423e3243af4b3b4fb410361579ea97193 wpt-pr: 47678 UltraBlame original commit: 9594de798831efee3071b69a642ba03e168cf33f

…ly to assigned nodes in a slot., a=testonly Automatic update from web-platform-tests Fix dir=auto traversal exclusions to apply to assigned nodes in a slot. This fixes Chromium code and web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in both spec and Chromium implementation) subtrees that were intended to be excluded from dir=auto traversal were not being excluded if they were an assigned node in a slot with dir=auto (which has a special traversal to traverse in the order of the assigned nodes, despite the general traversal being on the dom tree and not the flat tree). This fix is behind the DirAutoFixSlotExclusions flag in case we need to disable it. This also updates the test for bdi elements to only check for bdi elements in the HTML namespace. Bug: 576815, 352191350 Change-Id: I58a3bef05d27ba83a7e6b8e34d7bba74a8e0a482 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5796604 Reviewed-by: Di Zhang <dizhanggchromium.org> Commit-Queue: David Baron <dbaronchromium.org> Cr-Commit-Position: refs/heads/main{#1343841} -- wpt-commits: 24bea5f38d235db99296614c07bcab105825b6f8 wpt-pr: 47670 UltraBlame original commit: 5a235bc5aaa335bfffff109a441b242acdd3192c

…pply element-only rules to ancestors, a=testonly Automatic update from web-platform-tests Update dir=auto traversal tests to not apply element-only rules to ancestors This fixes web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in spec and Gecko, although not in the Chromium code) the special rules for dir=auto traversals that were intended to apply only to dir=auto on an element and not to dir=auto on its ancestors (relating to auto-directionality form-associated elements and to slots) were being applied in one ancestor case, where the special element was an assigned node of a slot. This makes the test changes corresponding to the fix to the HTML spec. Bug: 576815, 352191350 Change-Id: I6ffa8e22acc6f71777df1165655eaa1c8891867e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798082 Commit-Queue: David Baron <dbaronchromium.org> Reviewed-by: Di Zhang <dizhanggchromium.org> Cr-Commit-Position: refs/heads/main{#1343842} -- wpt-commits: fddc4be423e3243af4b3b4fb410361579ea97193 wpt-pr: 47678 UltraBlame original commit: 9594de798831efee3071b69a642ba03e168cf33f

…ent-only rules to ancestors, a=testonly Automatic update from web-platform-tests Fix dir=auto traversal to not apply element-only rules to ancestors This fixes Chromium code to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . I previously thought the code was fine, but I realize a small update is needed. In particular, the code could apply element-only rules for dir=auto to elements (in particular, button-type inputs) that are auto-directionality form-associated elements but were not in the list of exclusions. This also fixes the list of exclusions to exclude specifically textarea, so that any actual text content of input elements (but not the value) is traversed as required by the spec. This also removes the no-longer needed is_deferred argument to ResolveAutoDirectionality. Bug: 576815, 352191350 Change-Id: I7c13b9661d23f8904727c8298a8dca6360d6d732 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798880 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Commit-Queue: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1344402} -- wpt-commits: 5fd1516871dc70888b9895f887109c39713db03e wpt-pr: 47697

…ly to assigned nodes in a slot., a=testonly Automatic update from web-platform-tests Fix dir=auto traversal exclusions to apply to assigned nodes in a slot. This fixes Chromium code and web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in both spec and Chromium implementation) subtrees that were intended to be excluded from dir=auto traversal were not being excluded if they were an assigned node in a slot with dir=auto (which has a special traversal to traverse in the order of the assigned nodes, despite the general traversal being on the dom tree and not the flat tree). This fix is behind the DirAutoFixSlotExclusions flag in case we need to disable it. This also updates the test for bdi elements to only check for bdi elements in the HTML namespace. Bug: 576815, 352191350 Change-Id: I58a3bef05d27ba83a7e6b8e34d7bba74a8e0a482 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5796604 Reviewed-by: Di Zhang <dizhangg@chromium.org> Commit-Queue: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1343841} -- wpt-commits: 24bea5f38d235db99296614c07bcab105825b6f8 wpt-pr: 47670

…pply element-only rules to ancestors, a=testonly Automatic update from web-platform-tests Update dir=auto traversal tests to not apply element-only rules to ancestors This fixes web platform tests to match half of the spec update in whatwg/html#10488 and whatwg/html#10561 . Prior to this change (in spec and Gecko, although not in the Chromium code) the special rules for dir=auto traversals that were intended to apply only to dir=auto on an element and not to dir=auto on its ancestors (relating to auto-directionality form-associated elements and to slots) were being applied in one ancestor case, where the special element was an assigned node of a slot. This makes the test changes corresponding to the fix to the HTML spec. Bug: 576815, 352191350 Change-Id: I6ffa8e22acc6f71777df1165655eaa1c8891867e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798082 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Di Zhang <dizhangg@chromium.org> Cr-Commit-Position: refs/heads/main@{#1343842} -- wpt-commits: fddc4be423e3243af4b3b4fb410361579ea97193 wpt-pr: 47678

smaug---- · 2024-09-16T19:24:27Z

source

+      <ul class="brief">
+       <li><var>descendant</var></li>
+       <li>any ancestor element of <var>descendant</var> that is a descendant of <var>element</var></li>
+       <li>if <var>canExcludeRoot</var> is true, <var>element</var></li>


Just realized, this is a really odd algorithm. One goes through all the descendants, but if element is one of the things below, continue. The behavior is ok, but this is quite hard to read. I can blame myself for not thinking this more while reviewing :)

Yeah, it's really intended to represent a tree traversal that excludes certain subtrees. However, we don't have good spec language for that. (Maybe we should write some!)

dbaron requested review from annevk and nt1m August 15, 2024 16:44

Also fix the exclusion clause.

e9dd789

annevk reviewed Aug 16, 2024

View reviewed changes

source Show resolved Hide resolved

source Show resolved Hide resolved

source Show resolved Hide resolved

annevk added the i18n-tracker Group bringing to attention of Internationalization, or tracked by i18n but not needing response. label Aug 16, 2024

dbaron added 2 commits August 16, 2024 16:20

Try to address first round of review comments from @annevk.

9bbe199

Also use data-x.

2049e18

dbaron requested a review from annevk August 16, 2024 20:35

w3cbot mentioned this pull request Aug 17, 2024

Fix incorrect recursion for dir=auto special cases. w3c/i18n-activity#1900

Closed

annevk approved these changes Aug 19, 2024

View reviewed changes

smaug---- reviewed Aug 19, 2024

View reviewed changes

smaug---- approved these changes Aug 19, 2024

View reviewed changes

chromium-wpt-export-bot mentioned this pull request Aug 19, 2024

Fix dir=auto traversal exclusions to apply to assigned nodes in a slot. web-platform-tests/wpt#47670

Merged

chromium-wpt-export-bot mentioned this pull request Aug 20, 2024

Update dir=auto traversal tests to not apply element-only rules to ancestors web-platform-tests/wpt#47678

Merged

chromium-wpt-export-bot mentioned this pull request Aug 20, 2024

Fix dir=auto traversal to not apply element-only rules to ancestors web-platform-tests/wpt#47697

Merged

domenic merged commit 5b0f145 into whatwg:main Aug 21, 2024
2 checks passed

domenic added the topic: shadow Relates to shadow trees (as defined in DOM) label Aug 21, 2024

smaug---- reviewed Sep 16, 2024

View reviewed changes

AtkinsSJ mentioned this pull request Sep 18, 2024

LibWeb: Update "compute the auto directionality" algorithm to match spec LadybirdBrowser/ladybird#1426

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix incorrect recursion for dir=auto special cases. #10561

Fix incorrect recursion for dir=auto special cases. #10561

dbaron commented Aug 15, 2024 •

edited

Loading

nt1m commented Aug 15, 2024

vinhill commented Aug 15, 2024

dbaron commented Aug 15, 2024 •

edited

Loading

dbaron commented Aug 15, 2024 •

edited

Loading

annevk left a comment

dbaron commented Aug 19, 2024

dbaron commented Aug 19, 2024

smaug---- Aug 19, 2024

dbaron commented Aug 20, 2024

smaug---- Sep 16, 2024 •

edited

Loading

dbaron Sep 16, 2024

Fix incorrect recursion for dir=auto special cases. #10561

Fix incorrect recursion for dir=auto special cases. #10561

Conversation

dbaron commented Aug 15, 2024 • edited Loading

nt1m commented Aug 15, 2024

vinhill commented Aug 15, 2024

dbaron commented Aug 15, 2024 • edited Loading

dbaron commented Aug 15, 2024 • edited Loading

annevk left a comment

Choose a reason for hiding this comment

dbaron commented Aug 19, 2024

dbaron commented Aug 19, 2024

smaug---- Aug 19, 2024

Choose a reason for hiding this comment

dbaron commented Aug 20, 2024

smaug---- Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

dbaron Sep 16, 2024

Choose a reason for hiding this comment

dbaron commented Aug 15, 2024 •

edited

Loading

dbaron commented Aug 15, 2024 •

edited

Loading

dbaron commented Aug 15, 2024 •

edited

Loading

smaug---- Sep 16, 2024 •

edited

Loading