[Text autosizing] [iPadOS] Product label text is clipped in portrait mode on the...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Jul 2019 02:51:22 +0000 (02:51 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Jul 2019 02:51:22 +0000 (02:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199806
<rdar://problem/52902482>

Reviewed by Zalan Bujtas.

Source/WebCore:

On sephora.com, some product label text is currently boosted by idempotent text autosizing, which causes the
labels be vertically clipped. This patch augments the idempotent text autosizing heuristic to avoid this case by
checking if the element to be boosted has a fixed height or max height, whose value is very close to a small
integer multiple of the line height. In this case, it's likely that the website expects the text to be no more
than a few lines' worth of height, so boosting the text is likely to break the page.

Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html

* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::isIdempotentTextAutosizingCandidate const):
* rendering/style/TextSizeAdjustment.cpp:
(WebCore::AutosizeStatus::updateStatus):
* rendering/style/TextSizeAdjustment.h:

Rename Fields::DisplayNone to Fields::AvoidSubtree to avoid introducing another bit in RenderStyle's inherited
flags.

LayoutTests:

Add a new test case to an existing layout test, which mimics the product label text on sephora.com's front page.

* fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt:
* fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247467 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt
LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html
Source/WebCore/ChangeLog
Source/WebCore/rendering/style/RenderStyle.cpp
Source/WebCore/rendering/style/TextSizeAdjustment.cpp
Source/WebCore/rendering/style/TextSizeAdjustment.h

index df7c161..9f4e7db 100644 (file)
@@ -1,3 +1,16 @@
+2019-07-15  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Text autosizing] [iPadOS] Product label text is clipped in portrait mode on the front page of sephora.com
+        https://bugs.webkit.org/show_bug.cgi?id=199806
+        <rdar://problem/52902482>
+
+        Reviewed by Zalan Bujtas.
+
+        Add a new test case to an existing layout test, which mimics the product label text on sephora.com's front page.
+
+        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates-expected.txt:
+        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html:
+
 2019-07-11  Myles C. Maxfield  <mmaxfield@apple.com>
 
         New York font erroneously gets synthetic bold
index 6207278..464d944 100644 (file)
@@ -32,6 +32,8 @@ Checking target16:
 PASS result is >= 13
 Checking target17:
 PASS result is 12
+Checking target18:
+PASS result is 12
 PASS successfullyParsed is true
 
 TEST COMPLETE
@@ -51,3 +53,6 @@ Test
 Test
 Test
 Test
+Test 
+Test 
+Test
index 90ae44a..8c588ef 100644 (file)
@@ -28,6 +28,13 @@ if (window.internals) {
 <div style="background: green;"><span id="target15" style="font-size: 12px; overflow-y: hidden; float: right;">Test</span></div>
 <div style="background: green;"><span id="target16" style="font-size: 12px; float: right;">Test</span></div>
 <div style="background: green;"><span id="target17" style="font-size: 12px; -webkit-text-size-adjust: 100%;">Test</span></div>
+<div style="background: green; line-height: 1.25; max-height: 45px; font-size: 12px;">
+    <span>Test</span>
+    <br>
+    <span id="target18">Test</span>
+    <br>
+    <span>Test</span>
+</div>
 <script>
 let result;
 function check(name, shouldGetAutosized) {
@@ -69,6 +76,7 @@ check("target14", false);
 check("target15", true);
 check("target16", true);
 check("target17", false);
+check("target18", false);
 </script>
 <script src="../../../../resources/js-test-post.js"></script>
 </body>
index 37eb615..3851112 100644 (file)
@@ -1,3 +1,28 @@
+2019-07-15  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Text autosizing] [iPadOS] Product label text is clipped in portrait mode on the front page of sephora.com
+        https://bugs.webkit.org/show_bug.cgi?id=199806
+        <rdar://problem/52902482>
+
+        Reviewed by Zalan Bujtas.
+
+        On sephora.com, some product label text is currently boosted by idempotent text autosizing, which causes the
+        labels be vertically clipped. This patch augments the idempotent text autosizing heuristic to avoid this case by
+        checking if the element to be boosted has a fixed height or max height, whose value is very close to a small
+        integer multiple of the line height. In this case, it's likely that the website expects the text to be no more
+        than a few lines' worth of height, so boosting the text is likely to break the page.
+
+        Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidates.html
+
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::isIdempotentTextAutosizingCandidate const):
+        * rendering/style/TextSizeAdjustment.cpp:
+        (WebCore::AutosizeStatus::updateStatus):
+        * rendering/style/TextSizeAdjustment.h:
+
+        Rename Fields::DisplayNone to Fields::AvoidSubtree to avoid introducing another bit in RenderStyle's inherited
+        flags.
+
 2019-07-15  Myles C. Maxfield  <mmaxfield@apple.com>
 
         New York font erroneously gets synthetic bold
index 2d2e83a..04c977b 100644 (file)
@@ -496,7 +496,7 @@ bool RenderStyle::isIdempotentTextAutosizingCandidate() const
 {
     // Refer to <rdar://problem/51826266> for more information regarding how this function was generated.
     auto fields = OptionSet<AutosizeStatus::Fields>::fromRaw(m_inheritedFlags.autosizeStatus);
-    if (fields.contains(AutosizeStatus::Fields::DisplayNone))
+    if (fields.contains(AutosizeStatus::Fields::AvoidSubtree))
         return false;
 
     if (fields.contains(AutosizeStatus::Fields::FixedHeight)) {
index bd2130e..0528027 100644 (file)
@@ -46,8 +46,36 @@ void AutosizeStatus::updateStatus(RenderStyle& style)
 {
     auto result = style.autosizeStatus().fields();
 
-    if (style.display() == DisplayType::None)
-        result.add(Fields::DisplayNone);
+    auto shouldAvoidAutosizingEntireSubtree = [&] {
+        if (style.display() == DisplayType::None)
+            return true;
+
+        if (!style.lineHeight().isSpecified() || style.whiteSpace() == WhiteSpace::NoWrap)
+            return false;
+
+        Optional<Length> heightOrMaxHeightAsLength;
+        if (style.height().isFixed() && style.maxHeight().isAuto())
+            heightOrMaxHeightAsLength = style.height();
+        else if (style.maxHeight().isFixed())
+            heightOrMaxHeightAsLength = style.maxHeight();
+
+        if (!heightOrMaxHeightAsLength)
+            return false;
+
+        float heightOrMaxHeight = heightOrMaxHeightAsLength->value();
+        float computedLineHeight = style.computedLineHeight();
+        if (computedLineHeight <= 0)
+            return false;
+
+        float approximateNumberOfLines = heightOrMaxHeight / computedLineHeight;
+        const int maximumNumberOfLines = 5;
+        const float thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger = 0.01;
+        return approximateNumberOfLines <= maximumNumberOfLines + thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger
+            && approximateNumberOfLines - std::floor(approximateNumberOfLines) <= thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger;
+    };
+
+    if (shouldAvoidAutosizingEntireSubtree())
+        result.add(Fields::AvoidSubtree);
 
     if (style.height().isFixed())
         result.add(Fields::FixedHeight);
index 55f2f62..552ca41 100644 (file)
@@ -52,7 +52,7 @@ private:
 class AutosizeStatus {
 public:
     enum class Fields : uint8_t {
-        DisplayNone = 1 << 0,
+        AvoidSubtree = 1 << 0,
         FixedHeight = 1 << 1,
         FixedWidth = 1 << 2,
         Floating = 1 << 3,