Skip to content

Commit

Permalink
Invalid dir attribute should not affect directionality
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=244439

Reviewed by Darin Adler and Alan Bujtas.

An invalid dir content attribute value should not have any effect on the directionality of text.

The issue was caused by WebKit adding unicode-bidi: embed style whenever there is dir content attribute
regardless of whether its value is valid or not.

New behavior of WebKit matches that of Gecko and Blink.

* LayoutTests/fast/css/default-bidi-css-rules-expected.txt: Rebaselined.
* LayoutTests/fast/css/default-bidi-css-rules.html: Ditto.
* LayoutTests/fast/dom/HTMLElement/attr-dir-invalid-expected.html: Added.
* LayoutTests/fast/dom/HTMLElement/attr-dir-invalid.html: Added.
* Source/WebCore/html/HTMLElement.cpp:
(WebCore::HTMLElement::collectPresentationalHintsForAttribute):

Canonical link: https://commits.webkit.org/253878@main
  • Loading branch information
rniwa committed Aug 28, 2022
1 parent b76609a commit 7107058
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 17 deletions.
8 changes: 4 additions & 4 deletions LayoutTests/fast/css/default-bidi-css-rules-expected.txt
Expand Up @@ -9,7 +9,7 @@ PASS styleOf("div", {"dir":"rtl"}).unicodeBidi is "isolate"
PASS styleOf("div", {"dir":"auto"}).direction is "ltr"
PASS styleOf("div", {"dir":"auto"}).unicodeBidi is "isolate"
PASS styleOf("div", {"dir":""}).direction is "ltr"
PASS styleOf("div", {"dir":""}).unicodeBidi is "embed"
PASS styleOf("div", {"dir":""}).unicodeBidi is "normal"
PASS styleOf("span", {}).direction is "ltr"
PASS styleOf("span", {}).unicodeBidi is "normal"
PASS styleOf("span", {"dir":"ltr"}).direction is "ltr"
Expand All @@ -19,7 +19,7 @@ PASS styleOf("span", {"dir":"rtl"}).unicodeBidi is "isolate"
PASS styleOf("span", {"dir":"auto"}).direction is "ltr"
PASS styleOf("span", {"dir":"auto"}).unicodeBidi is "isolate"
PASS styleOf("span", {"dir":""}).direction is "ltr"
PASS styleOf("span", {"dir":""}).unicodeBidi is "embed"
PASS styleOf("span", {"dir":""}).unicodeBidi is "normal"
PASS styleOf("bdi", {}).direction is "ltr"
PASS styleOf("bdi", {}).unicodeBidi is "isolate"
PASS styleOf("bdi", {"dir":"ltr"}).direction is "ltr"
Expand Down Expand Up @@ -59,7 +59,7 @@ PASS styleOf("textarea", {"dir":"rtl"}).unicodeBidi is "isolate"
PASS styleOf("textarea", {"dir":"auto"}).direction is "ltr"
PASS styleOf("textarea", {"dir":"auto"}).unicodeBidi is "plaintext"
PASS styleOf("textarea", {"dir":""}).direction is "ltr"
PASS styleOf("textarea", {"dir":""}).unicodeBidi is "embed"
PASS styleOf("textarea", {"dir":""}).unicodeBidi is "normal"
PASS styleOf("pre", {}).direction is "ltr"
PASS styleOf("pre", {}).unicodeBidi is "normal"
PASS styleOf("pre", {"dir":"ltr"}).direction is "ltr"
Expand All @@ -69,7 +69,7 @@ PASS styleOf("pre", {"dir":"rtl"}).unicodeBidi is "isolate"
PASS styleOf("pre", {"dir":"auto"}).direction is "ltr"
PASS styleOf("pre", {"dir":"auto"}).unicodeBidi is "plaintext"
PASS styleOf("pre", {"dir":""}).direction is "ltr"
PASS styleOf("pre", {"dir":""}).unicodeBidi is "embed"
PASS styleOf("pre", {"dir":""}).unicodeBidi is "normal"
PASS successfullyParsed is true

TEST COMPLETE
Expand Down
8 changes: 4 additions & 4 deletions LayoutTests/fast/css/default-bidi-css-rules.html
Expand Up @@ -24,13 +24,13 @@
['div', {'dir': 'ltr'}, 'ltr', 'isolate'],
['div', {'dir': 'rtl'}, 'rtl', 'isolate'],
['div', {'dir': 'auto'}, 'ltr', 'isolate'],
['div', {'dir': ''}, 'ltr', 'embed'],
['div', {'dir': ''}, 'ltr', 'normal'],

['span', {}, 'ltr', 'normal'],
['span', {'dir': 'ltr'}, 'ltr', 'isolate'],
['span', {'dir': 'rtl'}, 'rtl', 'isolate'],
['span', {'dir': 'auto'}, 'ltr', 'isolate'],
['span', {'dir': ''}, 'ltr', 'embed'],
['span', {'dir': ''}, 'ltr', 'normal'],

['bdi', {}, 'ltr', 'isolate'],
['bdi', {'dir': 'ltr'}, 'ltr', 'isolate'],
Expand All @@ -54,13 +54,13 @@
['textarea', {'dir': 'ltr'}, 'ltr', 'isolate'],
['textarea', {'dir': 'rtl'}, 'rtl', 'isolate'],
['textarea', {'dir': 'auto'}, 'ltr', 'plaintext'],
['textarea', {'dir': ''}, 'ltr', 'embed'],
['textarea', {'dir': ''}, 'ltr', 'normal'],

['pre', {}, 'ltr', 'normal'],
['pre', {'dir': 'ltr'}, 'ltr', 'isolate'],
['pre', {'dir': 'rtl'}, 'rtl', 'isolate'],
['pre', {'dir': 'auto'}, 'ltr', 'plaintext'],
['pre', {'dir': ''}, 'ltr', 'embed'],
['pre', {'dir': ''}, 'ltr', 'normal'],
].forEach(function (test) {
shouldBe('styleOf("' + test[0] + '", ' + JSON.stringify(test[1]) + ').direction', '"' + test[2] + '"');
container.innerHTML = '';
Expand Down
12 changes: 12 additions & 0 deletions LayoutTests/fast/dom/HTMLElement/attr-dir-invalid-expected.html
@@ -0,0 +1,12 @@
<!DOCTYPE html>
<html>
<body>
<div id="container" dir="auto">
<span id="rtl">&#1514;</span><span>b</span>
<span>a</span>
</div>
<style>
#container { width: 100px; }
</style>
</body>
</html>
25 changes: 25 additions & 0 deletions LayoutTests/fast/dom/HTMLElement/attr-dir-invalid.html
@@ -0,0 +1,25 @@
<!DOCTYPE html>
<html class="reftest-wait">
<body>
<div id="container" dir="auto">
<span id="rtl" dir="x">&#1514;</span>
<span>a</span>
</div>
<style>
#container { width: 100px; }
</style>
<script>

requestAnimationFrame(() => {
setTimeout(() => {
rtl.dir = 'x';
const span = document.createElement('span');
span.append('b');
rtl.after(span);
document.documentElement.className = '';
}, 0);
});

</script>
</body>
</html>
12 changes: 3 additions & 9 deletions Source/WebCore/html/HTMLElement.cpp
Expand Up @@ -218,16 +218,10 @@ void HTMLElement::collectPresentationalHintsForAttribute(const QualifiedName& na
} else if (name == dirAttr) {
if (equalLettersIgnoringASCIICase(value, "auto"_s))
addPropertyToPresentationalHintStyle(style, CSSPropertyUnicodeBidi, unicodeBidiAttributeForDirAuto(*this));
else {
auto unicodeBidiValue = CSSValueEmbed;

if (equalLettersIgnoringASCIICase(value, "rtl"_s) || equalLettersIgnoringASCIICase(value, "ltr"_s)) {
addPropertyToPresentationalHintStyle(style, CSSPropertyDirection, value);
unicodeBidiValue = CSSValueIsolate;
}

else if (equalLettersIgnoringASCIICase(value, "rtl"_s) || equalLettersIgnoringASCIICase(value, "ltr"_s)) {
addPropertyToPresentationalHintStyle(style, CSSPropertyDirection, value);
if (!hasTagName(bdiTag) && !hasTagName(bdoTag) && !hasTagName(outputTag))
addPropertyToPresentationalHintStyle(style, CSSPropertyUnicodeBidi, unicodeBidiValue);
addPropertyToPresentationalHintStyle(style, CSSPropertyUnicodeBidi, CSSValueIsolate);
}
} else if (name.matches(XMLNames::langAttr))
mapLanguageAttributeToLocale(value, style);
Expand Down

0 comments on commit 7107058

Please sign in to comment.