Attachment 372807 Details for Bug 198763 – Patch

[patch] Patch

bug-198763-20190624153411.patch (text/plain), 17.32 KB, created by Wenson Hsieh on 2019-06-24 15:34:12 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-06-24 15:34:12 PDT

Size: 17.32 KB

patch

obsolete

>Subversion Revision: 246765
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 44bf9b656e440ba411f56452db5bc696e881dd1b..4f25d37cc273d31f8921e0b6b23a2a112a008639 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,40 @@
>+2019-06-24  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [Text autosizing] [iPadOS] Revise our heuristics to determine idempotent text autosizing candidates
>+        https://bugs.webkit.org/show_bug.cgi?id=198763
>+        <rdar://problem/51826266>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch adjusts existing text autosizing heuristics, based on a survey of text on websites in the Alexa top
>+        500 that shrink down to fit the viewport when requesting the desktop version of the site. The new heuristic is
>+        derived from training decision trees against the dataset obtained from this survey, and balances false positives
>+        (cases where layout is broken due to autosizing) against overall accuracy (measured using cross-validation).
>+
>+        Please refer to the link in the radar for more details.
>+
>+        Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip.html
>+
>+        * css/StyleResolver.cpp:
>+        (WebCore::StyleResolver::adjustRenderStyleForTextAutosizing):
>+        * rendering/style/RenderStyle.cpp:
>+        (WebCore::RenderStyle::isIdempotentTextAutosizingCandidate const):
>+
>+        Rename AutosizeStatus::shouldSkipSubtree to RenderStyle::isIdempotentTextAutosizingCandidate. We relocate this
>+        logic to RenderStyle, such that we're able to ask the element's RenderStyle questions when determining whether
>+        the element should be autosized.
>+
>+        * rendering/style/RenderStyle.h:
>+
>+        Grow the width of `autosizeStatus` from 4 to 8 (notably, this does not increase the size of RenderStyle).
>+
>+        * rendering/style/TextSizeAdjustment.cpp:
>+        (WebCore::AutosizeStatus::updateStatus):
>+        (WebCore::AutosizeStatus::shouldSkipSubtree const): Deleted.
>+        * rendering/style/TextSizeAdjustment.h:
>+
>+        Introduce new text autosizing state flags, and remove some existing ones.
>+
> 2019-06-24  Chris Dumez  <cdumez@apple.com>
> 
>         Pages using Google's anti-flicker optimization may take ~5 seconds to do initial paint
>diff --git a/Source/WebCore/css/StyleResolver.cpp b/Source/WebCore/css/StyleResolver.cpp
>index 0248790eeb0a8823eaaa6a926046bbc488b01569..b3e6e1415f55af5aec5123f3ec90155ff2ebc2c0 100644
>--- a/Source/WebCore/css/StyleResolver.cpp
>+++ b/Source/WebCore/css/StyleResolver.cpp
>@@ -879,17 +879,17 @@ static bool hasTextChildren(const Element& element)
> 
> void StyleResolver::adjustRenderStyleForTextAutosizing(RenderStyle& style, const Element& element)
> {
>-    auto newAutosizeStatus = AutosizeStatus::updateStatus(style);
>     if (!settings().textAutosizingEnabled() || !settings().textAutosizingUsesIdempotentMode())
>         return;
> 
>+    AutosizeStatus::updateStatus(style);
>     if (!hasTextChildren(element))
>         return;
> 
>     if (style.textSizeAdjust().isNone())
>         return;
> 
>-    if (newAutosizeStatus.shouldSkipSubtree())
>+    if (!style.isIdempotentTextAutosizingCandidate())
>         return;
> 
>     float initialScale = document().page() ? document().page()->initialScale() : 1;
>diff --git a/Source/WebCore/rendering/style/RenderStyle.cpp b/Source/WebCore/rendering/style/RenderStyle.cpp
>index 14b1ca03874bc8d7261673d4071a23b74fb6ff77..d13b0890b985849111eeb66f0f2e93f1f49c5ecf 100644
>--- a/Source/WebCore/rendering/style/RenderStyle.cpp
>+++ b/Source/WebCore/rendering/style/RenderStyle.cpp
>@@ -492,6 +492,69 @@ bool RenderStyle::equalForTextAutosizing(const RenderStyle& other) const
>         && m_rareNonInheritedData->textOverflow == other.m_rareNonInheritedData->textOverflow;
> }
> 
>+bool RenderStyle::isIdempotentTextAutosizingCandidate() const
>+{
>+    auto fields = OptionSet<AutosizeStatus::Fields>::fromRaw(m_inheritedFlags.autosizeStatus);
>+    if (fields.contains(AutosizeStatus::Fields::DisplayNone))
>+        return false;
>+
>+    if (fields.contains(AutosizeStatus::Fields::FixedHeight)) {
>+        if (whiteSpace() == WhiteSpace::NoWrap) {
>+            if (fields.contains(AutosizeStatus::Fields::OutOfFlowPosition))
>+                return true;
>+
>+            if (fields.contains(AutosizeStatus::Fields::OverflowYHidden))
>+                return fields.contains(AutosizeStatus::Fields::FixedWidth) || !fields.contains(AutosizeStatus::Fields::Floating);
>+
>+            return true;
>+        }
>+
>+        if (fields.contains(AutosizeStatus::Fields::Floating)) {
>+            if (fields.contains(AutosizeStatus::Fields::OutOfFlowPosition)) {
>+                if (fields.contains(AutosizeStatus::Fields::OverflowXHidden))
>+                    return fields.contains(AutosizeStatus::Fields::FixedWidth);
>+
>+                return fields.contains(AutosizeStatus::Fields::FixedMaxWidth);
>+            }
>+
>+            return !fields.contains(AutosizeStatus::Fields::FixedWidth);
>+        }
>+
>+        if (fields.contains(AutosizeStatus::Fields::FixedWidth))
>+            return !fields.contains(AutosizeStatus::Fields::OutOfFlowPosition) && !fields.contains(AutosizeStatus::Fields::OverflowXHidden);
>+
>+        return true;
>+    }
>+
>+    if (fields.contains(AutosizeStatus::Fields::Floating))
>+        return fields.contains(AutosizeStatus::Fields::OverflowYHidden) || !fields.contains(AutosizeStatus::Fields::OverflowXHidden);
>+
>+    if (fields.contains(AutosizeStatus::Fields::FixedWidth)) {
>+        if (fields.contains(AutosizeStatus::Fields::OverflowYHidden))
>+            return true;
>+
>+        if (whiteSpace() == WhiteSpace::NoWrap)
>+            return false;
>+
>+        return !fields.contains(AutosizeStatus::Fields::OverflowXHidden);
>+    }
>+
>+    if (fields.contains(AutosizeStatus::Fields::OverflowXHidden))
>+        return whiteSpace() == WhiteSpace::NoWrap;
>+
>+    if (fields.contains(AutosizeStatus::Fields::FixedMaxWidth)) {
>+        if (whiteSpace() == WhiteSpace::NoWrap)
>+            return false;
>+
>+        return !fields.contains(AutosizeStatus::Fields::OutOfFlowPosition);
>+    }
>+
>+    if (fields.contains(AutosizeStatus::Fields::OutOfFlowPosition))
>+        return whiteSpace() == WhiteSpace::NoWrap;
>+
>+    return true;
>+}
>+
> AutosizeStatus RenderStyle::autosizeStatus() const
> {
>     return OptionSet<AutosizeStatus::Fields>::fromRaw(m_inheritedFlags.autosizeStatus);
>diff --git a/Source/WebCore/rendering/style/RenderStyle.h b/Source/WebCore/rendering/style/RenderStyle.h
>index ec30ea63fa8507972363e1d4b8a113fb71b5eed6..eb2a2f6f9aa2986022d5ad824fb12fa682cac0e1 100644
>--- a/Source/WebCore/rendering/style/RenderStyle.h
>+++ b/Source/WebCore/rendering/style/RenderStyle.h
>@@ -740,7 +740,8 @@ public:
> 
> #if ENABLE(TEXT_AUTOSIZING)
>     TextSizeAdjustment textSizeAdjust() const { return m_rareInheritedData->textSizeAdjust; }
>-    AutosizeStatus autosizeStatus() const;
>+    WEBCORE_EXPORT AutosizeStatus autosizeStatus() const;
>+    WEBCORE_EXPORT bool isIdempotentTextAutosizingCandidate() const;
> #endif
> 
>     TextSecurity textSecurity() const { return static_cast<TextSecurity>(m_rareInheritedData->textSecurity); }
>@@ -1834,9 +1835,9 @@ private:
>         // 48 bits
> 
> #if ENABLE(TEXT_AUTOSIZING)
>-        unsigned autosizeStatus : 4;
>+        unsigned autosizeStatus : 8;
> #endif
>-        // 52 bits
>+        // 56 bits
>     };
> 
>     // This constructor is used to implement the replace operation.
>diff --git a/Source/WebCore/rendering/style/TextSizeAdjustment.cpp b/Source/WebCore/rendering/style/TextSizeAdjustment.cpp
>index 020027800f62a1c8b4305946a7f79d31b7c63a81..cc1da80a5b9743670507e7f545853677589a1934 100644
>--- a/Source/WebCore/rendering/style/TextSizeAdjustment.cpp
>+++ b/Source/WebCore/rendering/style/TextSizeAdjustment.cpp
>@@ -42,30 +42,35 @@ bool AutosizeStatus::contains(Fields fields) const
>     return m_fields.contains(fields);
> }
> 
>-AutosizeStatus AutosizeStatus::updateStatus(RenderStyle& style)
>+void AutosizeStatus::updateStatus(RenderStyle& style)
> {
>-    OptionSet<Fields> result = style.autosizeStatus().fields();
>+    auto result = style.autosizeStatus().fields();
>+
>+    if (style.display() == DisplayType::None)
>+        result.add(Fields::DisplayNone);
>+
>     if (style.hasOutOfFlowPosition())
>-        result.add(Fields::FoundOutOfFlowPosition);
>-    switch (style.display()) {
>-    case DisplayType::InlineBlock:
>-        result.add(Fields::FoundInlineBlock);
>-        break;
>-    case DisplayType::None:
>-        result.add(Fields::FoundDisplayNone);
>-        break;
>-    default: // FIXME: Add more cases.
>-        break;
>-    }
>+        result.add(Fields::OutOfFlowPosition);
>+
>     if (style.height().isFixed())
>-        result.add(Fields::FoundFixedHeight);
>-    style.setAutosizeStatus(result);
>-    return result;
>-}
>+        result.add(Fields::FixedHeight);
> 
>-bool AutosizeStatus::shouldSkipSubtree() const
>-{
>-    return m_fields.containsAny({ Fields::FoundOutOfFlowPosition, Fields::FoundInlineBlock, Fields::FoundFixedHeight, Fields::FoundDisplayNone });
>+    if (style.width().isFixed())
>+        result.add(Fields::FixedWidth);
>+
>+    if (style.maxWidth().isFixed())
>+        result.add(Fields::FixedMaxWidth);
>+
>+    if (style.overflowX() == Overflow::Hidden)
>+        result.add(Fields::OverflowXHidden);
>+
>+    if (style.overflowY() == Overflow::Hidden)
>+        result.add(Fields::OverflowYHidden);
>+
>+    if (style.isFloating())
>+        result.add(Fields::Floating);
>+
>+    style.setAutosizeStatus(result);
> }
> 
> float AutosizeStatus::idempotentTextSize(float specifiedSize, float pageScale)
>diff --git a/Source/WebCore/rendering/style/TextSizeAdjustment.h b/Source/WebCore/rendering/style/TextSizeAdjustment.h
>index 9f6b3fb88b15702a2879ccf335f8f3b132462959..ffc0344b2b4faeb1ede4669b86518e0fddd726ff 100644
>--- a/Source/WebCore/rendering/style/TextSizeAdjustment.h
>+++ b/Source/WebCore/rendering/style/TextSizeAdjustment.h
>@@ -52,10 +52,14 @@ private:
> class AutosizeStatus {
> public:
>     enum class Fields : uint8_t {
>-        FoundOutOfFlowPosition = 1 << 0,
>-        FoundInlineBlock = 1 << 1,
>-        FoundFixedHeight = 1 << 2,
>-        FoundDisplayNone = 1 << 3
>+        DisplayNone = 1 << 0,
>+        FixedHeight = 1 << 1,
>+        FixedWidth = 1 << 2,
>+        Floating = 1 << 3,
>+        OverflowXHidden = 1 << 4,
>+        OverflowYHidden = 1 << 5,
>+        OutOfFlowPosition = 1 << 6,
>+        FixedMaxWidth = 1 << 7
>         // Adding new values requires giving RenderStyle::InheritedFlags::autosizeStatus additional bits.
>     };
> 
>@@ -63,10 +67,9 @@ public:
>     OptionSet<Fields> fields() const { return m_fields; }
> 
>     bool contains(Fields) const;
>-    bool shouldSkipSubtree() const;
> 
>     static float idempotentTextSize(float specifiedSize, float pageScale);
>-    static AutosizeStatus updateStatus(RenderStyle&);
>+    static void updateStatus(RenderStyle&);
> 
> private:
>     OptionSet<Fields> m_fields;
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index bbeccf128507a65b937fa247c00036892ccf2884..7ee238982af4fb5efdd8b3b1187a30a62397b5c2 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-06-24  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [Text autosizing] [iPadOS] Revise our heuristics to determine idempotent text autosizing candidates
>+        https://bugs.webkit.org/show_bug.cgi?id=198763
>+        <rdar://problem/51826266>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Rebaseline an existing text autosizing test, and introduce some new test cases that correspond to several common
>+        patterns of autosizable (or non-autosizable) text on websites that were surveyed.
>+
>+        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip-expected.txt:
>+        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip.html:
>+
> 2019-06-24  Alexey Shvayka  <shvaikalesh@gmail.com>
> 
>         Add Array.prototype.{flat,flatMap} to unscopables
>diff --git a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip-expected.txt b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip-expected.txt
>index 339d190531fc8a37b8587023f8594e3c031d4757..1e1a95ee6a07b76708854ead961ce12d874a36d2 100644
>--- a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip-expected.txt
>+++ b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip-expected.txt
>@@ -1,10 +1,34 @@
>+Checking target1:
> PASS result is >= 13
>+Checking target2:
> PASS result is >= 13
>+Checking target3:
>+PASS result is >= 13
>+Checking target4:
>+PASS result is >= 13
>+Checking target5:
> PASS result is 12
>+Checking target6:
> PASS result is 12
>+Checking target7:
>+PASS result is >= result2
>+Checking target8:
>+PASS result is >= 13
>+Checking target9:
>+PASS result is >= 13
>+Checking target10:
>+PASS result is >= 13
>+Checking target11:
>+PASS result is >= 13
>+Checking target12:
> PASS result is 12
>+Checking target13:
>+PASS result is >= 13
>+Checking target14:
> PASS result is 12
>-PASS result is >= result2
>+Checking target15:
>+PASS result is >= 13
>+Checking target16:
> PASS result is >= 13
> PASS successfullyParsed is true
> 
>@@ -16,3 +40,11 @@ Test
> Test
> TestTestTestTest
> Test
>+Test
>+Test
>+Test
>+Test
>+Test
>+Test
>+Test
>+Test
>diff --git a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip.html b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip.html
>index 90cfa6d727a4b10a14e04bd20c83c8281425d0ea..e9be421c2ae8c1f903f63112ffe57b718b5b32cc 100644
>--- a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip.html
>+++ b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip.html
>@@ -11,32 +11,42 @@ if (window.internals) {
> <script src="../../../../resources/js-test-pre.js"></script>
> </head>
> <body>
>-<div style="background: green;"><span id="target" style="font-size: 12px;">Test</span></div>
>+<div style="background: green;"><span id="target1" style="font-size: 12px;">Test</span></div>
> <div style="background: green; overflow: auto;"><span id="target2" style="float: right; font-size: 12px;">Test</span></div>
>-<div style="background: green;"><span id="target3" style="display: inline-block; font-size: 12px;">Test</span></div>
>-<div style="background: green;"><span style="display: inline-block; font-size: 12px;"><span id="target4">Test</span></span></div>
>+<div style="background: green;"><span id="target3" style="display: inline-block; font-size: 12px; width: 20px;">Test</span></div>
>+<div style="background: green;"><span style="display: inline-block; height: 12px; font-size: 12px;"><span id="target4">Test</span></span></div>
> <div style="background: green;"><span id="target5" style="position: absolute; left: 0px; top: 0px; font-size: 12px;">Test</span></div>
> <div style="background: green;"><span id="target6" style="display: none; font-size: 12px;">Test</span></div>
> <div style="background: green;"><span id="comparison" style="font-size: 12px;">Test<span>Test<span>Test<span id="target7">Test</span></span></span></span></div>
> <div style="background: green;"><span id="target8" style="font-size: 12px; -webkit-text-size-adjust: 100%">Test</span></div>
>+<div style="background: green;"><span id="target9" style="font-size: 12px; display: inline-block;">Test</span></div>
>+<div style="background: green;"><span id="target10" style="font-size: 12px; width: 20px; height: 12px;">Test</span></div>
>+<div style="background: green;"><span id="target11" style="font-size: 12px; height: 20px; position: fixed; white-space: nowrap;">Test</span></div>
>+<div style="background: green;"><span id="target12" style="font-size: 12px; height: 20px; position: fixed; float: right;">Test</span></div>
>+<div style="background: green;"><span id="target13" style="font-size: 12px; height: 20px; position: fixed; float: right; overflow-x: hidden; width: 100px;">Test</span></div>
>+<div style="background: green;"><span id="target14" style="font-size: 12px; height: 20px; width: 100px; float: right;">Test</span></div>
>+<div style="background: green;"><span id="target15" style="overflow-y: hidden; float: right;">Test</span></div>
>+<div style="background: green;"><span id="target16" style="float: right;">Test</span></div>
> <script>
> let result;
> function check(name, shouldGetAutosized) {
>     let target = document.getElementById(name);
>     target.offsetWidth;
>     result = Number.parseInt(window.getComputedStyle(target).getPropertyValue("font-size"));
>+    debug(`Checking ${name}:`);
>     if (shouldGetAutosized)
>         shouldBeGreaterThanOrEqual("result", "13");
>     else
>         shouldBe("result", "12");
> }
>-check("target", true);
>+check("target1", true);
> check("target2", true);
>-check("target3", false);
>-check("target4", false);
>+check("target3", true);
>+check("target4", true);
> check("target5", false);
> check("target6", false);
> 
>+debug(`Checking target7:`);
> let target = document.getElementById("target7");
> target.offsetWidth;
> let comparison = document.getElementById("comparison");
>@@ -46,6 +56,17 @@ let result2 = Number.parseInt(window.getComputedStyle(comparison).getPropertyVal
> shouldBeGreaterThanOrEqual("result", "result2");
> 
> check("target8", true);
>+check("target9", true);
>+check("target10", true);
>+
>+// Below are some common scenarios where we prefer (or prefer to not) adjust text size. These examples are inspired by
>+// common patterns in real webpages.
>+check("target11", true);
>+check("target12", false);
>+check("target13", true);
>+check("target14", false);
>+check("target15", true);
>+check("target16", true);
> </script>
> <script src="../../../../resources/js-test-post.js"></script>
> </body>

Actions: View | Formatted Diff | Diff

Attachments on bug 198763: 371964 | 372807 | 372811 | 372817