Title: [257373] trunk
Revision
257373
Author
mmaxfi...@apple.com
Date
2020-02-25 13:51:12 -0800 (Tue, 25 Feb 2020)

Log Message

[iPadOS] REGRESSION(r247667): Autosizing style changes don't invalidate RenderText's preferred logical widths
https://bugs.webkit.org/show_bug.cgi?id=208084
<rdar://problem/59463898>

Reviewed by Darin Adler.

Source/WebCore:

Just use RenderElement::setStyle(), which handles all the invalidations, instead of RenderElement::setNeedsLayout().

Do a little refactoring so we don't clone RenderStyles unless we need to.

Test: fast/text-autosizing/ios/idempotentmode/viewport-change-relayout.html

* page/Page.cpp:
(WebCore::Page::recomputeTextAutoSizingInAllFrames):
* style/StyleAdjuster.cpp:
(WebCore::Style::Adjuster::adjustmentForTextAutosizing):
(WebCore::Style::Adjuster::adjustForTextAutosizing):
* style/StyleAdjuster.h:
(WebCore::Style::Adjuster::AdjustmentForTextAutosizing::operator bool const):

LayoutTests:

* fast/text-autosizing/ios/idempotentmode/viewport-change-relayout-expected.html: Added.
* fast/text-autosizing/ios/idempotentmode/viewport-change-relayout.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (257372 => 257373)


--- trunk/LayoutTests/ChangeLog	2020-02-25 21:45:40 UTC (rev 257372)
+++ trunk/LayoutTests/ChangeLog	2020-02-25 21:51:12 UTC (rev 257373)
@@ -1,3 +1,14 @@
+2020-02-25  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [iPadOS] REGRESSION(r247667): Autosizing style changes don't invalidate RenderText's preferred logical widths
+        https://bugs.webkit.org/show_bug.cgi?id=208084
+        <rdar://problem/59463898>
+
+        Reviewed by Darin Adler.
+
+        * fast/text-autosizing/ios/idempotentmode/viewport-change-relayout-expected.html: Added.
+        * fast/text-autosizing/ios/idempotentmode/viewport-change-relayout.html: Added.
+
 2020-02-25  Zalan Bujtas  <za...@apple.com>
 
         [LFC][TFC] Do not create a formatting context for empty subtree in TableFormattingContext::computePreferredWidthForColumns

Added: trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/viewport-change-relayout-expected.html (0 => 257373)


--- trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/viewport-change-relayout-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/viewport-change-relayout-expected.html	2020-02-25 21:51:12 UTC (rev 257373)
@@ -0,0 +1,16 @@
+<!DOCTYPE html><!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta id="viewport" name="viewport" content="width=400, initial-scale=0.70">
+<script>
+if (window.internals) {
+    internals.settings.setTextAutosizingEnabled(true);
+    internals.settings.setTextAutosizingUsesIdempotentMode(true);
+}
+</script>
+</head>
+<body style="font: 17px 'Arial'; width: 631px; -webkit-text-size-adjust: none;">
+<div><em id="em">Japan</em> with Everything about modern and traditional emphasis</div>
+<div style="height: 4000px; background: green;"></div>
+</body>
+</html>

Added: trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/viewport-change-relayout.html (0 => 257373)


--- trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/viewport-change-relayout.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/viewport-change-relayout.html	2020-02-25 21:51:12 UTC (rev 257373)
@@ -0,0 +1,40 @@
+<!DOCTYPE html><!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta id="viewport" name="viewport" content="width=400, initial-scale=1">
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+if (window.internals) {
+    internals.settings.setTextAutosizingEnabled(true);
+    internals.settings.setTextAutosizingUsesIdempotentMode(true);
+}
+</script>
+</head>
+<body style="font: 14px 'Arial'; width: 631px;">
+<div><em id="em">Japan</em> with Everything about modern and traditional emphasis</div>
+<div style="height: 4000px; background: green;"></div>
+<script>
+function _waitForCondition(condition, completionHandler)
+{
+  if (condition())
+    completionHandler();
+  else
+    setTimeout(_waitForCondition, 5, condition, completionHandler);
+}
+
+const em = document.getElementById("em");
+em.getBoundingClientRect();
+window.setTimeout(function() {
+    document.getElementById("viewport").content = "width=400, initial-scale=0.70";
+    em.getBoundingClientRect();
+}, 0);
+_waitForCondition(function() {
+    return window.getComputedStyle(document.getElementById("em")).getPropertyValue("font-size") != "14px";
+}, function() {
+    if (window.testRunner)
+        testRunner.notifyDone();
+});
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (257372 => 257373)


--- trunk/Source/WebCore/ChangeLog	2020-02-25 21:45:40 UTC (rev 257372)
+++ trunk/Source/WebCore/ChangeLog	2020-02-25 21:51:12 UTC (rev 257373)
@@ -1,3 +1,25 @@
+2020-02-25  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [iPadOS] REGRESSION(r247667): Autosizing style changes don't invalidate RenderText's preferred logical widths
+        https://bugs.webkit.org/show_bug.cgi?id=208084
+        <rdar://problem/59463898>
+
+        Reviewed by Darin Adler.
+
+        Just use RenderElement::setStyle(), which handles all the invalidations, instead of RenderElement::setNeedsLayout().
+
+        Do a little refactoring so we don't clone RenderStyles unless we need to.
+
+        Test: fast/text-autosizing/ios/idempotentmode/viewport-change-relayout.html
+
+        * page/Page.cpp:
+        (WebCore::Page::recomputeTextAutoSizingInAllFrames):
+        * style/StyleAdjuster.cpp:
+        (WebCore::Style::Adjuster::adjustmentForTextAutosizing):
+        (WebCore::Style::Adjuster::adjustForTextAutosizing):
+        * style/StyleAdjuster.h:
+        (WebCore::Style::Adjuster::AdjustmentForTextAutosizing::operator bool const):
+
 2020-02-25  Zalan Bujtas  <za...@apple.com>
 
         [LFC][TFC] Do not create a formatting context for empty subtree in TableFormattingContext::computePreferredWidthForColumns

Modified: trunk/Source/WebCore/page/Page.cpp (257372 => 257373)


--- trunk/Source/WebCore/page/Page.cpp	2020-02-25 21:45:40 UTC (rev 257372)
+++ trunk/Source/WebCore/page/Page.cpp	2020-02-25 21:51:12 UTC (rev 257373)
@@ -2986,8 +2986,11 @@
         if (auto* renderView = document.renderView()) {
             for (auto& renderer : descendantsOfType<RenderElement>(*renderView)) {
                 if (auto* element = renderer.element()) {
-                    if (Style::Adjuster::adjustForTextAutosizing(renderer.mutableStyle(), *element))
-                        renderer.setNeedsLayout();
+                    if (auto adjustment = Style::Adjuster::adjustmentForTextAutosizing(renderer.style(), *element)) {
+                        auto newStyle = RenderStyle::clone(renderer.style());
+                        Style::Adjuster::adjustForTextAutosizing(newStyle, *element, adjustment);
+                        renderer.setStyle(WTFMove(newStyle));
+                    }
                 }
             }
         }

Modified: trunk/Source/WebCore/style/StyleAdjuster.cpp (257372 => 257373)


--- trunk/Source/WebCore/style/StyleAdjuster.cpp	2020-02-25 21:45:40 UTC (rev 257372)
+++ trunk/Source/WebCore/style/StyleAdjuster.cpp	2020-02-25 21:51:12 UTC (rev 257373)
@@ -568,15 +568,16 @@
     return false;
 }
 
-bool Adjuster::adjustForTextAutosizing(RenderStyle& style, const Element& element)
+auto Adjuster::adjustmentForTextAutosizing(const RenderStyle& style, const Element& element) -> AdjustmentForTextAutosizing
 {
+    AdjustmentForTextAutosizing adjustmentForTextAutosizing;
+
     auto& document = element.document();
     if (!document.settings().textAutosizingEnabled() || !document.settings().textAutosizingUsesIdempotentMode())
-        return false;
+        return adjustmentForTextAutosizing;
 
-    AutosizeStatus::updateStatus(style);
     if (style.textSizeAdjust().isNone())
-        return false;
+        return adjustmentForTextAutosizing;
 
     float initialScale = document.page() ? document.page()->initialScale() : 1;
     auto adjustLineHeightIfNeeded = [&](auto computedFontSize) {
@@ -593,7 +594,7 @@
         if (AutosizeStatus::probablyContainsASmallFixedNumberOfLines(style))
             return;
 
-        style.setLineHeight({ minimumLineHeight, Fixed });
+        adjustmentForTextAutosizing.newLineHeight = minimumLineHeight;
     };
 
     auto fontDescription = style.fontDescription();
@@ -601,18 +602,16 @@
     auto specifiedFontSize = fontDescription.specifiedSize();
     bool isCandidate = style.isIdempotentTextAutosizingCandidate();
     if (!isCandidate && WTF::areEssentiallyEqual(initialComputedFontSize, specifiedFontSize))
-        return false;
+        return adjustmentForTextAutosizing;
 
     auto adjustedFontSize = AutosizeStatus::idempotentTextSize(fontDescription.specifiedSize(), initialScale);
     if (isCandidate && WTF::areEssentiallyEqual(initialComputedFontSize, adjustedFontSize))
-        return false;
+        return adjustmentForTextAutosizing;
 
     if (!hasTextChild(element))
-        return false;
+        return adjustmentForTextAutosizing;
 
-    fontDescription.setComputedSize(isCandidate ? adjustedFontSize : specifiedFontSize);
-    style.setFontDescription(WTFMove(fontDescription));
-    style.fontCascade().update(&document.fontSelector());
+    adjustmentForTextAutosizing.newFontSize = isCandidate ? adjustedFontSize : specifiedFontSize;
 
     // FIXME: We should restore computed line height to its original value in the case where the element is not
     // an idempotent text autosizing candidate; otherwise, if an element that is a text autosizing candidate contains
@@ -620,8 +619,27 @@
     if (isCandidate)
         adjustLineHeightIfNeeded(adjustedFontSize);
 
-    return true;
+    return adjustmentForTextAutosizing;
 }
+
+bool Adjuster::adjustForTextAutosizing(RenderStyle& style, const Element& element, AdjustmentForTextAutosizing adjustment)
+{
+    AutosizeStatus::updateStatus(style);
+    if (auto newFontSize = adjustment.newFontSize) {
+        auto fontDescription = style.fontDescription();
+        fontDescription.setComputedSize(*newFontSize);
+        style.setFontDescription(WTFMove(fontDescription));
+        style.fontCascade().update(&element.document().fontSelector());
+    }
+    if (auto newLineHeight = adjustment.newLineHeight)
+        style.setLineHeight({ *newLineHeight, Fixed });
+    return adjustment.newFontSize || adjustment.newLineHeight;
+}
+
+bool Adjuster::adjustForTextAutosizing(RenderStyle& style, const Element& element)
+{
+    return adjustForTextAutosizing(style, element, adjustmentForTextAutosizing(style, element));
+}
 #endif
 
 }

Modified: trunk/Source/WebCore/style/StyleAdjuster.h (257372 => 257373)


--- trunk/Source/WebCore/style/StyleAdjuster.h	2020-02-25 21:45:40 UTC (rev 257372)
+++ trunk/Source/WebCore/style/StyleAdjuster.h	2020-02-25 21:51:12 UTC (rev 257373)
@@ -49,6 +49,13 @@
     static void adjustAnimatedStyle(RenderStyle&, const RenderStyle* parentBoxStyle, OptionSet<AnimationImpact>);
 
 #if ENABLE(TEXT_AUTOSIZING)
+    struct AdjustmentForTextAutosizing {
+        Optional<float> newFontSize;
+        Optional<float> newLineHeight;
+        explicit operator bool() const { return newFontSize || newLineHeight; }
+    };
+    static AdjustmentForTextAutosizing adjustmentForTextAutosizing(const RenderStyle&, const Element&);
+    static bool adjustForTextAutosizing(RenderStyle&, const Element&, AdjustmentForTextAutosizing);
     static bool adjustForTextAutosizing(RenderStyle&, const Element&);
 #endif
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to