Fix styling of th elements when explicitly specifiying text-align:inherit
https://bugs.webkit.org/show_bug.cgi?id=138577
Patch by Ryan Reno <rreno@apple.com> on 2022-06-16
Reviewed by Tim Nguyen.
<th> elements were being incorrectly centered when specifying
text-align: inherit. This fixes that bug by adding a new internal CSS
value for use in the UA stylesheet. This also removes a non-inherited
flag that was meant to be used for detecting this special case but ultimately didn't
work due to conflicts with the `all` property.
* Source/WebCore/css/CSSValueKeywords.in:
* Source/WebCore/css/html.css:
(th):
* Source/WebCore/css/parser/CSSParserFastPaths.cpp:
(WebCore::CSSParserFastPaths::isValidKeywordPropertyAndValue):
* Source/WebCore/css/parser/CSSParserIdioms.cpp:
(WebCore::isValueAllowedInMode):
* Source/WebCore/rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::RenderStyle):
* Source/WebCore/rendering/style/RenderStyle.h:
(WebCore::RenderStyle::NonInheritedFlags::operator== const):
(WebCore::RenderStyle::hasExplicitlySetTextAlign const): Deleted.
(WebCore::RenderStyle::setHasExplicitlySetTextAlign): Deleted.
* Source/WebCore/style/StyleAdjuster.cpp:
(WebCore::Style::Adjuster::adjust const):
* Source/WebCore/style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::convertTextAlign):
* Source/WebCore/css/CSSProperties.json:
* Source/WebCore/style/StyleBuilderCustom.h:
(WebCore::Style::BuilderCustom::applyInitialTextAlign): Deleted.
(WebCore::Style::BuilderCustom::applyValueTextAlign): Deleted.
* LayoutTests/fast/css/internal-th-center-ua-only-expected.txt: Added.
* LayoutTests/fast/css/internal-th-center-ua-only.html: Added.
* LayoutTests/fast/table/center-th-when-parent-has-initial-text-align-expected.html:
* LayoutTests/fast/table/center-th-when-parent-has-initial-text-align.html:
Canonical link: https://commits.webkit.org/251630@main
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@295625 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/fast/css/internal-th-center-ua-only-expected.txt b/LayoutTests/fast/css/internal-th-center-ua-only-expected.txt
new file mode 100644
index 0000000..abdebbc
--- /dev/null
+++ b/LayoutTests/fast/css/internal-th-center-ua-only-expected.txt
@@ -0,0 +1,4 @@
+
+PASS "text-align" property does not support value "-internal-th-center".
+PASS "text-align" property cannot be set to "-internal-th-center" by the author stylesheet.
+
diff --git a/LayoutTests/fast/css/internal-th-center-ua-only.html b/LayoutTests/fast/css/internal-th-center-ua-only.html
new file mode 100644
index 0000000..5b2e549
--- /dev/null
+++ b/LayoutTests/fast/css/internal-th-center-ua-only.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+
+<style>
+th {
+ text-align: right;
+ text-align: -internal-th-center;
+}
+</style>
+<table>
+ <tr>
+ <th id="header"></th>
+ </tr>
+</table>
+
+<script>
+ test(function() {
+ assert_false(CSS.supports('text-align', '-internal-th-center'));
+ }, '"text-align" property does not support value "-internal-th-center".');
+
+ test(function() {
+ assert_equals(getComputedStyle(header).textAlign, 'right');
+ }, '"text-align" property cannot be set to "-internal-th-center" by the author stylesheet.');
+</script>
diff --git a/LayoutTests/fast/table/center-th-when-parent-has-initial-text-align-expected.html b/LayoutTests/fast/table/center-th-when-parent-has-initial-text-align-expected.html
index 6f388a7..51c4dc4 100644
--- a/LayoutTests/fast/table/center-th-when-parent-has-initial-text-align-expected.html
+++ b/LayoutTests/fast/table/center-th-when-parent-has-initial-text-align-expected.html
@@ -4,7 +4,7 @@
<title>This tests that th is text-align centered when parent has initial value.</title>
<style>
table {
- width: 500px; border: 1px solid green;
+ width: 500px; border: 1px solid green;
}
</style>
<table><tbody>
@@ -12,11 +12,13 @@
<tr><th style="text-align: left">th initial</th></tr>
<tr><th style="text-align: left">th start</th></tr>
<tr><th style="text-align: center">th center</th></tr>
+ <tr><th style="text-align: left">th initial</th></tr>
<tr><th style="text-align: right">th end</th></tr>
<tr><th style="text-align: center">tr initial</th></tr>
<tr><th style="text-align: center">tr start</th></tr>
<tr><th style="text-align: center">tr center</th></tr>
<tr><th style="text-align: right">tr end</th></tr>
+ <tr><th style="text-align: right">tr end</th></tr>
<tr><th style="text-align: left">tr initial, th initial</th></tr>
<tr><th style="text-align: left">tr initial, th start</th></tr>
<tr><th style="text-align: left">tr start, th start</th></tr>
diff --git a/LayoutTests/fast/table/center-th-when-parent-has-initial-text-align.html b/LayoutTests/fast/table/center-th-when-parent-has-initial-text-align.html
index 1c91d44..280a513 100644
--- a/LayoutTests/fast/table/center-th-when-parent-has-initial-text-align.html
+++ b/LayoutTests/fast/table/center-th-when-parent-has-initial-text-align.html
@@ -4,7 +4,7 @@
<title>This tests that th is text-align centered when parent has initial value.</title>
<style>
table {
- width: 500px; border: 1px solid green;
+ width: 500px; border: 1px solid green;
}
</style>
<table><tbody>
@@ -12,11 +12,13 @@
<tr><th style="text-align: initial">th initial</th></tr>
<tr><th style="text-align: start">th start</th></tr>
<tr><th style="text-align: center">th center</th></tr>
+ <tr><th style="text-align: inherit">th initial</th></tr>
<tr><th style="text-align: end">th end</th></tr>
<tr style="text-align: initial"><th>tr initial</th></tr>
<tr style="text-align: start"><th>tr start</th></tr>
<tr style="text-align: center"><th>tr center</th></tr>
<tr style="text-align: end"><th>tr end</th></tr>
+ <tr style="text-align: end"><th style="text-align: inherit">tr end</th></tr>
<tr style="text-align: initial"><th style="text-align: initial">tr initial, th initial</th></tr>
<tr style="text-align: initial"><th style="text-align: start">tr initial, th start</th></tr>
<tr style="text-align: start"><th style="text-align: start">tr start, th start</th></tr>
diff --git a/Source/WebCore/css/CSSProperties.json b/Source/WebCore/css/CSSProperties.json
index ddcc146..0a99924 100644
--- a/Source/WebCore/css/CSSProperties.json
+++ b/Source/WebCore/css/CSSProperties.json
@@ -4587,8 +4587,7 @@
}
],
"codegen-properties": {
- "converter": "TextAlign",
- "custom": "Initial|Value"
+ "converter": "TextAlign"
},
"specification": {
"category": "css-22",
diff --git a/Source/WebCore/css/CSSValueKeywords.in b/Source/WebCore/css/CSSValueKeywords.in
index 0569181..742e530 100644
--- a/Source/WebCore/css/CSSValueKeywords.in
+++ b/Source/WebCore/css/CSSValueKeywords.in
@@ -345,6 +345,7 @@
-webkit-center
match-parent
-webkit-match-parent
+-internal-th-center
//start
//end
//
diff --git a/Source/WebCore/css/html.css b/Source/WebCore/css/html.css
index d0b8593..6518bfc 100644
--- a/Source/WebCore/css/html.css
+++ b/Source/WebCore/css/html.css
@@ -269,6 +269,7 @@
th {
font-weight: bold;
+ text-align: -internal-th-center;
}
caption {
diff --git a/Source/WebCore/css/parser/CSSParserFastPaths.cpp b/Source/WebCore/css/parser/CSSParserFastPaths.cpp
index 2d6bab2..adb5e3c 100644
--- a/Source/WebCore/css/parser/CSSParserFastPaths.cpp
+++ b/Source/WebCore/css/parser/CSSParserFastPaths.cpp
@@ -715,7 +715,7 @@
case CSSPropertyTableLayout: // auto | fixed
return valueID == CSSValueAuto || valueID == CSSValueFixed;
case CSSPropertyTextAlign:
- return (valueID >= CSSValueWebkitAuto && valueID <= CSSValueWebkitMatchParent) || valueID == CSSValueStart || valueID == CSSValueEnd;
+ return (valueID >= CSSValueWebkitAuto && valueID <= CSSValueInternalThCenter) || valueID == CSSValueStart || valueID == CSSValueEnd;
case CSSPropertyTextAlignLast:
// auto | start | end | left | right | center | justify | match-parent
return (valueID >= CSSValueLeft && valueID <= CSSValueJustify) || valueID == CSSValueStart || valueID == CSSValueEnd || valueID == CSSValueAuto || valueID == CSSValueMatchParent;
diff --git a/Source/WebCore/css/parser/CSSParserIdioms.cpp b/Source/WebCore/css/parser/CSSParserIdioms.cpp
index 6544f26..4a1fcba 100644
--- a/Source/WebCore/css/parser/CSSParserIdioms.cpp
+++ b/Source/WebCore/css/parser/CSSParserIdioms.cpp
@@ -40,6 +40,8 @@
return isUASheetBehavior(mode);
case CSSValueWebkitFocusRingColor:
return isUASheetBehavior(mode) || isQuirksModeBehavior(mode);
+ case CSSValueInternalThCenter:
+ return isUASheetBehavior(mode);
default:
return true;
}
diff --git a/Source/WebCore/rendering/style/RenderStyle.cpp b/Source/WebCore/rendering/style/RenderStyle.cpp
index bcb1b93..e397e9c 100644
--- a/Source/WebCore/rendering/style/RenderStyle.cpp
+++ b/Source/WebCore/rendering/style/RenderStyle.cpp
@@ -197,7 +197,6 @@
m_nonInheritedFlags.hasExplicitlySetBorderTopRightRadius = false;
m_nonInheritedFlags.hasExplicitlySetDirection = false;
m_nonInheritedFlags.hasExplicitlySetWritingMode = false;
- m_nonInheritedFlags.hasExplicitlySetTextAlign = false;
m_nonInheritedFlags.usesViewportUnits = false;
m_nonInheritedFlags.usesContainerUnits = false;
m_nonInheritedFlags.hasExplicitlyInheritedProperties = false;
diff --git a/Source/WebCore/rendering/style/RenderStyle.h b/Source/WebCore/rendering/style/RenderStyle.h
index 7ef353b..06b17e5 100644
--- a/Source/WebCore/rendering/style/RenderStyle.h
+++ b/Source/WebCore/rendering/style/RenderStyle.h
@@ -1570,9 +1570,6 @@
bool hasExplicitlySetWritingMode() const { return m_nonInheritedFlags.hasExplicitlySetWritingMode; }
void setHasExplicitlySetWritingMode(bool v) { m_nonInheritedFlags.hasExplicitlySetWritingMode = v; }
- bool hasExplicitlySetTextAlign() const { return m_nonInheritedFlags.hasExplicitlySetTextAlign; }
- void setHasExplicitlySetTextAlign(bool v) { m_nonInheritedFlags.hasExplicitlySetTextAlign = v; }
-
// A unique style is one that has matches something that makes it impossible to share.
bool unique() const { return m_nonInheritedFlags.isUnique; }
void setUnique() { m_nonInheritedFlags.isUnique = true; }
@@ -1990,7 +1987,6 @@
unsigned hasExplicitlySetBorderTopRightRadius : 1;
unsigned hasExplicitlySetDirection : 1;
unsigned hasExplicitlySetWritingMode : 1;
- unsigned hasExplicitlySetTextAlign : 1;
#if ENABLE(DARK_MODE_CSS)
unsigned hasExplicitlySetColorScheme : 1;
#endif
@@ -2132,7 +2128,6 @@
&& hasExplicitlySetBorderTopRightRadius == other.hasExplicitlySetBorderTopRightRadius
&& hasExplicitlySetDirection == other.hasExplicitlySetDirection
&& hasExplicitlySetWritingMode == other.hasExplicitlySetWritingMode
- && hasExplicitlySetTextAlign == other.hasExplicitlySetTextAlign
#if ENABLE(DARK_MODE_CSS)
&& hasExplicitlySetColorScheme == other.hasExplicitlySetColorScheme
#endif
diff --git a/Source/WebCore/style/StyleAdjuster.cpp b/Source/WebCore/style/StyleAdjuster.cpp
index 55478f0..589e9b8 100644
--- a/Source/WebCore/style/StyleAdjuster.cpp
+++ b/Source/WebCore/style/StyleAdjuster.cpp
@@ -315,13 +315,6 @@
style.setFloating(Float::None);
}
- // User agents are expected to have a rule in their user agent stylesheet that matches th elements that have a parent
- // node whose computed value for the 'text-align' property is its initial value, whose declaration block consists of
- // just a single declaration that sets the 'text-align' property to the value 'center'.
- // https://html.spec.whatwg.org/multipage/rendering.html#rendering
- if (m_element->hasTagName(thTag) && !style.hasExplicitlySetTextAlign() && m_parentStyle.textAlign() == RenderStyle::initialTextAlign())
- style.setTextAlign(TextAlignMode::Center);
-
if (m_element->hasTagName(legendTag))
style.setEffectiveDisplay(DisplayType::Block);
}
diff --git a/Source/WebCore/style/StyleBuilderConverter.h b/Source/WebCore/style/StyleBuilderConverter.h
index b0abcd9..ecdf339 100644
--- a/Source/WebCore/style/StyleBuilderConverter.h
+++ b/Source/WebCore/style/StyleBuilderConverter.h
@@ -610,23 +610,37 @@
inline TextAlignMode BuilderConverter::convertTextAlign(BuilderState& builderState, const CSSValue& value)
{
- auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
+ const auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
ASSERT(primitiveValue.isValueID());
- if (primitiveValue.valueID() != CSSValueWebkitMatchParent && primitiveValue.valueID() != CSSValueMatchParent)
- return primitiveValue;
+ const auto& parentStyle = builderState.parentStyle();
- auto* element = builderState.element();
- if (element && element == builderState.document().documentElement())
- return TextAlignMode::Start;
+ // User agents are expected to have a rule in their user agent stylesheet that matches th elements that have a parent
+ // node whose computed value for the 'text-align' property is its initial value, whose declaration block consists of
+ // just a single declaration that sets the 'text-align' property to the value 'center'.
+ // https://html.spec.whatwg.org/multipage/rendering.html#rendering
+ if (primitiveValue.valueID() == CSSValueInternalThCenter) {
+ if (parentStyle.textAlign() == RenderStyle::initialTextAlign())
+ return TextAlignMode::Center;
+ return parentStyle.textAlign();
+ }
- auto& parentStyle = builderState.parentStyle();
- if (parentStyle.textAlign() == TextAlignMode::Start)
- return parentStyle.isLeftToRightDirection() ? TextAlignMode::Left : TextAlignMode::Right;
- if (parentStyle.textAlign() == TextAlignMode::End)
- return parentStyle.isLeftToRightDirection() ? TextAlignMode::Right : TextAlignMode::Left;
- return parentStyle.textAlign();
+ if (primitiveValue.valueID() == CSSValueWebkitMatchParent || primitiveValue.valueID() == CSSValueMatchParent) {
+ const auto* element = builderState.element();
+
+ if (element && element == builderState.document().documentElement())
+ return TextAlignMode::Start;
+ if (parentStyle.textAlign() == TextAlignMode::Start)
+ return parentStyle.isLeftToRightDirection() ? TextAlignMode::Left : TextAlignMode::Right;
+ if (parentStyle.textAlign() == TextAlignMode::End)
+ return parentStyle.isLeftToRightDirection() ? TextAlignMode::Right : TextAlignMode::Left;
+
+ return parentStyle.textAlign();
+ }
+
+ return primitiveValue;
}
+
inline TextAlignLast BuilderConverter::convertTextAlignLast(BuilderState& builderState, const CSSValue& value)
{
auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
diff --git a/Source/WebCore/style/StyleBuilderCustom.h b/Source/WebCore/style/StyleBuilderCustom.h
index b61b81e..a0fc700 100644
--- a/Source/WebCore/style/StyleBuilderCustom.h
+++ b/Source/WebCore/style/StyleBuilderCustom.h
@@ -136,10 +136,6 @@
static void applyInheritVerticalAlign(BuilderState&);
static void applyValueVerticalAlign(BuilderState&, CSSValue&);
- // Custom handling of initial + value only.
- static void applyInitialTextAlign(BuilderState&);
- static void applyValueTextAlign(BuilderState&, CSSValue&);
-
// Custom handling of value setting only.
static void applyValueBaselineShift(BuilderState&, CSSValue&);
static void applyValueDirection(BuilderState&, CSSValue&);
@@ -192,18 +188,6 @@
builderState.style().setHasExplicitlySetDirection(true);
}
-inline void BuilderCustom::applyInitialTextAlign(BuilderState& builderState)
-{
- builderState.style().setTextAlign(RenderStyle::initialTextAlign());
- builderState.style().setHasExplicitlySetTextAlign(true);
-}
-
-inline void BuilderCustom::applyValueTextAlign(BuilderState& builderState, CSSValue& value)
-{
- builderState.style().setTextAlign(BuilderConverter::convertTextAlign(builderState, value));
- builderState.style().setHasExplicitlySetTextAlign(true);
-}
-
inline void BuilderCustom::resetEffectiveZoom(BuilderState& builderState)
{
// Reset the zoom in effect. This allows the setZoom method to accurately compute a new zoom in effect.
