Changeset 246348 in webkit

Timestamp:

Jun 11, 2019 9:51:10 PM (5 years ago)

Author:

Wenson Hsieh

Message:

[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 Zalan Bujtas.

Source/WebCore:

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):

Source/WebKit:

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):

LayoutTests:

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale-expected.txt (added)
• LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-after-changing-initial-scale.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/StyleResolver.cpp (modified) (2 diffs)
• Source/WebCore/css/StyleResolver.h (modified) (1 diff)
• Source/WebCore/rendering/style/TextSizeAdjustment.cpp (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/WebPage.h (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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 Zalan Bujtas.
+
+        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-11  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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 Zalan Bujtas.
+
+        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-11  Timothy Hatcher  <timothy@apple.com>
 

trunk/Source/WebCore/css/StyleResolver.cpp

@@ -878 +878 @@
 }
 
-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
@@ -1141 +1151 @@
 #endif
 
+    if (element) {
 #if ENABLE(TEXT_AUTOSIZING)
-    adjustRenderStyleForTextAutosizing(style, element);
+        adjustRenderStyleForTextAutosizing(style, *element);
 #endif
-
-    if (element)
         adjustRenderStyleForSiteSpecificQuirks(style, *element);
+    }
 }
 

trunk/Source/WebCore/css/StyleResolver.h

@@ -501 +501 @@
     void sweepMatchedPropertiesCache();
 
-    void adjustRenderStyleForTextAutosizing(RenderStyle&, const Element*);
+    void adjustRenderStyleForTextAutosizing(RenderStyle&, const Element&);
 
     typedef HashMap<unsigned, MatchedPropertiesCacheItem> MatchedPropertiesCache;

trunk/Source/WebCore/rendering/style/TextSizeAdjustment.cpp

@@ -71 +71 @@
 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} };

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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 Zalan Bujtas.
+
+        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-11  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

@@ -1245 +1245 @@
     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);

trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

@@ -3334 +3334 @@
 }
 
+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()))