Attachment 371849 Details for Bug 198736 – Patch

[patch] Patch

bug-198736-20190611094813.patch (text/plain), 17.18 KB, created by Wenson Hsieh on 2019-06-11 09:48:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-06-11 09:48:13 PDT

Size: 17.18 KB

patch

obsolete

>Subversion Revision: 246278
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9fe43aea33c596a282c80ec202e15199c425a5e4..9f4997b9baeb4b0d5bbbf7c6e8b5e4ddc108d258 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2019-06-11  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Idempotent text autosizing needs to react properly to viewport changes
>+        https://bugs.webkit.org/show_bug.cgi?id=198736
>+        <rdar://problem/50591911>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Minor refactoring and some adjustments around StyleResolver::adjustRenderStyleForTextAutosizing. See below for
>+        more details, as well as the WebKit ChangeLog.
>+
>+        Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale.html
>+
>+        * css/StyleResolver.cpp:
>+        (WebCore::StyleResolver::adjustRenderStyleForTextAutosizing):
>+
>+        Rewrite this using early return statements, to make it easier to debug why elements fall out of text autosizing.
>+        Additionally, this function currently bails if the initial scale is exactly 1, whereas we can really avoid text
>+        autosizing in the case where the initial scale is at least 1; handle this by making idempotentTextSize return
>+        immediately with the specified size, in the case where the scale is at least 1.
>+
>+        Lastly, remove the null check for element by making this method take an Element&, and only call this from
>+        adjustRenderStyle if the element is nonnull (which matches adjustRenderStyleForSiteSpecificQuirks).
>+
>+        (WebCore::StyleResolver::adjustRenderStyle):
>+        * css/StyleResolver.h:
>+        * rendering/style/TextSizeAdjustment.cpp:
>+        (WebCore::AutosizeStatus::idempotentTextSize):
>+
> 2019-06-10  Antti Koivisto  <antti@apple.com>
> 
>         Event region should be set on scrolledContentsLayer if it exists
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 98c6ebab6b5446f5eafc3aee3924902c180986d7..04ba407b042c94dd7806e3d98f87e3ba09c8e40e 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-06-11  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Idempotent text autosizing needs to react properly to viewport changes
>+        https://bugs.webkit.org/show_bug.cgi?id=198736
>+        <rdar://problem/50591911>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        If idempotent text autosizing is enabled, respond to viewport initial scale changes by forcing a style recalc,
>+        since the amount by which idempotent text autosizing boosts font sizes depends on the Page's initial scale.
>+
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::resetIdempotentTextAutosizingIfNeeded):
>+        (WebKit::WebPage::viewportConfigurationChanged):
>+
> 2019-06-10  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS] fast/xsl tests are flaky when run after certain viewport shrink-to-fit tests
>diff --git a/Source/WebCore/css/StyleResolver.cpp b/Source/WebCore/css/StyleResolver.cpp
>index fb8f87d1faa6619ee484f072983bf82a0e19ce87..655c6f5bcf9cb87230b33b1a20f042b3cd90a18e 100644
>--- a/Source/WebCore/css/StyleResolver.cpp
>+++ b/Source/WebCore/css/StyleResolver.cpp
>@@ -884,16 +884,26 @@ static bool hasTextChildren(const Element& element)
>     return false;
> }
> 
>-void StyleResolver::adjustRenderStyleForTextAutosizing(RenderStyle& style, const Element* element)
>+void StyleResolver::adjustRenderStyleForTextAutosizing(RenderStyle& style, const Element& element)
> {
>     auto newAutosizeStatus = AutosizeStatus::updateStatus(style);
>-    auto pageScale = document().page() ? document().page()->initialScale() : 1.0f;
>-    if (settings().textAutosizingEnabled() && settings().textAutosizingUsesIdempotentMode() && element && !newAutosizeStatus.shouldSkipSubtree() && !style.textSizeAdjust().isNone() && hasTextChildren(*element) && pageScale != 1.0f) {
>-        auto fontDescription = style.fontDescription();
>-        fontDescription.setComputedSize(AutosizeStatus::idempotentTextSize(fontDescription.specifiedSize(), pageScale));
>-        style.setFontDescription(WTFMove(fontDescription));
>-        style.fontCascade().update(&document().fontSelector());
>-    }
>+    if (!settings().textAutosizingEnabled() || !settings().textAutosizingUsesIdempotentMode())
>+        return;
>+
>+    if (!hasTextChildren(element))
>+        return;
>+
>+    if (style.textSizeAdjust().isNone())
>+        return;
>+
>+    if (newAutosizeStatus.shouldSkipSubtree())
>+        return;
>+
>+    float initialScale = document().page() ? document().page()->initialScale() : 1;
>+    auto fontDescription = style.fontDescription();
>+    fontDescription.setComputedSize(AutosizeStatus::idempotentTextSize(fontDescription.specifiedSize(), initialScale));
>+    style.setFontDescription(WTFMove(fontDescription));
>+    style.fontCascade().update(&document().fontSelector());
> }
> #endif
> 
>@@ -1147,12 +1157,12 @@ void StyleResolver::adjustRenderStyle(RenderStyle& style, const RenderStyle& par
>     style.setEffectiveTouchActions(computeEffectiveTouchActions(style, parentStyle.effectiveTouchActions()));
> #endif
> 
>+    if (element) {
> #if ENABLE(TEXT_AUTOSIZING)
>-    adjustRenderStyleForTextAutosizing(style, element);
>+        adjustRenderStyleForTextAutosizing(style, *element);
> #endif
>-
>-    if (element)
>         adjustRenderStyleForSiteSpecificQuirks(style, *element);
>+    }
> }
> 
> void StyleResolver::adjustRenderStyleForSiteSpecificQuirks(RenderStyle& style, const Element& element)
>diff --git a/Source/WebCore/css/StyleResolver.h b/Source/WebCore/css/StyleResolver.h
>index ee1207c60dfee7f1b78a1e0a0a539b5630238fc4..2a34b26d71b245f613b261f351f2740cc584945e 100644
>--- a/Source/WebCore/css/StyleResolver.h
>+++ b/Source/WebCore/css/StyleResolver.h
>@@ -500,7 +500,7 @@ private:
>     // the last reference to a style declaration are garbage collected.
>     void sweepMatchedPropertiesCache();
> 
>-    void adjustRenderStyleForTextAutosizing(RenderStyle&, const Element*);
>+    void adjustRenderStyleForTextAutosizing(RenderStyle&, const Element&);
> 
>     typedef HashMap<unsigned, MatchedPropertiesCacheItem> MatchedPropertiesCache;
>     MatchedPropertiesCache m_matchedPropertiesCache;
>diff --git a/Source/WebCore/rendering/style/TextSizeAdjustment.cpp b/Source/WebCore/rendering/style/TextSizeAdjustment.cpp
>index 0dbdaa068b6ece13fb38f91e1ba7457ec25b20a1..020027800f62a1c8b4305946a7f79d31b7c63a81 100644
>--- a/Source/WebCore/rendering/style/TextSizeAdjustment.cpp
>+++ b/Source/WebCore/rendering/style/TextSizeAdjustment.cpp
>@@ -70,6 +70,9 @@ bool AutosizeStatus::shouldSkipSubtree() const
> 
> float AutosizeStatus::idempotentTextSize(float specifiedSize, float pageScale)
> {
>+    if (pageScale >= 1)
>+        return specifiedSize;
>+
>     // This describes a piecewise curve when the page scale is 2/3.
>     FloatPoint points[] = { {0.0f, 0.0f}, {6.0f, 9.0f}, {14.0f, 17.0f} };
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index 18a35ad27c9664d7a7a4ba5cdfcd85ced32be1f7..044f6130a7910f8744a942f0fd5f38ab9ce3f6a0 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -1248,6 +1248,7 @@ private:
>     void completeSyntheticClick(WebCore::Node& nodeRespondingToClick, const WebCore::FloatPoint& location, OptionSet<WebKit::WebEvent::Modifier>, WebCore::SyntheticClickType, WebCore::PointerID = WebCore::mousePointerID);
>     void sendTapHighlightForNodeIfNecessary(uint64_t requestID, WebCore::Node*);
>     void resetTextAutosizing();
>+    void resetIdempotentTextAutosizingIfNeeded(double previousInitialScale);
>     WebCore::VisiblePosition visiblePositionInFocusedNodeForPoint(const WebCore::Frame&, const WebCore::IntPoint&, bool isInteractingWithFocusedElement);
>     RefPtr<WebCore::Range> rangeForGranularityAtPoint(WebCore::Frame&, const WebCore::IntPoint&, uint32_t granularity, bool isInteractingWithFocusedElement);
>     void dispatchSyntheticMouseEventsForSelectionGesture(SelectionTouch, const WebCore::IntPoint&);
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index 8ae3025b64921c05557415c917057a7b8cf98e1a..16d04a6ded5ccb197c1bae3c3baab64dc234ab83 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -3303,10 +3303,27 @@ bool WebPage::shouldIgnoreMetaViewport() const
>     return m_page->settings().shouldIgnoreMetaViewport();
> }
> 
>+void WebPage::resetIdempotentTextAutosizingIfNeeded(double previousInitialScale)
>+{
>+    if (!m_page->settings().textAutosizingUsesIdempotentMode())
>+        return;
>+
>+    const float minimumScaleChangeBeforeRecomputingTextAutosizing = 0.01;
>+    if (std::abs(previousInitialScale - m_page->initialScale()) < minimumScaleChangeBeforeRecomputingTextAutosizing)
>+        return;
>+
>+    if (m_page->initialScale() >= 1 && previousInitialScale >= 1)
>+        return;
>+
>+    m_page->setNeedsRecalcStyleInAllFrames();
>+}
>+
> void WebPage::viewportConfigurationChanged(ZoomToInitialScale zoomToInitialScale)
> {
>+    double previousInitialScale = m_page->initialScale();
>     double initialScale = m_viewportConfiguration.initialScale();
>     m_page->setInitialScale(initialScale);
>+    resetIdempotentTextAutosizingIfNeeded(previousInitialScale);
> 
>     if (setFixedLayoutSize(m_viewportConfiguration.layoutSize()))
>         resetTextAutosizing();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 1b9d124d825008c3f65eacf144f9936ac4142b57..d696a73b36c6b344cf1a6e888daacad3043f6385 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-06-11  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Idempotent text autosizing needs to react properly to viewport changes
>+        https://bugs.webkit.org/show_bug.cgi?id=198736
>+        <rdar://problem/50591911>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a new layout test that programmatically adjusts the meta viewport initial scale, and dumps the resulting
>+        computed sizes of several paragraphs of text, after adjusting for text autosizing.
>+
>+        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale-expected.txt: Added.
>+        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale.html: Added.
>+
> 2019-06-10  Antti Koivisto  <antti@apple.com>
> 
>         Event region should be set on scrolledContentsLayer if it exists
>diff --git a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale-expected.txt b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..c0d431f79cfaa6370f72d4960ff97f93e84134b9
>--- /dev/null
>+++ b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale-expected.txt
>@@ -0,0 +1,41 @@
>+At initial scale 1
>+none	auto
>+8px	8px
>+12px	12px
>+14px	14px
>+16px	16px
>+20px	20px
>+
>+After changing initial scale to 0.5
>+none	auto
>+8px	13px
>+12px	17px
>+14px	19px
>+16px	19px
>+20px	20px
>+
>+After changing initial scale to 0.75
>+none	auto
>+8px	10px
>+12px	14px
>+14px	16px
>+16px	16px
>+20px	20px
>+
>+After changing initial scale back to 1
>+none	auto
>+8px	8px
>+12px	12px
>+14px	14px
>+16px	16px
>+20px	20px
>+
>+After changing initial scale to 1.25
>+none	auto
>+8px	8px
>+12px	12px
>+14px	14px
>+16px	16px
>+20px	20px
>+
>+
>diff --git a/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale.html b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..6e40502831d098837a8d773b7547e29a74ee5b95
>--- /dev/null
>+++ b/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale.html
>@@ -0,0 +1,158 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+<meta name="viewport" content="width=device-width, initial-scale=1">
>+<style>
>+body, html {
>+    margin: 0;
>+}
>+
>+.no-autosizing {
>+    -webkit-text-size-adjust: none;
>+}
>+
>+pre, code {
>+    color: white;
>+    background-color: black;
>+    padding: 4px;
>+}
>+
>+.container {
>+    border: 1px solid black;
>+    padding: 8px 12px 12px 12px;
>+    max-width: 320px;
>+}
>+
>+#text0, #text5 {
>+    font-size: 8px;
>+}
>+
>+#text1, #text6 {
>+    font-size: 12px;
>+}
>+
>+#text2, #text7 {
>+    font-size: 14px;
>+}
>+
>+#text3, #text8 {
>+    font-size: 16px;
>+}
>+
>+#text4, #text9 {
>+    font-size: 20px;
>+}
>+
>+table, td {
>+    border: 1px solid gray;
>+}
>+
>+thead, tfoot {
>+    background-color: tomato;
>+    color: white;
>+}
>+
>+th, td {
>+    padding: 4px;
>+}
>+</style>
>+<script src="../../../../resources/ui-helper.js"></script>
>+<script type="text/javascript">
>+if (window.internals && window.testRunner) {
>+    internals.settings.setTextAutosizingEnabled(true);
>+    internals.settings.setTextAutosizingUsesIdempotentMode(true);
>+    testRunner.dumpAsText();
>+    testRunner.waitUntilDone();
>+}
>+
>+async function dumpTextSizes(description) {
>+    if (window.testRunner)
>+        await UIHelper.ensurePresentationUpdate();
>+
>+    const table = document.querySelector("template#table").content.cloneNode("true").firstElementChild;
>+    table.querySelector(".description").textContent = description;
>+    for (let index = 0; index < 10; ++index) {
>+        const computedFontSize = getComputedStyle(document.querySelector(`#text${index}`)).fontSize;
>+        table.querySelector(`.text${index}`).textContent = computedFontSize;
>+    }
>+
>+    document.body.appendChild(table);
>+    document.body.appendChild(document.createElement("br"));
>+}
>+
>+addEventListener("load", async () => {
>+    const metaViewport = document.querySelector("meta");
>+    await dumpTextSizes("At initial scale 1");
>+
>+    metaViewport.content = "width=device-width, initial-scale=0.5";
>+    await dumpTextSizes("After changing initial scale to 0.5");
>+
>+    metaViewport.content = "width=device-width, initial-scale=0.75";
>+    await dumpTextSizes("After changing initial scale to 0.75");
>+
>+    metaViewport.content = "width=device-width, initial-scale=1.0";
>+    await dumpTextSizes("After changing initial scale back to 1");
>+
>+    metaViewport.content = "width=device-width, initial-scale=1.25";
>+    await dumpTextSizes("After changing initial scale to 1.25");
>+
>+    document.querySelector(".text-containers").remove();
>+    if (window.testRunner)
>+        testRunner.notifyDone();
>+});
>+</script>
>+</head>
>+<body>
>+    <template id="table">
>+        <table>
>+            <thead>
>+                <tr>
>+                    <th colspan="2" class="description"></th>
>+                </tr>
>+                <tr>
>+                    <th>none</th>
>+                    <th>auto</th>
>+                </tr>
>+            </thead>
>+            <tbody>
>+                <tr>
>+                    <td class="text0"></td>
>+                    <td class="text5"></td>
>+                </tr>
>+                <tr>
>+                    <td class="text1"></td>
>+                    <td class="text6"></td>
>+                </tr>
>+                <tr>
>+                    <td class="text2"></td>
>+                    <td class="text7"></td>
>+                </tr>
>+                <tr>
>+                    <td class="text3"></td>
>+                    <td class="text8"></td>
>+                </tr>
>+                <tr>
>+                    <td class="text4"></td>
>+                    <td class="text9"></td>
>+                </tr>
>+            </tbody>
>+        </table>
>+    </template>
>+    <div class="text-containers">
>+        <div class="container no-autosizing">
>+            <p id="text0">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean laoreet nisi felis, eu scelerisque dolor imperdiet in.</p>
>+            <p id="text1">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean laoreet nisi felis, eu scelerisque dolor imperdiet in.</p>
>+            <p id="text2">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean laoreet nisi felis, eu scelerisque dolor imperdiet in.</p>
>+            <p id="text3">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean laoreet nisi felis, eu scelerisque dolor imperdiet in.</p>
>+            <p id="text4">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean laoreet nisi felis, eu scelerisque dolor imperdiet in.</p>
>+        </div>
>+        <div class="container">
>+            <p id="text5">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean laoreet nisi felis, eu scelerisque dolor imperdiet in.</p>
>+            <p id="text6">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean laoreet nisi felis, eu scelerisque dolor imperdiet in.</p>
>+            <p id="text7">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean laoreet nisi felis, eu scelerisque dolor imperdiet in.</p>
>+            <p id="text8">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean laoreet nisi felis, eu scelerisque dolor imperdiet in.</p>
>+            <p id="text9">Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean laoreet nisi felis, eu scelerisque dolor imperdiet in.</p>
>+        </div>
>+    </div>
>+</body>
>+</html>
>\ No newline at end of file

Actions: View | Formatted Diff | Diff

Attachments on bug 198736: 371849