Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the directionality account for missing parent element #9554

Merged
merged 6 commits into from
Aug 29, 2023
Merged

Conversation

annevk
Copy link
Member

@annevk annevk commented Jul 24, 2023

The current algorithm has no defined exit for when an element has no parent element and its dir attribute is in the undefined state.

And also rewrite it to make it easier to change as we want to account for shadow trees in a future version.

This was inspired by work from Brian Kardell and fantasai in this area.

Closes #9452.

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


/dom.html ( diff )

@annevk
Copy link
Member Author

annevk commented Jul 24, 2023

I found a test that hits this parent condition and fails in WebKit and Gecko: http://software.hixie.ch/utilities/js/live-dom-viewer/saved/11875. @emilio @rniwa what do you think? That should be fixed, right?

@bkardell @fantasai what do you think of this approach?

@annevk annevk added the i18n-tracker Group bringing to attention of Internationalization, or tracked by i18n but not needing response. label Jul 24, 2023
@annevk annevk mentioned this pull request Jul 24, 2023
4 tasks
@annevk annevk added needs tests Moving the issue forward requires someone to write tests needs implementer interest Moving the issue forward requires implementers to express interest labels Jul 24, 2023
@bkardell
Copy link
Contributor

Yes please. This is what I was aiming for initially, but better. It reads much more clearly and seems like it should be much easier to weave Shadow DOM bits into that. Thanks @annevk

source Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated
Comment on lines 13534 to 13538
<li><p>If <var>codePoint</var> is non-null and it is of bidirectional character type AL or R,
then return '<span data-x="concept-rtl">rtl</span>'.</p></li>

<p>Otherwise, if the element is a <span>document element</span>, <span>the directionality</span>
of the element is '<span data-x="concept-ltr">ltr</span>'.</p>
<li><p>If <var>codePoint</var> is non-null and it is of bidirectional character type L, then
return '<span data-x="concept-ltr">ltr</span>'.</p></li>
Copy link
Contributor

@fantasai fantasai Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would group these two rather than flatten them out, I think it helps structure the logic better have the LTR vs RTL fork grouped into a single step of the algorithm. But your call.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this. Have a separate callout algorithm for these two steps?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you restructured this bit, so it's less awkward now. :)

Might consider putting "If codepoint is ..." as a switch with 3 branches rather than 3 independent steps, though. They're not really ordered steps...

Copy link
Contributor

@fantasai fantasai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how this will work with the shadow DOM changes since codePoint will no longer necessarily be a codepoint. But in isolation this change seems fine? Some comments inline for bits that were hard to read.

@dbaron
Copy link
Member

dbaron commented Aug 11, 2023

I'm happy to see this moving along.

A few comments:

From #9554 (comment) :

I found a test that hits this parent condition and fails in WebKit and Gecko: http://software.hixie.ch/utilities/js/live-dom-viewer/saved/11875.

In both Gecko and WebKit, the behavior of that testcase seems to depend on the use of createElementNS to create a non-HTML element, rather than the lack of a parent. In both engines, appending the element to the body before matching doesn't change the behavior, whereas changing createElementNS to createElement does. (In the case of Gecko, I suspect this is because one chunk of relevant code lives in nsGenericHTMLElement::IntrinsicState().)


I notice that the PR preview is not up-to-date with the most recent commit to this PR; it would be nice if it were, but I don't know why it wouldn't be.


One thing that I find a little jarring is that there seems to be some confusion about whether definitions or algorithms are being invoked; one of @fantasai's review comments points to this. Maybe it's sort of the norm in some specs these days, but I find it a bit confusing. I'd expect a definition X to be invoked as "the X of E" whereas an algorithm Y to be invoked as "compute the Y given E" or maybe "compute the Y with E".

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 22, 2023
…cestors.

This change treats a <slot> element as being a strong character, of its
resolved directionality, when resolving dir=auto on its shadow tree
ancestor.

This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag
because we're hoping to ship that feature soon and it makes sense to
ship related changes to direction handling all at once rather than
piecemeal.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9166
whatwg/html#9452
whatwg/html#9554

This fixes the failures of:
external/wpt/shadow-dom/directionality/dir-shadow-30.html
external/wpt/shadow-dom/directionality/dir-shadow-34.html
in the still-unlanded WPT PR at
#29820

This also changes the existing WPT
html/dom/elements/global-attributes/dir-slots-directionality.tentative.html
in the following ways:
 * split the test into separate test() functions to get separate results
 * add a sixth test testing <slot dir=auto></slot>
 * add tests of the :dir() selector for each test (where Chromium fails
   this test for test 1)
 * change the expected result of the fourth test to match this code
   change and the proposed specification

Bug: 576815
Change-Id: I83551e9bc5807109c5318bace486cfc93fc25bbb
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 22, 2023
…cestors.

This change treats a <slot> element as being a strong character, of its
resolved directionality, when resolving dir=auto on its shadow tree
ancestor.

This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag
because we're hoping to ship that feature soon and it makes sense to
ship related changes to direction handling all at once rather than
piecemeal.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9166
whatwg/html#9452
whatwg/html#9554

This fixes the failures of:
external/wpt/shadow-dom/directionality/dir-shadow-30.html
external/wpt/shadow-dom/directionality/dir-shadow-34.html
in the still-unlanded WPT PR at
#29820

This also changes the existing WPT
html/dom/elements/global-attributes/dir-slots-directionality.tentative.html
in the following ways:
 * split the test into separate test() functions to get separate results
 * add a sixth test testing <slot dir=auto></slot>
 * add tests of the :dir() selector for each test (where Chromium fails
   this test for test 1)
 * change the expected result of the fourth test to match this code
   change and the proposed specification

Bug: 576815
Change-Id: I83551e9bc5807109c5318bace486cfc93fc25bbb
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 22, 2023
…cestors.

This change treats a <slot> element as being a strong character, of its
resolved directionality, when resolving dir=auto on its shadow tree
ancestor.

This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag
because we're hoping to ship that feature soon and it makes sense to
ship related changes to direction handling all at once rather than
piecemeal.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9166
whatwg/html#9452
whatwg/html#9554

This fixes the failures of:
external/wpt/shadow-dom/directionality/dir-shadow-30.html
external/wpt/shadow-dom/directionality/dir-shadow-34.html
in the still-unlanded WPT PR at
#29820

This also changes the existing WPT
html/dom/elements/global-attributes/dir-slots-directionality.tentative.html
in the following ways:
 * split the test into separate test() functions to get separate results
 * add a sixth test testing <slot dir=auto></slot>
 * add tests of the :dir() selector for each test (where Chromium fails
   this test for test 1)
 * change the expected result of the fourth test to match this code
   change and the proposed specification

Bug: 576815
Change-Id: I83551e9bc5807109c5318bace486cfc93fc25bbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800366
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1186743}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 22, 2023
…cestors.

This change treats a <slot> element as being a strong character, of its
resolved directionality, when resolving dir=auto on its shadow tree
ancestor.

This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag
because we're hoping to ship that feature soon and it makes sense to
ship related changes to direction handling all at once rather than
piecemeal.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9166
whatwg/html#9452
whatwg/html#9554

This fixes the failures of:
external/wpt/shadow-dom/directionality/dir-shadow-30.html
external/wpt/shadow-dom/directionality/dir-shadow-34.html
in the still-unlanded WPT PR at
#29820

This also changes the existing WPT
html/dom/elements/global-attributes/dir-slots-directionality.tentative.html
in the following ways:
 * split the test into separate test() functions to get separate results
 * add a sixth test testing <slot dir=auto></slot>
 * add tests of the :dir() selector for each test (where Chromium fails
   this test for test 1)
 * change the expected result of the fourth test to match this code
   change and the proposed specification

Bug: 576815
Change-Id: I83551e9bc5807109c5318bace486cfc93fc25bbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800366
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1186743}
The current algorithm has no defined exit for when an element has no parent element and its dir attribute is in the undefined state.

And also rewrite it to make it easier to change as we want to account for shadow trees in a future version.

This was inspired by work from Brian Kardell and fantasai in this area.

Closes #9452.
annevk added a commit to web-platform-tests/wpt that referenced this pull request Aug 24, 2023
@annevk annevk removed needs implementer interest Moving the issue forward requires implementers to express interest needs tests Moving the issue forward requires someone to write tests labels Aug 24, 2023
aarongable pushed a commit to chromium/chromium that referenced this pull request Oct 24, 2023
This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.

This implements a part of the behavior described in:
whatwg/html#3699 (comment)
which has been specified in:
whatwg/html#9554
whatwg/html#9796

Bug: 576815
Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1214439}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 24, 2023
This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.

This implements a part of the behavior described in:
whatwg/html#3699 (comment)
which has been specified in:
whatwg/html#9554
whatwg/html#9796

Bug: 576815
Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1214439}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 24, 2023
This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.

This implements a part of the behavior described in:
whatwg/html#3699 (comment)
which has been specified in:
whatwg/html#9554
whatwg/html#9796

Bug: 576815
Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1214439}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 25, 2023
This reverts commit 198d41cfd6eac5fd9ee34e6314e34f938b6d31b5.

Reason for revert: This reverts a change that was the first of two closely related changes.  Given that the other change, in https://crrev.com/c/4973701, turned out to have problems (and I haven't attempted to land it), I've proposed reverting both in the HTML spec at whatwg/html#9880.  I'm therefore reverting this (the first half) until the HTML spec discussion is sorted out.

Original change's description:
> Give <slot> element default unicode-bidi: isolate style.
>
> This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.
>
> This implements a part of the behavior described in:
> whatwg/html#3699 (comment)
> which has been specified in:
> whatwg/html#9554
> whatwg/html#9796
>
> Bug: 576815
> Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Reviewed-by: Di Zhang <dizhangg@chromium.org>
> Commit-Queue: David Baron <dbaron@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1214439}

Bug: 576815
Change-Id: I0f52b30c345d4962a36a36e5d2651221db96bcde
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
aarongable pushed a commit to chromium/chromium that referenced this pull request Oct 25, 2023
This reverts commit 198d41c.

Reason for revert: This reverts a change that was the first of two closely related changes.  Given that the other change, in https://crrev.com/c/4973701, turned out to have problems (and I haven't attempted to land it), I've proposed reverting both in the HTML spec at whatwg/html#9880.  I'm therefore reverting this (the first half) until the HTML spec discussion is sorted out.

Original change's description:
> Give <slot> element default unicode-bidi: isolate style.
>
> This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.
>
> This implements a part of the behavior described in:
> whatwg/html#3699 (comment)
> which has been specified in:
> whatwg/html#9554
> whatwg/html#9796
>
> Bug: 576815
> Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Reviewed-by: Di Zhang <dizhangg@chromium.org>
> Commit-Queue: David Baron <dbaron@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1214439}

Bug: 576815
Change-Id: I0f52b30c345d4962a36a36e5d2651221db96bcde
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974023
Reviewed-by: Philip Rogers <pdr@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: David Baron <dbaron@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1215009}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 25, 2023
This reverts commit 198d41cfd6eac5fd9ee34e6314e34f938b6d31b5.

Reason for revert: This reverts a change that was the first of two closely related changes.  Given that the other change, in https://crrev.com/c/4973701, turned out to have problems (and I haven't attempted to land it), I've proposed reverting both in the HTML spec at whatwg/html#9880.  I'm therefore reverting this (the first half) until the HTML spec discussion is sorted out.

Original change's description:
> Give <slot> element default unicode-bidi: isolate style.
>
> This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.
>
> This implements a part of the behavior described in:
> whatwg/html#3699 (comment)
> which has been specified in:
> whatwg/html#9554
> whatwg/html#9796
>
> Bug: 576815
> Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Reviewed-by: Di Zhang <dizhangg@chromium.org>
> Commit-Queue: David Baron <dbaron@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1214439}

Bug: 576815
Change-Id: I0f52b30c345d4962a36a36e5d2651221db96bcde
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974023
Reviewed-by: Philip Rogers <pdr@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: David Baron <dbaron@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1215009}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 25, 2023
This reverts commit 198d41cfd6eac5fd9ee34e6314e34f938b6d31b5.

Reason for revert: This reverts a change that was the first of two closely related changes.  Given that the other change, in https://crrev.com/c/4973701, turned out to have problems (and I haven't attempted to land it), I've proposed reverting both in the HTML spec at whatwg/html#9880.  I'm therefore reverting this (the first half) until the HTML spec discussion is sorted out.

Original change's description:
> Give <slot> element default unicode-bidi: isolate style.
>
> This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.
>
> This implements a part of the behavior described in:
> whatwg/html#3699 (comment)
> which has been specified in:
> whatwg/html#9554
> whatwg/html#9796
>
> Bug: 576815
> Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Reviewed-by: Di Zhang <dizhangg@chromium.org>
> Commit-Queue: David Baron <dbaron@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1214439}

Bug: 576815
Change-Id: I0f52b30c345d4962a36a36e5d2651221db96bcde
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974023
Reviewed-by: Philip Rogers <pdr@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: David Baron <dbaron@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1215009}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 26, 2023
…auto and descendant directionality to consider non-HTML elements., a=testonly

Automatic update from web-platform-tests
Fix :dir() selector and updates for dir=auto and descendant directionality to consider non-HTML elements.

This changes behavior only when the CSSPseudoDir flag is enabled.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9554
whatwg/html#9796
and on the behavior specified in:
https://drafts.csswg.org/selectors-4/#the-dir-pseudo

Bug: 576815
Change-Id: I57323aeda8850f382756cd36b3717d34e8911f5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4908695
Commit-Queue: David Baron <dbaron@chromium.org>
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1204886}

--

wpt-commits: 9c46bae54706a175a99a9f127a4a8065704c2cc2
wpt-pr: 42315
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this pull request Oct 27, 2023
…auto and descendant directionality to consider non-HTML elements., a=testonly

Automatic update from web-platform-tests
Fix :dir() selector and updates for dir=auto and descendant directionality to consider non-HTML elements.

This changes behavior only when the CSSPseudoDir flag is enabled.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9554
whatwg/html#9796
and on the behavior specified in:
https://drafts.csswg.org/selectors-4/#the-dir-pseudo

Bug: 576815
Change-Id: I57323aeda8850f382756cd36b3717d34e8911f5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4908695
Commit-Queue: David Baron <dbaron@chromium.org>
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1204886}

--

wpt-commits: 9c46bae54706a175a99a9f127a4a8065704c2cc2
wpt-pr: 42315
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 30, 2023
…auto and descendant directionality to consider non-HTML elements., a=testonly

Automatic update from web-platform-tests
Fix :dir() selector and updates for dir=auto and descendant directionality to consider non-HTML elements.

This changes behavior only when the CSSPseudoDir flag is enabled.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9554
whatwg/html#9796
and on the behavior specified in:
https://drafts.csswg.org/selectors-4/#the-dir-pseudo

Bug: 576815
Change-Id: I57323aeda8850f382756cd36b3717d34e8911f5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4908695
Commit-Queue: David Baron <dbaronchromium.org>
Reviewed-by: Di Zhang <dizhanggchromium.org>
Cr-Commit-Position: refs/heads/main{#1204886}

--

wpt-commits: 9c46bae54706a175a99a9f127a4a8065704c2cc2
wpt-pr: 42315

UltraBlame original commit: 28cd492bf06128adefbd727f510e3773c4c531ab
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 30, 2023
…auto and descendant directionality to consider non-HTML elements., a=testonly

Automatic update from web-platform-tests
Fix :dir() selector and updates for dir=auto and descendant directionality to consider non-HTML elements.

This changes behavior only when the CSSPseudoDir flag is enabled.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9554
whatwg/html#9796
and on the behavior specified in:
https://drafts.csswg.org/selectors-4/#the-dir-pseudo

Bug: 576815
Change-Id: I57323aeda8850f382756cd36b3717d34e8911f5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4908695
Commit-Queue: David Baron <dbaronchromium.org>
Reviewed-by: Di Zhang <dizhanggchromium.org>
Cr-Commit-Position: refs/heads/main{#1204886}

--

wpt-commits: 9c46bae54706a175a99a9f127a4a8065704c2cc2
wpt-pr: 42315

UltraBlame original commit: 28cd492bf06128adefbd727f510e3773c4c531ab
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 30, 2023
…auto and descendant directionality to consider non-HTML elements., a=testonly

Automatic update from web-platform-tests
Fix :dir() selector and updates for dir=auto and descendant directionality to consider non-HTML elements.

This changes behavior only when the CSSPseudoDir flag is enabled.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9554
whatwg/html#9796
and on the behavior specified in:
https://drafts.csswg.org/selectors-4/#the-dir-pseudo

Bug: 576815
Change-Id: I57323aeda8850f382756cd36b3717d34e8911f5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4908695
Commit-Queue: David Baron <dbaronchromium.org>
Reviewed-by: Di Zhang <dizhanggchromium.org>
Cr-Commit-Position: refs/heads/main{#1204886}

--

wpt-commits: 9c46bae54706a175a99a9f127a4a8065704c2cc2
wpt-pr: 42315

UltraBlame original commit: 28cd492bf06128adefbd727f510e3773c4c531ab
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 7, 2023
…: isolate style., a=testonly

Automatic update from web-platform-tests
Give <slot> element default unicode-bidi: isolate style.

This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.

This implements a part of the behavior described in:
whatwg/html#3699 (comment)
which has been specified in:
whatwg/html#9554
whatwg/html#9796

Bug: 576815
Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1214439}

--

wpt-commits: a79306e06659bde55be85c3af5c1d7d4a51c0cbd
wpt-pr: 42717
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 7, 2023
…ode-bidi: isolate style.", a=testonly

Automatic update from web-platform-tests
Revert "Give <slot> element default unicode-bidi: isolate style."

This reverts commit 198d41cfd6eac5fd9ee34e6314e34f938b6d31b5.

Reason for revert: This reverts a change that was the first of two closely related changes.  Given that the other change, in https://crrev.com/c/4973701, turned out to have problems (and I haven't attempted to land it), I've proposed reverting both in the HTML spec at whatwg/html#9880.  I'm therefore reverting this (the first half) until the HTML spec discussion is sorted out.

Original change's description:
> Give <slot> element default unicode-bidi: isolate style.
>
> This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.
>
> This implements a part of the behavior described in:
> whatwg/html#3699 (comment)
> which has been specified in:
> whatwg/html#9554
> whatwg/html#9796
>
> Bug: 576815
> Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Reviewed-by: Di Zhang <dizhangg@chromium.org>
> Commit-Queue: David Baron <dbaron@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1214439}

Bug: 576815
Change-Id: I0f52b30c345d4962a36a36e5d2651221db96bcde
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974023
Reviewed-by: Philip Rogers <pdr@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: David Baron <dbaron@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1215009}

--

wpt-commits: 8cc01fc73b14dcbc92109574c26d85b25f5cfba0
wpt-pr: 42746
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this pull request Nov 7, 2023
…: isolate style., a=testonly

Automatic update from web-platform-tests
Give <slot> element default unicode-bidi: isolate style.

This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.

This implements a part of the behavior described in:
whatwg/html#3699 (comment)
which has been specified in:
whatwg/html#9554
whatwg/html#9796

Bug: 576815
Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1214439}

--

wpt-commits: a79306e06659bde55be85c3af5c1d7d4a51c0cbd
wpt-pr: 42717
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this pull request Nov 7, 2023
…ode-bidi: isolate style.", a=testonly

Automatic update from web-platform-tests
Revert "Give <slot> element default unicode-bidi: isolate style."

This reverts commit 198d41cfd6eac5fd9ee34e6314e34f938b6d31b5.

Reason for revert: This reverts a change that was the first of two closely related changes.  Given that the other change, in https://crrev.com/c/4973701, turned out to have problems (and I haven't attempted to land it), I've proposed reverting both in the HTML spec at whatwg/html#9880.  I'm therefore reverting this (the first half) until the HTML spec discussion is sorted out.

Original change's description:
> Give <slot> element default unicode-bidi: isolate style.
>
> This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.
>
> This implements a part of the behavior described in:
> whatwg/html#3699 (comment)
> which has been specified in:
> whatwg/html#9554
> whatwg/html#9796
>
> Bug: 576815
> Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Reviewed-by: Di Zhang <dizhangg@chromium.org>
> Commit-Queue: David Baron <dbaron@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1214439}

Bug: 576815
Change-Id: I0f52b30c345d4962a36a36e5d2651221db96bcde
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974023
Reviewed-by: Philip Rogers <pdr@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: David Baron <dbaron@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1215009}

--

wpt-commits: 8cc01fc73b14dcbc92109574c26d85b25f5cfba0
wpt-pr: 42746
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Nov 8, 2023
…: isolate style., a=testonly

Automatic update from web-platform-tests
Give <slot> element default unicode-bidi: isolate style.

This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.

This implements a part of the behavior described in:
whatwg/html#3699 (comment)
which has been specified in:
whatwg/html#9554
whatwg/html#9796

Bug: 576815
Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
Reviewed-by: Chris Harrelson <chrishtrchromium.org>
Reviewed-by: Di Zhang <dizhanggchromium.org>
Commit-Queue: David Baron <dbaronchromium.org>
Cr-Commit-Position: refs/heads/main{#1214439}

--

wpt-commits: a79306e06659bde55be85c3af5c1d7d4a51c0cbd
wpt-pr: 42717

UltraBlame original commit: c1d0594b7c2341b1247e69987405d8038e57308b
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Nov 8, 2023
…ode-bidi: isolate style.", a=testonly

Automatic update from web-platform-tests
Revert "Give <slot> element default unicode-bidi: isolate style."

This reverts commit 198d41cfd6eac5fd9ee34e6314e34f938b6d31b5.

Reason for revert: This reverts a change that was the first of two closely related changes.  Given that the other change, in https://crrev.com/c/4973701, turned out to have problems (and I haven't attempted to land it), I've proposed reverting both in the HTML spec at whatwg/html#9880.  I'm therefore reverting this (the first half) until the HTML spec discussion is sorted out.

Original change's description:
> Give <slot> element default unicode-bidi: isolate style.
>
> This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.
>
> This implements a part of the behavior described in:
> whatwg/html#3699 (comment)
> which has been specified in:
> whatwg/html#9554
> whatwg/html#9796
>
> Bug: 576815
> Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
> Reviewed-by: Chris Harrelson <chrishtrchromium.org>
> Reviewed-by: Di Zhang <dizhanggchromium.org>
> Commit-Queue: David Baron <dbaronchromium.org>
> Cr-Commit-Position: refs/heads/main{#1214439}

Bug: 576815
Change-Id: I0f52b30c345d4962a36a36e5d2651221db96bcde
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974023
Reviewed-by: Philip Rogers <pdrchromium.org>
Bot-Commit: Rubber Stamper <rubber-stamperappspot.gserviceaccount.com>
Auto-Submit: David Baron <dbaronchromium.org>
Commit-Queue: Philip Rogers <pdrchromium.org>
Cr-Commit-Position: refs/heads/main{#1215009}

--

wpt-commits: 8cc01fc73b14dcbc92109574c26d85b25f5cfba0
wpt-pr: 42746

UltraBlame original commit: 4f46a8fcb456fa55fd784b0714da59ad7b206c79
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Nov 8, 2023
…: isolate style., a=testonly

Automatic update from web-platform-tests
Give <slot> element default unicode-bidi: isolate style.

This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.

This implements a part of the behavior described in:
whatwg/html#3699 (comment)
which has been specified in:
whatwg/html#9554
whatwg/html#9796

Bug: 576815
Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
Reviewed-by: Chris Harrelson <chrishtrchromium.org>
Reviewed-by: Di Zhang <dizhanggchromium.org>
Commit-Queue: David Baron <dbaronchromium.org>
Cr-Commit-Position: refs/heads/main{#1214439}

--

wpt-commits: a79306e06659bde55be85c3af5c1d7d4a51c0cbd
wpt-pr: 42717

UltraBlame original commit: c1d0594b7c2341b1247e69987405d8038e57308b
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Nov 8, 2023
…ode-bidi: isolate style.", a=testonly

Automatic update from web-platform-tests
Revert "Give <slot> element default unicode-bidi: isolate style."

This reverts commit 198d41cfd6eac5fd9ee34e6314e34f938b6d31b5.

Reason for revert: This reverts a change that was the first of two closely related changes.  Given that the other change, in https://crrev.com/c/4973701, turned out to have problems (and I haven't attempted to land it), I've proposed reverting both in the HTML spec at whatwg/html#9880.  I'm therefore reverting this (the first half) until the HTML spec discussion is sorted out.

Original change's description:
> Give <slot> element default unicode-bidi: isolate style.
>
> This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.
>
> This implements a part of the behavior described in:
> whatwg/html#3699 (comment)
> which has been specified in:
> whatwg/html#9554
> whatwg/html#9796
>
> Bug: 576815
> Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
> Reviewed-by: Chris Harrelson <chrishtrchromium.org>
> Reviewed-by: Di Zhang <dizhanggchromium.org>
> Commit-Queue: David Baron <dbaronchromium.org>
> Cr-Commit-Position: refs/heads/main{#1214439}

Bug: 576815
Change-Id: I0f52b30c345d4962a36a36e5d2651221db96bcde
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974023
Reviewed-by: Philip Rogers <pdrchromium.org>
Bot-Commit: Rubber Stamper <rubber-stamperappspot.gserviceaccount.com>
Auto-Submit: David Baron <dbaronchromium.org>
Commit-Queue: Philip Rogers <pdrchromium.org>
Cr-Commit-Position: refs/heads/main{#1215009}

--

wpt-commits: 8cc01fc73b14dcbc92109574c26d85b25f5cfba0
wpt-pr: 42746

UltraBlame original commit: 4f46a8fcb456fa55fd784b0714da59ad7b206c79
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Nov 8, 2023
…: isolate style., a=testonly

Automatic update from web-platform-tests
Give <slot> element default unicode-bidi: isolate style.

This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.

This implements a part of the behavior described in:
whatwg/html#3699 (comment)
which has been specified in:
whatwg/html#9554
whatwg/html#9796

Bug: 576815
Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
Reviewed-by: Chris Harrelson <chrishtrchromium.org>
Reviewed-by: Di Zhang <dizhanggchromium.org>
Commit-Queue: David Baron <dbaronchromium.org>
Cr-Commit-Position: refs/heads/main{#1214439}

--

wpt-commits: a79306e06659bde55be85c3af5c1d7d4a51c0cbd
wpt-pr: 42717

UltraBlame original commit: c1d0594b7c2341b1247e69987405d8038e57308b
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Nov 8, 2023
…ode-bidi: isolate style.", a=testonly

Automatic update from web-platform-tests
Revert "Give <slot> element default unicode-bidi: isolate style."

This reverts commit 198d41cfd6eac5fd9ee34e6314e34f938b6d31b5.

Reason for revert: This reverts a change that was the first of two closely related changes.  Given that the other change, in https://crrev.com/c/4973701, turned out to have problems (and I haven't attempted to land it), I've proposed reverting both in the HTML spec at whatwg/html#9880.  I'm therefore reverting this (the first half) until the HTML spec discussion is sorted out.

Original change's description:
> Give <slot> element default unicode-bidi: isolate style.
>
> This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag.
>
> This implements a part of the behavior described in:
> whatwg/html#3699 (comment)
> which has been specified in:
> whatwg/html#9554
> whatwg/html#9796
>
> Bug: 576815
> Change-Id: Ice471838c7251fc69d87abfa723f3eef1a4c455a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800075
> Reviewed-by: Chris Harrelson <chrishtrchromium.org>
> Reviewed-by: Di Zhang <dizhanggchromium.org>
> Commit-Queue: David Baron <dbaronchromium.org>
> Cr-Commit-Position: refs/heads/main{#1214439}

Bug: 576815
Change-Id: I0f52b30c345d4962a36a36e5d2651221db96bcde
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974023
Reviewed-by: Philip Rogers <pdrchromium.org>
Bot-Commit: Rubber Stamper <rubber-stamperappspot.gserviceaccount.com>
Auto-Submit: David Baron <dbaronchromium.org>
Commit-Queue: Philip Rogers <pdrchromium.org>
Cr-Commit-Position: refs/heads/main{#1215009}

--

wpt-commits: 8cc01fc73b14dcbc92109574c26d85b25f5cfba0
wpt-pr: 42746

UltraBlame original commit: 4f46a8fcb456fa55fd784b0714da59ad7b206c79
Lightning00Blade pushed a commit to Lightning00Blade/wpt that referenced this pull request Dec 11, 2023
…cestors.

This change treats a <slot> element as being a strong character, of its
resolved directionality, when resolving dir=auto on its shadow tree
ancestor.

This is behind the RuntimeEnabledFeatures::CSSPseudoDirEnabled() flag
because we're hoping to ship that feature soon and it makes sense to
ship related changes to direction handling all at once rather than
piecemeal.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9166
whatwg/html#9452
whatwg/html#9554

This fixes the failures of:
external/wpt/shadow-dom/directionality/dir-shadow-30.html
external/wpt/shadow-dom/directionality/dir-shadow-34.html
in the still-unlanded WPT PR at
web-platform-tests#29820

This also changes the existing WPT
html/dom/elements/global-attributes/dir-slots-directionality.tentative.html
in the following ways:
 * split the test into separate test() functions to get separate results
 * add a sixth test testing <slot dir=auto></slot>
 * add tests of the :dir() selector for each test (where Chromium fails
   this test for test 1)
 * change the expected result of the fourth test to match this code
   change and the proposed specification

Bug: 576815
Change-Id: I83551e9bc5807109c5318bace486cfc93fc25bbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4800366
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1186743}
Lightning00Blade pushed a commit to Lightning00Blade/wpt that referenced this pull request Dec 11, 2023
Lightning00Blade pushed a commit to Lightning00Blade/wpt that referenced this pull request Dec 11, 2023
…or dir=auto

This rewrites significant aspects of inheritance of HTML direction
(which affects the unshipped :dir() selector and also the shipped
dirname attribute), including its invalidation, to match current spec
proposals regarding how the inheritance operates on Shadow DOM, and to
improve invalidation in response to dynamic changes.  Inheritance of
direction now operates on the node tree, except that shadow roots and
slots both inherit from the shadow host.  It previously operated on the
flat tree.  This change should not affect the computed values of the CSS
direction property, since the HTML direction is only mapped to CSS
direction on elements where the HTML direction is not inherited.

This also restructures the shadow tree-related invalidation code for
dir=auto to match the changes implemented in
https://crrev.com/26b1b30268b7af4f0e44f298c10338a65e656f40 , which made
dir=auto operate on the node tree, and the additional behavior
implemented in
https://crrev.com/6abc9cbde67c0700be364c7aab86cb29188c7d5d that
implemented special rules (looking at slotted children) for <slot
dir=auto>.  Certain text-like form controls also have similar special
rules in which they look at their value.  This change can affect the
computed values of the CSS direction property when the direction
computed by dir=auto is different.

The invalidation code for these two distinct things was (in the old
code) too tied together to cleanly separate these changes.

The reason these changes are the next step of work is because they
change (when CSSPseudoDirEnabled) the one caller that passed a
stay_within parameter to CalculateAndAdjustAutoDirectionality that is
different from this; this enables further (smaller) refactoring that I
believe will be needed to fix the failure of
external/wpt/shadow-dom/directionality/dir-shadow-41.html in the
still-unlanded WPT PR at
web-platform-tests#29820 .  I believe this
existing using of stay_within does not make sense; it doesn't make sense
to try to partially traverse the descendants of a dir=auto element with
a different termination point.  The traversal of a dir=auto element
should always start at its start and should terminate at the first
strong character or the end of the contents that should influence the
dir=auto.  (There are cases where the interaction of the stay_within and
the use of NodeTraversal/FlatTreeTraversal meant that the stay_within
parameter was missed entirely and the tree was searched beyond the
contents that should be; this is a step towards the refactoring needed
to fix that.)

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9166
whatwg/html#9452
whatwg/html#9554

Bug: 576815
Change-Id: Ic31a3f801f64042a3b4979afdc4e141f45e3b228
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4811757
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1199647}
Lightning00Blade pushed a commit to Lightning00Blade/wpt that referenced this pull request Dec 11, 2023
When dir=auto fails to find text with strong directionality (or a <slot>
element) to determine the directionality of the element, it should fall
back to the directionality that it would have inherited from its parent
(or, for <slot>, its shadow host) rather than falling back to ltr.

This requires updating directionality invalidation to account for the
possibility that elements with dir=auto can inherit directionality.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9554
whatwg/html#9796

This is one of two changes needed to fix the failure of:
external/wpt/html/dom/elements/global-attributes/dir-shadow-41.html
in the still-unlanded WPT PR at
web-platform-tests#29820

Bug: 576815
Change-Id: I9fc7c85875074ad41704ab45ec70c7632c3f8d31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4805163
Commit-Queue: David Baron <dbaron@chromium.org>
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1202893}
Lightning00Blade pushed a commit to Lightning00Blade/wpt that referenced this pull request Dec 11, 2023
…ality to consider non-HTML elements.

This changes behavior only when the CSSPseudoDir flag is enabled.

This is based on the proposed behavior described in:
whatwg/html#3699 (comment)
which is in the process of being specified in:
whatwg/html#9554
whatwg/html#9796
and on the behavior specified in:
https://drafts.csswg.org/selectors-4/#the-dir-pseudo

Bug: 576815
Change-Id: I57323aeda8850f382756cd36b3717d34e8911f5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4908695
Commit-Queue: David Baron <dbaron@chromium.org>
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1204886}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n-tracker Group bringing to attention of Internationalization, or tracked by i18n but not needing response.
Development

Successfully merging this pull request may close these issues.

6 participants