Merge r222588 - Minimum font size may cause elements to have an infin…

…ite line-height https://bugs.webkit.org/show_bug.cgi?id=177573 <rdar://problem/34573792> Reviewed by Dan Bernstein. Source/WebCore: When minimum font size is specified, we were trying to preserve the ratio of specified font-size and specified line-height in order to boost the computed font size proportionately to the font-size boost. However, this doesn't work when the specified font-size is 0, because the ratio between line-height and font-size is infinite. The most straightforward solution is just to make small font-sizes opt out of the line-height adjustment because the result would be too big. Test: fast/text/line-height-minimumFontSize-text-small-font-size.html * css/StyleBuilderCustom.h: (WebCore::computeLineHeightMultiplierDueToFontSize): (WebCore::StyleBuilderCustom::applyValueLineHeight): LayoutTests: * fast/text/line-height-minimumFontSize-text-small-font-size-expected.txt: Added. * fast/text/line-height-minimumFontSize-text-small-font-size.html: Added.
WebKit · Oct 17, 2017 · 16eac30 · 16eac30
1 parent da0c1a8
commit 16eac30
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 6 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2017-09-27  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Minimum font size may cause elements to have an infinite line-height
+        https://bugs.webkit.org/show_bug.cgi?id=177573
+        <rdar://problem/34573792>
+
+        Reviewed by Dan Bernstein.
+
+        * fast/text/line-height-minimumFontSize-text-small-font-size-expected.txt: Added.
+        * fast/text/line-height-minimumFontSize-text-small-font-size.html: Added.
+
 2017-09-27  Myles C. Maxfield  <mmaxfield@apple.com>
 
         "Tag" codepoints require the complex text codepath

diff --git a/LayoutTests/fast/text/line-height-minimumFontSize-text-small-font-size-expected.txt b/LayoutTests/fast/text/line-height-minimumFontSize-text-small-font-size-expected.txt
@@ -0,0 +1,5 @@
+PASS document.getElementById('target').offsetHeight < 100 is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Hi
diff --git a/LayoutTests/fast/text/line-height-minimumFontSize-text-small-font-size.html b/LayoutTests/fast/text/line-height-minimumFontSize-text-small-font-size.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals) {
+    internals.settings.setMinimumFontSize(32);
+}
+</script>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body style="font-size: 16px;">
+<div id="target" style="font-size: 0px; line-height: 1rem;">Hi</div>
+<script>
+shouldBeTrue("document.getElementById('target').offsetHeight < 100");
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2017-09-27  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Minimum font size may cause elements to have an infinite line-height
+        https://bugs.webkit.org/show_bug.cgi?id=177573
+        <rdar://problem/34573792>
+
+        Reviewed by Dan Bernstein.
+
+        When minimum font size is specified, we were trying to preserve the ratio of specified font-size
+        and specified line-height in order to boost the computed font size proportionately to the font-size
+        boost. However, this doesn't work when the specified font-size is 0, because the ratio between
+        line-height and font-size is infinite.
+
+        The most straightforward solution is just to make small font-sizes opt out of the line-height
+        adjustment because the result would be too big.
+
+        Test: fast/text/line-height-minimumFontSize-text-small-font-size.html
+
+        * css/StyleBuilderCustom.h:
+        (WebCore::computeLineHeightMultiplierDueToFontSize):
+        (WebCore::StyleBuilderCustom::applyValueLineHeight):
+
 2017-09-27  Myles C. Maxfield  <mmaxfield@apple.com>
 
         "Tag" codepoints require the complex text codepath

diff --git a/Source/WebCore/css/StyleBuilderCustom.h b/Source/WebCore/css/StyleBuilderCustom.h
@@ -670,7 +670,9 @@ static inline float computeLineHeightMultiplierDueToFontSize(const Document& doc
         auto minimumFontSize = document.settings().minimumFontSize();
         if (minimumFontSize > 0) {
             auto specifiedFontSize = computeBaseSpecifiedFontSize(document, style, percentageAutosizingEnabled);
-            if (specifiedFontSize < minimumFontSize) {
+            // Small font sizes cause a preposterously large (near infinity) line-height. Add a fuzz-factor of 1px which opts out of
+            // boosted line-height.
+            if (specifiedFontSize < minimumFontSize && specifiedFontSize >= 1) {
                 // FIXME: There are two settings which are relevant here: minimum font size, and minimum logical font size (as
                 // well as things like the zoom property, text zoom on the page, and text autosizing). The minimum logical font
                 // size is nonzero by default, and already incorporated into the computed font size, so if we just use the ratio
@@ -706,11 +708,8 @@ inline void StyleBuilderCustom::applyValueLineHeight(StyleResolver& styleResolve
         auto multiplier = computeLineHeightMultiplierDueToFontSize(styleResolver.document(), *styleResolver.style(), primitiveValue);
         if (multiplier == 1)
             computedLineHeight = lineHeight.value();
-        else {
-            std::optional<Length> lineHeight = StyleBuilderConverter::convertLineHeight(styleResolver, value, multiplier);
-            ASSERT(static_cast<bool>(lineHeight));
-            computedLineHeight = lineHeight.value();
-        }
+        else
+            computedLineHeight = StyleBuilderConverter::convertLineHeight(styleResolver, value, multiplier).value();
     }
 
     styleResolver.style()->setLineHeight(WTFMove(computedLineHeight));