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.

Sign up for GitHub

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

Added shadow directionality to the dir attribute algorithm #7424

Closed
wants to merge 6 commits into from
Closed

Added shadow directionality to the dir attribute algorithm #7424

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fe78255
Added shadow directionality to the dir attribute
meyerweb Dec 16, 2021
b3bdb24
Edited to use Elika's rewrite
meyerweb Feb 8, 2022
d46d084
Fixing Wattsi build errors
meyerweb Feb 8, 2022
b24d8e2
data-x cross reference added
meyerweb Mar 15, 2022
3cf5ac7
Updates to try to make things more concrete
meyerweb Apr 15, 2022
b7b84e8
Edits as per comments on 7427
meyerweb Jul 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 88 additions & 31 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11896,8 +11896,15 @@ Transport Protocol">HTTP&lt;/abbr> today.&lt;/p></code></pre> <!-- DO NOT REWRAP
data-x="">lang</code></dfn> attribute in the <span>XML namespace</span> is defined in XML. <ref
spec=XML></p>

<p>If these attributes are omitted from an element, then the language of this element is the same
as the language of its parent element, if any.</p>
<p>If these attributes are omitted from an element, then the language of this element is:
<dl class="switch">
<dt>If the element's parent is a <span>shadow root</span>
<dt>If the element itself is a <code>slot</code> element
<dd>the same as the language of its <span>shadow root</span>'s <span data-x="dom-hyperlink-host">host</span> element, if any.</dd>
meyerweb marked this conversation as resolved.
Show resolved Hide resolved

<dt>Otherwise</dt>
<dd>the same as the language of its parent element, if any.</dd>
</dl>
Copy link
Member

Choose a reason for hiding this comment

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

Is there a benefit to tackling language in the same PR? Do we have tests for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. They need to work together. (And were part of the proposal that was accepted).

Copy link
Member

Choose a reason for hiding this comment

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

How do they work together?

And are there tests for that?

Copy link
Contributor

Choose a reason for hiding this comment

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

How do they work together?

If you have one set of rules for inheriting language, and a different set for inheriting direction, and you're working with French and Arabic, you're going to end up with elements that have lang=ar dir=ltr or lang=fr dir-rtl, and this is clearly nonsensical.

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’ll check on the state of tests for this and resolve once I’ve confirmed (or created) them.

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’ve drafted 8 tests here: https://github.com/meyerweb/wpt/tree/direction-3699/shadow-dom. They’re lang-attribute-001.html through -008.html. They’re basically a mixture of host element, light tree element, and slot element with varying lang attribute values (if any) and CSS meant to highlight which elements match certain values.

So: Am I testing the right things? If these are proper tests, I’ll generate reference renderings. If not, feel free to reach out via email or other channel to help me out. Thanks!

Copy link
Contributor

@mfreed7 mfreed7 Sep 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for writing these tests. They generally look good to me. A few comments:

  • I think you've included all of the relevant permutations of the placement of the lang attribute and matching/mismatching values. You might think about adding one closed shadow root, just for completeness?
  • Per the comments above, do you need to add some permutations that include both dir and lang?
  • Given that all of these tests are appearance tests, you could combine (I think) all 8 tests into a single test. They all share the same stylesheet anyway, so you could just put them all together into a single file.
  • You could think about using declarative shadow DOM to avoid the script block(s) and make the test cases (much!) easier to read. You can include the polyfill from here (css/css-contain/container-queries/support/cq-testcommon.js). I've been meaning to move it to a more central location.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, Mason! I’m a bit wary of conflating multiple tests in a single page, but I’ll investigate that and your polyfill.

What are your thoughts regard Ryosuke’s feedback on #3699 (#3699 (comment))?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, Mason! I’m a bit wary of conflating multiple tests in a single page, but I’ll investigate that and your polyfill.

Thanks! For non-visual tests, we often include many, many tests in a single file. So IMHO it’s ok/good to do here too.

What are your thoughts regard Ryosuke’s feedback on #3699 (#3699 (comment))?

I have to admit that I’m not sure I understand the nuance of the difference between the fantasai proposal and your text.


<p>The <code data-x="attr-lang">lang</code> attribute in no namespace may be used on any <span
data-x="HTML elements">HTML element</span>.</p>
Expand Down Expand Up @@ -12198,6 +12205,16 @@ Transport Protocol">HTTP&lt;/abbr> today.&lt;/p></code></pre> <!-- DO NOT REWRAP
</dd>


<dt>If the element has a parent node that is a <span>shadow root</span> and its own
<code data-x="attr-dir">dir</code> attribute is not in a defined state (i.e. it is not present or
has an invalid value)</dt>
<dt>If the element is a <code>slot</code> element and its <code data-x="attr-dir">dir</code>
attribute is not in a defined state (i.e. it is not present or has an invalid value)</dt>

Copy link

Choose a reason for hiding this comment

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

I assume we'll have tests (both for lang and dir) for slot elements which aren't in shadow DOM. Or which are first in shadow DOM but are then moved away from there to light DOM.

<dd><p><span>The directionality</span> of the element is the same as its <span>shadow
root</span>'s <span data-x="dom-hyperlink-host">host</span> element's
<span data-x="the directionality">directionality</span>.</p></dd>
Copy link
Member

Choose a reason for hiding this comment

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

Edge case: this text doesn't work if the slot element is not inside a shadow root. (Do we have a test for that?)

You'd want some language akin to "If the element is a slot element whose root is a shadow root and whose dir attribute is not ..." I think. And then below instead of talking about shadow root you just want to talk about its root. (As elements don't really have a shadow root, unless they're a shadow host, but then it means something different.)

Copy link
Member

Choose a reason for hiding this comment

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

At least the test question here seems unresolved?



<dt>If the element's <code data-x="attr-dir">dir</code> attribute is in the <span
data-x="attr-dir-auto-state">auto</span> state</dt>
Expand All @@ -12206,44 +12223,84 @@ Transport Protocol">HTTP&lt;/abbr> today.&lt;/p></code></pre> <!-- DO NOT REWRAP
attribute is not in a defined state (i.e. it is not present or has an invalid value)</dt>

<dd>
<p>Find the first character in <span>tree order</span> that matches the following criteria:</p>

<ul>

<li><p>The character is from a <code>Text</code> node that is a descendant of the element whose
<span data-x="the directionality">directionality</span> is being determined.</p></li>

<li><p>The character is of bidirectional character type L, AL, or R. <ref spec=BIDI></li>

<li>
<p>The character is not in a <code>Text</code> node that has an ancestor element that is a
descendant of the element whose <span data-x="the directionality">directionality</span> is
being determined and that is either:</p>
<dl class="switch">
<dt>If the element is not a <code>slot</code> element</dt>
<dd>Find among its descendant elements and <code>Text</code> nodes the first (in <span>tree
order</span>) <span>bidi-available</span> <code>slot</code> element or <span>strong
meyerweb marked this conversation as resolved.
Show resolved Hide resolved
character</span>.</dd>

<dt>If the element is a <code>slot</code> element</dt>
<dd>Find among its slotted elements and <code>Text</code> nodes (including their descendants)
the first (in <span>tree order</span>) <span>bidi-available</span> <code>slot</code> element or
<span>strong character</span>.</dd>
Copy link
Member

Choose a reason for hiding this comment

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

You need to run "find slottables" here to get these. Although maybe this should be "find flattened slottables"? I don't really understand how bidi-available can work here (except perhaps as an assert as assigned slot elements would necessarily be from another tree).

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 found a <dfn> of “find flattened slottables” in the source, but not “find slottables”. Does “Find flattened slottables, and then find among its slotted elements…” suffice?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what that sentence means. Ideally we use concrete algorithms so it's unambiguous. Both of the algorithms I mentioned are defined in the DOM Standard.

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 think I’ve addressed that in my latest push (made just now) in response to your feedback. Let me know!

Copy link
Member

Choose a reason for hiding this comment

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

If you use flattened, how do you end up with slot elements? And how does bidi-available end up mattering here?

Copy link
Member

Choose a reason for hiding this comment

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

@fantasai and @tabatkins, maybe you can chime in here?

As for adding a reference, look for dfn elements with data-x-href attributes. That's where you can add additional references to other specifications. You probably want to place it next to the one for flattened.

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’ve pushed a change changing this to “find slottables” and also including a new reference to finding slottables”

Copy link
Member

Choose a reason for hiding this comment

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

I'd still like to see some kind of explanation. Also why one of these is used over the other.

Copy link
Contributor

@fantasai fantasai Oct 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@annevk I don't think I'm following the conversation here... From what I understand, slot elements exist in the flattened tree (but are display: contents by default, so they don't show up in the box tree by default). Wrt recursion, we don't need to recursively flatten the tree because once we hit a slot element, we return the directionality of that element; its content doesn't matter (except insofar as it influences the directionality of the slot element itself).

The concept of bidi-available is in order to exclude the contents of e.g. script/style/textarea elements and any bidi-isolated elements, as these are things which are embedded within content and not representative of it such that we would want to extract directionality info from them.

Let me know if that answers your question; if not I need a bit more context in the question to understand what I'm being asked. ^_^

Copy link
Member

Choose a reason for hiding this comment

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

Well, if you use "find flattened slotables" the slot elements are omitted (unless their root is not a shadow root). See https://dom.spec.whatwg.org/#find-flattened-slotables for its definition.

This is a purposely different algorithm from https://drafts.csswg.org/css-scoping/#flattening.

</dl>

Copy link

@smaug---- smaug---- Aug 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

What if the element is a slot element, but nothing is slotted to it - it has only the default content?

I guess I don't understand how one should read all the "If the..."
One says "If the element's dir attribute is in the auto state" and underneath it handles slot in a special way, but there is another "If the element is a slot element and the dir attribute is in the auto state"

How is <slot dir=auto> supposed to work?

Copy link
Contributor

@fantasai fantasai Oct 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@smaug---- I think a bunch of the lines below need to be deleted; added a comment to that effect. But to your point about the default content, I think a slot with dir=auto that has its default content instead of slotted content should be looking at its default content here. But I don't know how to express that in spec terminology.

<ul class="brief">
<li>A <code>bdi</code> element.
<li>A <code>script</code> element.
<li>A <code>style</code> element.
<li>A <code>textarea</code> element.
<li>An element with a <code data-x="attr-dir">dir</code> attribute in a defined state.
</ul>
</li>
<p class="note">This search does not recurse into <span data-x="shadow tree">shadow trees</span>.</p>
meyerweb marked this conversation as resolved.
Show resolved Hide resolved

</ul>
<p>A <code>slot</code> element or <span>strong character</span> is only <dfn>bidi-available</dfn>
if it does <em>not</em> have an ancestor element that is both a descendant of the element
meyerweb marked this conversation as resolved.
Show resolved Hide resolved
whose <span data-x="the directionality">directionality</span> is being determined and also
a <dfn>bidi-excluded element</dfn>:</p>
meyerweb marked this conversation as resolved.
Show resolved Hide resolved
<ul class="brief">
<li>A <code>bdi</code> element.
<li>A <code>script</code> element.
<li>A <code>style</code> element.
<li>A <code>textarea</code> element.
meyerweb marked this conversation as resolved.
Show resolved Hide resolved
<li>An element with a <code data-x="attr-dir">dir</code> attribute in a defined state.
meyerweb marked this conversation as resolved.
Show resolved Hide resolved
</ul>

<p>If such a character is found and it is of bidirectional character type AL or R, <span>the
directionality</span> of the element is '<span data-x="concept-rtl">rtl</span>'.</p>
<p>A character is a <dfn>strong character</dfn> if the character is of bidirectional character type
L, AL, or R. <ref spec=BIDI></p>

<p>If such a character is found and it is of bidirectional character type L, <span>the
directionality</span> of the element is '<span data-x="concept-ltr">ltr</span>'.</p>
<p>If such a <span>strong character</span> or <code>slot</code> element is found:</p>
<dl class="switch">
<dt>If the character is of bidirectional character type AL or R</dt>
<dt>If the <code>slot</code> has a <span data-x="the directionality">directionality</span> of <span data-x="concept-rtl">rtl</span></dt>
<dd><span>The directionality</span> of the element is '<span data-x="concept-rtl">rtl</span>'.</dd>
<dt>If the character is of bidirectional character type L</dt>
<dt>If the <code>slot</code> has a <span data-x="the directionality">directionality</span> of <span data-x="concept-ltr">ltr</span></dt>
<dd><span>The directionality</span> of the element is '<span data-x="concept-ltr">ltr</span>'.</dd>
</dl>

meyerweb marked this conversation as resolved.
Show resolved Hide resolved
<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>

<p>Otherwise, if the element is a <code>slot</code> element, <span>the directionality</span>
of the element is the same as its <span>shadow root</span>'s <span data-x="dom-hyperlink-host">host</span> element's
meyerweb marked this conversation as resolved.
Show resolved Hide resolved
<span data-x="the directionality">directionality</span>.</p>

<p>Otherwise, <span>the directionality</span> of the element is the same as the element's parent
element's <span data-x="the directionality">directionality</span>.</p>
</dd>

<dt>If the element is slotted and its <code data-x="attr-dir">dir</code> attribute
is not in a defined state (i.e. it is not present or has an invalid value)</dt>

<dd><p><span>The directionality</span> of the element is the
<span data-x="the directionality">directionality</span> of its <span>assigned slot</span>.</p></dd>

<dt>If the element is slotted and the <code data-x="attr-dir">dir</code> attribute of its
associated slot is in a defined state</dt>

<dd><p><span>The directionality</span> of the element is the same as the slot's
<span data-x="the directionality">directionality</span>.</p></dd>

<dt>If the element is slotted and the <code data-x="attr-dir">dir</code> attribute of its
associated slot is in the <span data-x="attr-dir-auto-state">auto</span> state</dt>

<dd><p><span>The directionality</span> of the element is the same as the
slot's host element's <span data-x="the directionality">directionality</span>.</p></dd>

<dt>If the element is a <code>slot</code> element and the <code data-x="attr-dir">dir</code>
attribute is in the <span data-x="attr-dir-auto-state">auto</span> state</dt>

<dd><p><span>The directionality</span> of the element is the same as its host
element's <span data-x="the directionality">directionality</span>.</p></dd>

<dt>If the element has a parent node which is a <span>shadow root</span></dt>

<dd><p><span>The directionality</span> of the <span>shadow root</span> is the same its host
element's <span data-x="the directionality">directionality</span>.</p></dd>

<dt>If the element has a parent element and the <code data-x="attr-dir">dir</code> attribute is
not in a defined state (i.e. it is not present or has an invalid value)</dt>
Expand Down Expand Up @@ -117542,13 +117599,13 @@ br[clear=all i], br[clear=both i] { clear: both; }</code></pre>

<pre><code class="css">@namespace url(http://www.w3.org/1999/xhtml);

[dir]:dir(ltr), bdi:dir(ltr), input[type=tel i]:dir(ltr) { direction: ltr; }
[dir]:dir(rtl), bdi:dir(rtl) { direction: rtl; }
[dir]:dir(ltr), bdi:dir(ltr), slot:dir(ltr), input[type=tel i]:dir(ltr) { direction: ltr; }
[dir]:dir(rtl), bdi:dir(rtl), slot:dir(rtl) { direction: rtl; }

address, blockquote, center, div, figure, figcaption, footer, form, header, hr,
legend, listing, main, p, plaintext, pre, summary, xmp, article, aside, h1, h2,
h3, h4, h5, h6, hgroup, nav, section, table, caption, colgroup, col, thead,
tbody, tfoot, tr, td, th, dir, dd, dl, dt, menu, ol, ul, li, bdi, output,
tbody, tfoot, tr, td, th, dir, dd, dl, dt, menu, ol, ul, li, bdi, output, slot,
[dir=ltr i], [dir=rtl i], [dir=auto i] {
unicode-bidi: isolate; <!-- anything that's similar to display:block, plus <bdi>, <output>, and dir="" -->
}
Expand Down