Treat 'rem' and 'rlh' as absolute units for font size

https://bugs.webkit.org/show_bug.cgi?id=246566 rdar://101209364 Reviewed by Darin Adler. Ported Chromium revision from: https://chromium.googlesource.com/chromium/src/+/17ea96288ad2c9abb048f5b50e02097d20c283d9 by Xiaocheng Hu <xiaochengh@chromium.org> StyleBuilderCustom does a size adjustment for monospace that, for all elements using monospace font family and relative font sizes (em, ex, ch etc), their computed font-size values are scaled by a fixed factor. To make sure this doesn't affect non-monospace children using relative font sizes, we also divide the childrens' font sizes by the same factor. See usages of FontCascadeDescription::useFixedDefaultSize(). However, this causes children using 'rem' to be also scaled, because 'rem' is also considered a font-relative length unit, even though it doesn't depend on the parent font size. Therefore, we fix it by treating 'rem' an absolute unit for font size. This also applies to the rlh unit, which the Chromium fix did not cover, this revision adds a WPT for it. * LayoutTests/TestExpectations: * LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/rlh-in-monospace-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/rlh-in-monospace-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/rlh-in-monospace.html: Added. * Source/WebCore/css/CSSPrimitiveValue.h: * Source/WebCore/style/StyleBuilderCustom.h: (WebCore::Style::BuilderCustom::applyValueFontSize): Canonical link: https://commits.webkit.org/255594@main
WebKit · Oct 15, 2022 · bf519d1022247b0b33a3ce7f4c283da6fd74abff · bf519d1
1 parent 2f796e7
commit bf519d1022247b0b33a3ce7f4c283da6fd74abff
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 2 deletions.
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -3756,7 +3756,6 @@ webkit.org/b/206881 imported/w3c/web-platform-tests/css/css-fonts/font-face-weig
 webkit.org/b/206881 imported/w3c/web-platform-tests/css/css-fonts/font-face-weight-default-variable.html [ ImageOnlyFailure ]
 
 # Triaged css-fonts failures
-webkit.org/b/246566 imported/w3c/web-platform-tests/css/css-fonts/rem-in-monospace.html [ ImageOnlyFailure ]
 webkit.org/b/246565 imported/w3c/web-platform-tests/css/css-fonts/font-palette-21.html [ ImageOnlyFailure ]
 webkit.org/b/246564 imported/w3c/web-platform-tests/css/css-fonts/font-synthesis-small-caps-not-applied.html [ ImageOnlyFailure ]
 

diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/rlh-in-monospace-expected.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/rlh-in-monospace-expected.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<p>The following two lines should look exactly the same.</p>
+
+<div style="font-family: sans-serif; font-size: 1rlh">
+  Text.
+</div>
+
+<div style="font-family: sans-serif; font-size: 1rlh">
+  Text.
+</div>
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/rlh-in-monospace-ref.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/rlh-in-monospace-ref.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<p>The following two lines should look exactly the same.</p>
+
+<div style="font-family: sans-serif; font-size: 1rlh">
+  Text.
+</div>
+
+<div style="font-family: sans-serif; font-size: 1rlh">
+  Text.
+</div>
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/rlh-in-monospace.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/rlh-in-monospace.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<link rel="help" href="https://drafts.csswg.org/css-fonts-4/#font-size-prop">
+<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=623842">
+<link rel="help" href="https://bugs.webkit.org/show_bug.cgi?id=246566">
+<link rel="author" title="Tim Nguyen" href="https://github.com/nt1m">
+<link rel="match" href="rlh-in-monospace-ref.html">
+
+<p>The following two lines should look exactly the same.</p>
+
+<div style="font-family: monospace; font-size: 1.5em">
+  <div style="font-family: sans-serif; font-size: 1rlh">
+    Text.
+  </div>
+</div>
+
+<div style="font-family: sans-serif; font-size: 1rlh">
+  Text.
+</div>
diff --git a/Source/WebCore/css/CSSPrimitiveValue.h b/Source/WebCore/css/CSSPrimitiveValue.h
@@ -87,6 +87,7 @@ class CSSPrimitiveValue final : public CSSValue {
     bool isCounter() const { return primitiveUnitType() == CSSUnitType::CSS_COUNTER; }
     bool isFontIndependentLength() const { return isFontIndependentLength(primitiveUnitType()); }
     bool isFontRelativeLength() const { return isFontRelativeLength(primitiveUnitType()); }
+    bool isParentFontRelativeLength() const { return isPercentage() || (isFontRelativeLength() && primitiveType() != CSSUnitType::CSS_REMS && primitiveType() != CSSUnitType::CSS_RLHS); }
     bool isQuirkyEms() const { return primitiveType() == CSSUnitType::CSS_QUIRKY_EMS; }
     bool isLength() const { return isLength(static_cast<CSSUnitType>(primitiveType())); }
     bool isNumber() const { return primitiveType() == CSSUnitType::CSS_NUMBER; }

diff --git a/Source/WebCore/style/StyleBuilderCustom.h b/Source/WebCore/style/StyleBuilderCustom.h
@@ -1890,7 +1890,7 @@ inline void BuilderCustom::applyValueFontSize(BuilderState& builderState, CSSVal
             break;
         }
     } else {
-        fontDescription.setIsAbsoluteSize(parentIsAbsoluteSize || !(primitiveValue.isPercentage() || primitiveValue.isFontRelativeLength()));
+        fontDescription.setIsAbsoluteSize(parentIsAbsoluteSize || !primitiveValue.isParentFontRelativeLength());
         if (primitiveValue.isLength()) {
             auto conversionData = builderState.cssToLengthConversionData().copyForFontSize();
             size = primitiveValue.computeLength<float>(conversionData);