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

Tests for directionality and shadow DOM #29820

Merged
merged 28 commits into from
Oct 6, 2023

Conversation

meyerweb
Copy link
Contributor

40 tests and equivalent rendering references, with the tests adapted from various links posted in whatwg-html issue #3699.

@meyerweb
Copy link
Contributor Author

meyerweb commented Aug 9, 2021

@bkardell pointed out that Safari is failing these tests even though it really shouldn’t, due to the use of :dir(). See, for example, this test result which (if you disable the diff highlighting) shows Safari would have passed except for the generated content that only gets changed if :dir() is supported.

One outcome of the most recent WHAT WG triage meeting was that these tests need to be moved to a different area of WPT anyway, so I’ll bundle the test rework with the move to the different area. Stay tuned.

@meyerweb meyerweb changed the title Tests for :dir() and shadow DOM Tests for directionality and shadow DOM Aug 9, 2021
@meyerweb
Copy link
Contributor Author

meyerweb commented Aug 9, 2021

The tests should now only be reliant on implementations’ Shadow DOM and directionality behavior, plus a scrap of very basic JS. They’ve also been relocated as requested by WHAT WG. Absent any errors of assertion in the tests themselves (e.g., expecting LTR where it should be RTL), I believe these tests are ready.

@bkardell
Copy link
Contributor

bkardell commented Aug 11, 2021

Just summing up where things stand to hopefully make it easier to focus on where we need discussion/desictions. We have here 40 tests which set up various scenarios for shadow DOM and test the dir reported by getComputedStyle on some element. Of these, as of today..

  • 31/40 tests where we have tests/interop agreement from the 3 engines (1,2,3,4,5,7,8, 9,10,11,12,13,14,15,16,17,19,20,21,22,23,25,26,27,28,29,31,32,35,37,40)
  • 7 more where we have agreement in 2 engines and the test, but Safari disagrees (18, 24, 30, 33, 34, 38, 39).
    • I think the first question is can we verify that these are just bugs and move them into the "done" colum, or is there some actually disagreement? @rniwa ? If we can agree these lone holdouts are bugs, we're all the way up to agreeing on the results of 37/40 tests and a long way to interop on them....

Finally, what this leaves for more difficult challenges is only 2 items... On both 6 and 36, both FF and Safari currently disagree with Chrome and the test. Which is right, and why? These harder ones are:

1. dir-shadow-06.html

This is using dir=auto on the slot element and a paragraph inside containing some hebrew text. The test checked the computed style of the paragraph projected into the slot. Chrome and the test suggest this should be rtl, FF and Safari report LTR.

2. dir-shadow-36.html  

A host with one child slot. The slot itself has dir=auto and some default hebrew text, but projected into it is a text node with an english word. The test operates by calling getComputedStyle on the slot. Chrome and the test believe the correct answer is LTR, but FF and Safari say it is RTL.

@meyerweb
Copy link
Contributor Author

In addition to @bkardell’s fantastic summary of the test results, I want to add what I understand from the test assertions themselves, so we can find out if they’re the behaviors people actually want. In summary:

Explicit dir values on shadow hosts, slots, or elements that are slotted will hold sway over the content within, whether it is light-tree or shadow text.

Thus, if dir="ltr" is set on a shadow host, and the slotted content is a paragraph containing Arabic, the directionality of the Arabic will be LTR instead of its inherent RTL. And analogously for dir="RTL" filtering down to English to make it RTL instead of its inherent LTR. This is true whether or not the text content has an enclosing element between it and the shadow host.

Explicit dir="auto" values on shadow hosts, slots, or elements that are slotted means the content within will act according to its inherent directionality, whether it is light-tree or shadow text.

This demonstrates there is no inherent directionality in slots, shadow elements, or light-tree elements. Absent a directional dir value, content asserts its inherent directionality, and dir="auto" does not cause a specific direction to be imposed on the content.

@meyerweb
Copy link
Contributor Author

An addendum/clarification from @MyidShin:

Regarding dir-shadow-036.html, AFAIK, we shouldn't violate encapsulation of shadow DOM and should resolve the light tree element's directionality in the light tree and do not walk into the shadow tree for encapsulation (as per whatwg/html#3699). So, dir-shadow-036.html's result should be 'LTR' and Chromium fixed recently this issue at https://chromium-review.googlesource.com/c/chromium/src/+/3066807.

The dir-shadow-036.html does currently look for LTR, so as long as the above behavior is what’s desired, the test is good to go. However, the above does address something my previous summary may have elided.

chromium-wpt-export-bot pushed a commit that referenced this pull request Dec 16, 2021
This CL applies the new agreements for a <slot> elelement on the
standards meeting[1][2].
 - Resolve the slot element's directionality from the flattened tree
 - Apply one exception that the slot element's directionality should
   be the same as a shadow host's directionality if the slot element
   has a dir=auto attribute and has no slotted content.

This CL adds the unit tests on blink and WPT tests will be added on [3].

[1] whatwg/html#7299
[2] whatwg/html#3699 (comment)
[3] #29820

Bug: 1236384
Change-Id: I43ca7351fc4d65dd4f72fe4f9627ffa5382e5c20
chromium-wpt-export-bot pushed a commit that referenced this pull request Dec 21, 2021
This CL applies the new agreements for a <slot> elelement on the
standards meeting[1][2].
 - Resolve the slot element's directionality from the flattened tree
 - Apply one exception that the slot element's directionality should
   be the same as a shadow host's directionality if the slot element
   has a dir=auto attribute and has no slotted content.

This CL adds the unit tests on blink and WPT tests will be added on [3].

[1] whatwg/html#7299
[2] whatwg/html#3699 (comment)
[3] #29820

Bug: 1236384
Change-Id: I43ca7351fc4d65dd4f72fe4f9627ffa5382e5c20
chromium-wpt-export-bot pushed a commit that referenced this pull request Jan 12, 2022
This CL applies the new agreements for a <slot> elelement on the
standards meeting[1][2].
 - Resolve the slot element's directionality from the flattened tree
 - Apply one exception that the slot element's directionality should
   be the same as a shadow host's directionality if the slot element
   has a dir=auto attribute and has no slotted content.

This CL adds the unit tests on blink and WPT tests will be added on [3].

[1] whatwg/html#7299
[2] whatwg/html#3699 (comment)
[3] #29820

Bug: 1236384
Change-Id: I43ca7351fc4d65dd4f72fe4f9627ffa5382e5c20
chromium-wpt-export-bot pushed a commit that referenced this pull request Jan 12, 2022
This CL applies the new agreements for a <slot> elelement on the
standards meeting[1][2].
 - Resolve the slot element's directionality from the flattened tree
 - Apply one exception that the slot element's directionality should
   be the same as a shadow host's directionality if the slot element
   has a dir=auto attribute and has no slotted content.

This CL adds the unit tests on blink and WPT tests will be added on [3].

[1] whatwg/html#7299
[2] whatwg/html#3699 (comment)
[3] #29820

Bug: 1236384
Change-Id: I43ca7351fc4d65dd4f72fe4f9627ffa5382e5c20
chromium-wpt-export-bot pushed a commit that referenced this pull request Jan 18, 2022
This CL applies the new agreements for a <slot> elelement on the
standards meeting[1][2].
 - Resolve the slot element's directionality from the flattened tree
 - Apply one exception that the slot element's directionality should
   be the same as a shadow host's directionality if the slot element
   has a dir=auto attribute and has no slotted content.

This CL adds the unit tests on blink and WPT tests will be added on [3].

[1] whatwg/html#7299
[2] whatwg/html#3699 (comment)
[3] #29820

Bug: 1236384
Change-Id: I43ca7351fc4d65dd4f72fe4f9627ffa5382e5c20
chromium-wpt-export-bot pushed a commit that referenced this pull request Jan 20, 2022
This CL applies the new agreements for a <slot> element on the
standards meeting[1][2].
 - Resolve the slot element's directionality from the flattened tree
 - Apply one exception that the slot element's directionality should
   be the same as a shadow host's directionality if the slot element
   has a dir=auto attribute and has no slotted content.

This CL adds the unit tests on blink and WPT tests will be added on [3].

[1] whatwg/html#7299
[2] whatwg/html#3699 (comment)
[3] #29820

Bug: 1236384
Change-Id: I43ca7351fc4d65dd4f72fe4f9627ffa5382e5c20
chromium-wpt-export-bot pushed a commit that referenced this pull request Jan 20, 2022
This CL applies the new agreements for a <slot> element on the
standards meeting[1][2].
 - Resolve the slot element's directionality from the flattened tree
 - Apply one exception that the slot element's directionality should
   be the same as a shadow host's directionality if the slot element
   has a dir=auto attribute and has no slotted content.

This CL adds the unit tests on blink and WPT tests will be added on [3].

[1] whatwg/html#7299
[2] whatwg/html#3699 (comment)
[3] #29820

Bug: 1236384
Change-Id: I43ca7351fc4d65dd4f72fe4f9627ffa5382e5c20
chromium-wpt-export-bot pushed a commit that referenced this pull request Jul 20, 2022
This CL applies the new agreements for a <slot> element on the
standards meeting[1][2].
 - Resolve the slot element's directionality from the flattened tree
 - Apply one exception that the slot element's directionality should
   be the same as a shadow host's directionality if the slot element
   has a dir=auto attribute and has no slotted content.

This CL adds the unit tests on blink and WPT tests will be added on [3].

[1] whatwg/html#7299
[2] whatwg/html#3699 (comment)
[3] #29820

Bug: 1236384
Change-Id: I43ca7351fc4d65dd4f72fe4f9627ffa5382e5c20
@rniwa
Copy link
Contributor

rniwa commented Sep 1, 2022

What's up with 8 lang attribute tests without reference files? Maybe inadvertently included in the PR?

@rniwa
Copy link
Contributor

rniwa commented Sep 1, 2022

  • 7 more where we have agreement in 2 engines and the test, but Safari disagrees (18, 24, 30, 33, 34, 38, 39).

    • I think the first question is can we verify that these are just bugs and move them into the "done" colum, or is there some actually disagreement? @rniwa ? If we can agree these lone holdouts are bugs, we're all the way up to agreeing on the results of 37/40 tests and a long way to interop on them....

No, these are not WebKit bugs. On the contrary, according to @fantasai's definition in whatwg/html#3699 (comment) to which I had previously agreed, WebKit's behavior is correct and the tests and Firefox/Chrome are wrong. Specifically, dir=auto should resolve in each tree without crossing shadow boundaries first. Only then should direction property inherit from a parent to a child in the flat tree.

meyerweb and others added 17 commits October 6, 2023 08:42
This change matches the behavior of Chromium, Gecko, and WebKit.

I believe this is correct because in the test, the dir attribute is on a
slot that has display: contents.  Therefore there is nothing that
establishes a new bidi embedding.

(This could also be fixed by just removing the dir attribute, but I
think this fix is slightly clearer.)
Per @annevk's comment in
web-platform-tests#29820 (comment)
this moves all of the tests into html/dom/elements/global-attributes/,
drops the directionality/ subdirectory for some of them, and renames the
new lang-attribute-* to lang-attribute-shadow-*.
This changes the expectation for dir-shadow-036.html as described in
web-platform-tests#29820 (comment) .

Based on the proposal in whatwg/html#3699 (comment) which says:
> dir=auto on a slot resolves against its slotted content.
I believe the expectation for this test should be changed to "ltr" for
both halves.  This matches test 006 which I think tests the same
behavior, rather than contradicting it.
@dbaron dbaron merged commit f8712c6 into web-platform-tests:master Oct 6, 2023
20 checks passed
dbaron added a commit that referenced this pull request Oct 6, 2023
This tests the lang attribute changes in whatwg/html#9796.  It replaces
the lang attribute tests from #29820.

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>
cookiecrook pushed a commit to cookiecrook/wpt that referenced this pull request Oct 11, 2023
Tests for :dir() and shadow DOM for issue web-platform-tests#3699

Co-authored-by: Brian Kardell <bkardell@gmail.com>
Co-authored-by: L. David Baron <dbaron@chromium.org>
cookiecrook pushed a commit to cookiecrook/wpt that referenced this pull request Oct 11, 2023
This tests the lang attribute changes in whatwg/html#9796.  It replaces
the lang attribute tests from web-platform-tests#29820.

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>
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
…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
Tests for :dir() and shadow DOM for issue web-platform-tests#3699

Co-authored-by: Brian Kardell <bkardell@gmail.com>
Co-authored-by: L. David Baron <dbaron@chromium.org>
Lightning00Blade pushed a commit to Lightning00Blade/wpt that referenced this pull request Dec 11, 2023
This tests the lang attribute changes in whatwg/html#9796.  It replaces
the lang attribute tests from web-platform-tests#29820.

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>
thanatkron

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.