Tests for directionality and shadow DOM

[meyerweb]

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

[meyerweb]

@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]

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]

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]

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]

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 https://github.com/whatwg/html/issues/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.

[rniwa]

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

[rniwa]

• 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 https://github.com/whatwg/html/issues/3699#issuecomment-951423468 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]

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

I believe those are the tests I drafted and solicited feedback on. If so, there are no reference files yet because I didn't want to create references without being sure whether the tests were correct or not.

(Apologies for the delay in replying. The GitHub notifications somehow went astray, probably due to me doing a mass-mark-as-read or something.)

[fantasai]

Responding to https://github.com/web-platform-tests/wpt/pull/29820#issuecomment-1233969556, I agree with @rniwa on 18,24,30,33,34:

• Tests 18, 24, 30 - We've specced dir=auto to search only within the light tree, not to recurse into shadows, so the contents of the shadow have no effect on the resolution of the light DIV's dir=auto; and hence it resolves to LTR before inheriting through into the shadow tree.
• Test 33, 34 - The slot has no dir attribute, so it inherits directly from the host element, whose directionality is LTR. Then the shadow element with dir=auto picks up the slot's directionality (LTR).

I believe tests 38 and 39 are correct, though:

• Test 38, 39 - The slot's dir=auto resolves to LTR because of the slotted LTR content from the light tree. The div#host resolves to LTR because of inheritance from its parent. (The test is a bit confusing to read though.)

[rniwa]

Any update on this PR?

[bkardell]

Alright, I have updated the expectations of the 5 tests that @rniwa and @fantasai (who helped create the spec text so far) agreed were wrong (18, 24, 30, 33, 34), as well as added the punctation that @fantasai requested.

It seems that there is disagreement on 38 and 39? Though I agree they are kind of exceptionally confusing tests.. Maybe we should actually rewrite those two... In 39 the thing you're seeing reported is the computed directionality of the thing that isn't rendered on the screen (the default text, in an element of its own). Safari sees that as RTL because the content in it would be... I get it? I guess? But Safari also sees the slot (not reported distinctly) as RTL - but there it feels like the projected text is what should count.

Idk, I guess I'd like to confirm that aside from 38 and 39 we all agree on those tests? Then if so we can figure out what 38,39 should be and why.

[meyerweb]

Thank you, @bkardell!

Idk, I guess I'd like to confirm that aside from 38 and 39 we all agree on those tests? Then if so we can figure out what 38,39 should be and why.

I’m entirely willing to drop them if they’re too obscure or misleading.

[mfreed7]

I just wanted to comment from Chromium's perspective on this PR:

1. Generally the tests look good. I still think these tests would be so much easier to understand and reason about if they made use of declarative shadow dom. That would get rid of the need to mentally cut/paste the shadow contents. You could even abstract the result.innerHTML += bit of the script into a helper, so that each test case is then pure HTML. With the complexity of the rules for dir, anything you can do to reduce the mental load of understanding a) the structure of the DOM, and b) the node being tested, would be great. Please see some sample code below that I think would simplify things.
2. Along the same lines as above, it would also be super helpful to include the behavior in addition to the structure, in each test's descriptive <p> element. For example, for test 18, replace existing <p> with <p>dir=auto on the shadow host, paragraph in the shadow tree: resolve dir=auto against the light tree first, then shadow content resolves against the shadow host.</p>
3. Ok, with nits out of the way, I think tests 18, 24, 30, 33, and 34 all look "correct" now, with "correct" defined by this comment. I looked at each test and followed the rules before peeking at the -ref file answer. They all seem to match my expectations.
4. Test 38 also looks correct. The slot resolves against its slotted content (ltr) and the host resolves against the light tree (also ltr).
5. Test 39 I'm not sure. Nothing (I think?) in the proposal mentions what happens for slot fallback content. I'm not clear it matters too much since it isn't rendered anyway.

---

Sample for test 18:

<!DOCTYPE html>
{...header stuff...}

<p>dir=auto on the shadow host, paragraph in the shadow tree:
resolve dir=auto against the light tree first, then shadow content
resolves against the shadow host.</p><hr>

<div dir="auto">
  <template shadowrootmode=open>
    <p id=target>اختبر.</p>
  </template>
</div>

<script src="resources/directionality_helper.js">

with directionality_helper.js doing this:

function findTarget(el) {
  let root = el.shadowRoot || el;
  const target = root.querySelector('#target');
  if (target) return target;
  for(child of root.children) {
    const val = findTarget(child);
    if (val) return val;
  }
  return undefined;
}
window.addEventListener('load',() => {
  const targetDir = getComputedStyle(findTarget(document.body)).direction;
  const result = document.createElement('div');
  result.innerHTML = `<hr>Computed direction for target: ${targetDir}`;
  document.body.appendChild(result);
});

[dbaron]

FYI, one thing that I noticed in terms of status of the tests (at least as of February, when the PR was last updated) was that implementations were passing the tests (based on the wpt.fyi data in the Checks tab) with the exception of:

directionality/dir-shadow-06.html - Gecko WebKit directionality/dir-shadow-18.html - Gecko directionality/dir-shadow-24.html - Gecko directionality/dir-shadow-30.html - Chromium Gecko directionality/dir-shadow-31.html - Chromium** Gecko** WebKit** directionality/dir-shadow-33.html - Gecko directionality/dir-shadow-34.html - Chromium Gecko directionality/dir-shadow-36.html - Chromium directionality/dir-shadow-38.html - Gecko WebKit directionality/dir-shadow-39.html - Gecko WebKit directionality/dir-shadow-41.html - Chromium Gecko WebKit

The "**" for test 31 indicates that the test is failing because of a position mismatch on the text rather than because of the directionality check; I think it's likely something needs to be fixed for the reference for that test. In all of the other tests the directionality check failed (and in many cases the text was different as well).

[dbaron]

I updated dir-shadow-31-ref.html per my comment above in 6e02b193e2313fbc1965bcf7371a455459812174. If others want to double-check my work, that would be appreciated, although I think this will make dir-shadow-31.html pass across engines again.

I'd also note that the fact that it was failing was a result of the immediately preceding change in this PR, be133fea41784108d70c0e48c891253c06786019.

[dbaron]

So I just looked into the Chromium failure on dir-shadow-036.html, which is a test that's closely related to dir-shadow-006.html (the main differences being the directionality being the other way around, and the latter test having interesting default content in the slot), and which was previously discussed in https://github.com/web-platform-tests/wpt/pull/29820#issuecomment-897065194 and https://github.com/web-platform-tests/wpt/pull/29820#issuecomment-899546941 . I think these tests are essentially asserting opposite behaviors right now (and Chromium passes 006 and fails 036, and Gecko/WebKit do the reverse).

Based on the proposal in https://github.com/whatwg/html/issues/3699#issuecomment-951423468 which says:

dir=auto on a slot resolves against its slotted content.

I believe the Chromium behavior is correct, and the expectation for test -036 should be changed to "ltr" for both halves.

(https://github.com/web-platform-tests/wpt/pull/41584 also adds another test of this behavior into a different test that's already in WPT.)

---

On another note, I just submitted https://chromium-review.googlesource.com/c/chromium/src/+/4800366 which fixes the Chromium failures on dir-shadow-030.html and dir-shadow-034.html.

[annevk]

I suggest we move these tests to html/dom/elements/global-attributes/ as there's no longer a standalone "Shadow DOM" specification.

[dbaron]

For what it's worth, dir-shadow-041.html seems correct to me, although I'm not 100% sure yet. It led me to finding two real Chromium bugs (fix 1, fix 2), the second of which requires fixing a bunch of other things that I haven't finished with yet. So since I don't have both bug fixes fully written I can't be sure that the end result will make the test pass.

[dbaron]

And I just moved the tests as requested in https://github.com/web-platform-tests/wpt/pull/29820#issuecomment-1692921692 . That triggered a new round of review requests -- though I think we may already have enough reviewers at this point :-)

[dbaron]

Given the lack of feedback (so far) on https://github.com/web-platform-tests/wpt/pull/29820#issuecomment-1688749979, I updated the expectation for dir-shadow-036.html in 2c55e60c as described in that comment. Still happy to get further feedback, though.

[dbaron]

Oh, and another point about dir-shadow-36.html -- it's actually the same test as dir-shadow-38.html except that in the latter the (unrendered) Arabic text inside the slot has a period at the end. So its expectation was (before the change I just made) also clearly contradicting that of dir-shadow-38.html!

[dbaron]

Status: currently addressed some of @annevk's review comments but not all of them. (And considering writing a bunch of lang tests in JS and dropping this set.) Hopefully will finish that tomorrow.

[dbaron]

I'm not sure I understand the language tests. If they're just about what selectors match, then I'd make them JS tests.

I've dropped the language tests (I only added references for them since your last comment!) and replaced them with a separate PR in #42364.

[annevk]

I'm going to rebase and force push this. I suspect the checks are failing because we're not running the latest tooling.