-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Added shadow directionality to the dir attribute algorithm #7424
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that in case just text is assigned to a slot, it will use the direction of the host element and not the slot?
@tabatkins could you also help review this? We're looking for someone who is more of an expert in layout-type stuff, which the HTML editors aren't :) |
We should also loop in fantasai, but GitHub doesn’t seem to want to autocomplete her account here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also loop in fantasai, but GitHub doesn’t seem to want to autocomplete her account here.
Probably for the best, the chances I'd notice the GH notification is like 0.5% :/ Please always feel free to ping me by email/IRC/sms.
Our goal is speccing #3699 (comment) right? Here's what I suggest:
diff --git a/source b/source
index 66182bdd..f3efdb9f 100644
--- a/source
+++ b/source
@@ -11897,8 +11897,15 @@ <h5>The <code data-x="attr-lang">lang</code> and <code data-x="attr-xml-lang">xm
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>host</span> element, if any.</dd>
+
+ <dt>Otherwise</dt>
+ <dd>the same as the language of its parent element, if any.</dd>
+ </dl>
<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>
@@ -12198,6 +12205,15 @@ <h5>The <code data-x="attr-dir">dir</code> attribute</h5>
element's <span data-x="the directionality">directionality</span>.</p>
</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>
+
+ <dd><p><span>The directionality</span> of the element is the same as its <span>shadow
+ root</span>'s <span>host</span> element's
+ <span data-x="the directionality">directionality</span>.</p></dd>
<dt>If the element's <code data-x="attr-dir">dir</code> attribute is in the <span
@@ -12207,44 +12223,63 @@ <h5>The <code data-x="attr-dir">dir</code> attribute</h5>
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
+ 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>
+ </dl>
- <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>shadow trees</span>.</p>
- </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
+ whose <span data-x="the directionality">directionality</span> is being determined and also
+ a <dfn>bidi-excluded element</dfn>:</p>
+ <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>
- <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>
<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>host</span> element's
+ <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 <span>slotted</span> 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 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>
@@ -117591,13 +117626,13 @@ <h4 id="bidi-rendering">Bidirectional text</h4>
<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="" -->
}
What this does in comparison to the original PR:
- Specs the inheritance of the content language, which follows the same rules as directionality.
- Specs slot elements and shadow root elements without a defined directionality to inherit from their host element.
- Defines
dir=auto
on a slot to take the directionality from its contents (matching all other elements, as we had agreed), not its host element. - Incorporates slot elements into the "first strong" search for
dir=auto
elements. - Adds a note that this search does not recurse into shadows.
- Makes the inheritance of slotted content with undefined directionality from its slot unconditional and shifts it down the priority list.
- Adds in the requisite UA stylesheet rules.
Lmk if that makes sense, if there's anything you'd like explained, if anything seems wrong, if you'd prefer me to post the suggested edits as an actual PR, or if you'd like me to re-review once you've incorporated the necessary fixes (which don't have to match exactly the proposed text above, of course) into this PR. :)
Oh, and I might suggest dropping a link directly to #3699 (comment) (and/or copy in its text) into the extended description part of the commit message. This is a pretty confusing area of HTML, and the discussion in 3699 is quite long, so it's probably helpful to have that info easily accessible in the version history. |
Very interested in feedback from @annevk and @tabatkins, but happy to hear from anyone, on these latest changes from Elika! Hoping to have all feedback in and changes made before the next triage meeting, so that this can be merged and closed. |
+1 to fantasai's suggested changes. As far as I can tell, they match the intent of her comment and generally describe a good behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had another look at this. There are still some issues and I worry that the language is getting too opaque, but I also don't have a concrete suggestion.
|
||
<dt>Otherwise</dt> | ||
<dd>the same as the language of its parent element, if any.</dd> | ||
</dl> |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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 oneclosed
shadow root, just for completeness? - Per the comments above, do you need to add some permutations that include both
dir
andlang
? - 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.
There was a problem hiding this comment.
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))?
There was a problem hiding this comment.
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.
source
Outdated
<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> | ||
|
||
<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> |
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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?
source
Outdated
<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> |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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?
There was a problem hiding this 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 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.
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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”
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. ^_^
There was a problem hiding this comment.
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.
I believe this is ready to be merged. |
There's a couple unresolved questions still. It also seems that this branch has merge conflicts. You want to rebase against main, resolve conflicts, and force push. Also, I think we should have at least one other person look at this. Perhaps @smaug----? |
<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 whose root is a shadow root and whose <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> |
There was a problem hiding this comment.
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><span data-x="finding slottables">Find slottables</span>, and then 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> |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
<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> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm understanding correctly, I believe all of these lines should be deleted.
<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> |
Editorial comment: We have a lot of |
Closing in favor of an updated and reworked PR #9166 |
This specifies (with some additional detail) the proposal in whatwg#3699 (comment) . This changes three things: * the inheritance of directionality * the inheritance of language * the computation of dir=auto to account for Shadow DOM. This builds on the work in whatwg#9452 and whatwg#9554 to refactor this section, and builds on work by Brian Kardell, Eric Meyer, and others in whatwg#7424 and in whatwg#9166 and work by fantasai, rniwa, smaug, MyIdShin, Brian Kardell, and others in whatwg#3699. Closes whatwg#9166. Fixes whatwg#3699.
This specifies (with some additional detail) the proposal in #3699 (comment). This changes three things: * the inheritance of directionality, * the inheritance of language, and * the computation of dir=auto to account for shadow trees. This builds on the work in #9452 and #9554 to refactor this section, and builds on work by Brian Kardell, Eric Meyer, and others in #7424 and in #9166 and work by fantasai, rniwa, smaug, MyIdShin, Brian Kardell, and others in #3699. Fixes #3699.
This specifies (with some additional detail) the proposal in whatwg#3699 (comment). This changes three things: * the inheritance of directionality, * the inheritance of language, and * the computation of dir=auto to account for shadow trees. This builds on the work in whatwg#9452 and whatwg#9554 to refactor this section, and builds on work by Brian Kardell, Eric Meyer, and others in whatwg#7424 and in whatwg#9166 and work by fantasai, rniwa, smaug, MyIdShin, Brian Kardell, and others in whatwg#3699. Fixes whatwg#3699.
Closes #3699. See #3699 (comment) for a summary of what the changes are meant to do.
(See WHATWG Working Mode: Changes for more details.)
/dom.html ( diff )
/infrastructure.html ( diff )
/rendering.html ( diff )