Skip to content

Commit

Permalink
Rewrite (behind a flag) HTML direction inheritance and invalidation f…
Browse files Browse the repository at this point in the history
…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 <[email protected]>
Commit-Queue: David Baron <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1199647}
  • Loading branch information
dbaron authored and Lightning00Blade committed Dec 11, 2023
1 parent bbc4ac2 commit bd32f8e
Show file tree
Hide file tree
Showing 2 changed files with 187 additions and 0 deletions.
18 changes: 18 additions & 0 deletions html/dom/elements/global-attributes/dir-assorted.window.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,21 @@ test(() => {
ele.append(ele2);
assert_true(ele2.matches(":dir(rtl)"));
}, "Non-HTML element without direction has parent element direction");

test(() => {
let container1 = document.createElement("div");
document.body.appendChild(container1);
let container2 = document.createElement("div");

for (let container of [container1, container2]) {
container.dir = "rtl";
let e = document.createElement("div");
assert_true(e.matches(":dir(ltr)"));
container.appendChild(e);
assert_false(e.matches(":dir(ltr)"));
e.remove();
assert_true(e.matches(":dir(ltr)"));
}

container1.remove();
}, "dir inheritance is correct after insertion and removal from document");
169 changes: 169 additions & 0 deletions html/dom/elements/global-attributes/dir-auto-dynamic-changes.window.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
function setup_tree(light_tree, shadow_tree) {
let body = document.body;
let old_length = body.childNodes.length;
body.insertAdjacentHTML("beforeend", light_tree.trim());
if (body.childNodes.length != old_length + 1) {
throw "unexpected markup";
}
let result = body.lastChild;
if (shadow_tree) {
let shadow = result.querySelector("#root").attachShadow({mode: "open"});
shadow.innerHTML = shadow_tree.trim();
return [result, shadow];
}
return result;
}

test(t => {
let a = setup_tree(`
<div id="a" dir="auto">
<div id="b"></div>
hello
</div>
`);

let acs = getComputedStyle(a);
assert_true(a.matches(":dir(ltr)"), ":dir(ltr) matches before insertion");
assert_false(a.matches(":dir(rtl)"), ":dir(rtl) does not match before insertion");
assert_equals(acs.direction, "ltr", "CSSdirection before insertion");
b.innerHTML = "\u05D0";
assert_false(a.matches(":dir(ltr)"), ":dir(ltr) does not match after insertion");
assert_true(a.matches(":dir(rtl)"), ":dir(rtl) matches after insertion");
assert_equals(acs.direction, "rtl", "CSSdirection after insertion");

a.remove();
}, "dynamic insertion of RTL text in a child element");

test(() => {
let div_rtlchar = document.createElement("div");
div_rtlchar.innerHTML = "\u05D0";

let container1 = document.createElement("div");
document.body.appendChild(container1);
let container2 = document.createElement("div");

for (let container of [container1, container2]) {
container.dir = "auto";
assert_true(container.matches(":dir(ltr)"));
container.appendChild(div_rtlchar);
assert_false(container.matches(":dir(ltr)"));
div_rtlchar.remove();
assert_true(container.matches(":dir(ltr)"));
}

container1.remove();
}, "dir=auto changes for content insertion and removal, in and out of document");

test(() => {
let tree, shadow;
[tree, shadow] = setup_tree(`
<div>
<div id="root">
<span id="l">A</span>
<span id="r">\u05D0</span>
</div>
</div>
`, `
<slot id="one" name="one" dir="auto">\u05D0</slot>
<slot id="two" dir="auto"></slot>
`);

let one = shadow.getElementById("one");
let two = shadow.getElementById("two");
let l = tree.querySelector("#l");
let r = tree.querySelector("#r");
assert_false(one.matches(":dir(ltr)"), "#one while empty");
assert_true(two.matches(":dir(ltr)"), "#two with both spans");
l.slot = "one";
assert_true(one.matches(":dir(ltr)"), "#one with LTR child span");
assert_false(two.matches(":dir(ltr)"), "#two with RTL child span");
r.slot = "one";
assert_true(one.matches(":dir(ltr)"), "#one with both child spans");
assert_true(two.matches(":dir(ltr)"), "#two while empty");
l.slot = "";
assert_false(one.matches(":dir(ltr)"), "#one with RTL child span");
assert_true(two.matches(":dir(ltr)"), "#two with LTR child span");

tree.remove();
}, "dir=auto changes for slot reassignment");

test(() => {
let tree, shadow;
[tree, shadow] = setup_tree(`
<div dir=auto>
<div id=root>
<div id=text>A</div>
</div>
</div>
`, `
<div dir=ltr>
<slot id=slot dir=auto></slot>
</div>
`);

let text = tree.querySelector("#text");
let slot = shadow.querySelector("#slot");

assert_true(tree.matches(":dir(ltr)"), "node tree ancestor before first text change");
assert_true(slot.matches(":dir(ltr)"), "slot before first text change");
text.innerText = "\u05D0";
assert_false(tree.matches(":dir(ltr)"), "node tree ancestor after first text change");
assert_false(slot.matches(":dir(ltr)"), "slot after first text change");
tree.dir = "rtl";
assert_false(tree.matches(":dir(ltr)"), "node tree ancestor before second text change");
assert_false(slot.matches(":dir(ltr)"), "slot before second text change");
text.innerText = "A";
assert_false(tree.matches(":dir(ltr)"), "node tree ancestor after second text change");
assert_true(slot.matches(":dir(ltr)"), "slot after second text change");
slot.dir = "ltr";
assert_false(tree.matches(":dir(ltr)"), "node tree ancestor before third text change");
assert_true(slot.matches(":dir(ltr)"), "slot before third text change");
text.innerText = "\u05D0";
assert_false(tree.matches(":dir(ltr)"), "node tree ancestor after third text change");
assert_true(slot.matches(":dir(ltr)"), "slot after third text change");
slot.dir = "auto";
tree.dir = "auto";
assert_false(tree.matches(":dir(ltr)"), "node tree ancestor after fourth text change");
assert_false(slot.matches(":dir(ltr)"), "slot after fourth text change");
text.innerText = "A";
assert_true(tree.matches(":dir(ltr)"), "node tree ancestor before fourth text change");
assert_true(slot.matches(":dir(ltr)"), "slot before fourth text change");
slot.dir = "rtl";
assert_true(tree.matches(":dir(ltr)"), "node tree ancestor before fifth text change");
assert_false(slot.matches(":dir(ltr)"), "slot before fifth text change");
text.innerText = "\u05D0";
assert_false(tree.matches(":dir(ltr)"), "node tree ancestor before fifth text change");
assert_false(slot.matches(":dir(ltr)"), "slot before fifth text change");
}, "text changes affecting both slot and ancestor with dir=auto");

test(() => {
let tree = setup_tree(`
<div dir="auto">
<span id="a1">A</span>
<span id="aleph1">\u05D0</span>
<span id="a2">A</span>
<span id="aleph2">\u05D0</span>
</div>
`);

let a1 = tree.querySelector("#a1");
let aleph1 = tree.querySelector("#aleph1");
assert_true(tree.matches(":dir(ltr)"), "initial state");
assert_false(tree.matches(":dir(rtl)"), "initial state");
a1.dir = "ltr";
assert_false(tree.matches(":dir(ltr)"), "after change 1");
a1.dir = "invalid";
assert_true(tree.matches(":dir(ltr)"), "after change 2");
a1.dir = "rtl";
assert_false(tree.matches(":dir(ltr)"), "after change 3");
a1.removeAttribute("dir");
assert_true(tree.matches(":dir(ltr)"), "after change 4");
a1.dir = "invalid";
assert_true(tree.matches(":dir(ltr)"), "after change 5");
a1.dir = "rtl";
assert_false(tree.matches(":dir(ltr)"), "after change 6");
aleph1.dir = "auto";
assert_true(tree.matches(":dir(ltr)"), "after change 7");
aleph1.dir = "invalid";
assert_false(tree.matches(":dir(ltr)"), "after change 8");
}, "dynamic changes to subtrees excluded as a result of the dir attribute");

0 comments on commit bd32f8e

Please sign in to comment.