- Revision
- 260147
- Author
- wenson_hs...@apple.com
- Date
- 2020-04-15 13:22:45 -0700 (Wed, 15 Apr 2020)
Log Message
[iPadOS] Some pages indefinitely zoom in and out due to idempotent text autosizing
https://bugs.webkit.org/show_bug.cgi?id=210551
<rdar://problem/56820674>
Reviewed by Tim Horton.
Source/WebCore:
Rename m_initialScale and initialScale() on Page to m_initialScaleIgnoringContentSize and
initialScaleIgnoringContentSize(), respectively. See WebKit/ChangeLog for more details.
Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state.html
* page/Page.cpp:
(WebCore::Page::setInitialScaleIgnoringContentSize):
(WebCore::Page::setInitialScale): Deleted.
* page/Page.h:
(WebCore::Page::initialScaleIgnoringContentSize const):
(WebCore::Page::initialScale const): Deleted.
* style/StyleAdjuster.cpp:
(WebCore::Style::Adjuster::adjustmentForTextAutosizing):
Source/WebKit:
On a non-responsive web page with one or lines of non-wrapping text of a specific length (just under 1920px),
it's possible for the combination of idempotent text autosizing heuristics and viewport shrink-to-fit to cause
the single line of text to grow and shrink in size indefinitely, and additionally cause the initial scale to
thrash between multiple values indefinitely. This manifests in the entire page repeatedly zooming in and out
immediately after page load.
Consider the following scenario:
(1) A viewport configuration change (e.g. due to parsing the viewport meta tag) schedules the timer to reset
idempotent text autosizing. Let's suppose the page has a really long line of non-wrapping 12px text that is
below 1920px wide.
(2) The timer fires, invalidating styles and recomputing text autosizing given the current initial scale. The
current initial scale is below 1, since ViewportConfiguration will attempt to shrink to fit the page to
avoid horizontal scrolling. This causes text autosizing to boost the long line of text to a larger value
(let's say 17px).
(3) The next time we perform style recomputation and layout, we discover that the content width of the page is
now larger than 1920px, which is the maximum width which we'll attempt to shrink to fit; when computing
initial scale, we give up trying to shrink down to avoid making the inital scale too small, and instead just
keep it at 1.
(4) This change in viewport configuration then schedules another idempotent text autosizing reset. When this
timer fires, it sees that the initial scale is now 1, which means that the text is no longer boosted, so we
make the single line of text small again (12px).
(5) After the next style recomputation and layout, this causes the content width of the page to dip below the
1920px threshold, causing the initial scale to dip below 1 again. As detailed above, this schedules another
idempotent text autosizing update, which now boosts font size once again, and the cycle continues.
To fix this, instead of consulting the initial scale (`ViewportConfiguration::initialScale()`) when computing
the boosted font size for idempotent text autosizing, we can instead ask for the initial scale ignoring content
size (`ViewportConfiguration::initialScaleIgnoringContentSize()`). This prevents changes in content size due to
idempotent autosizing from affecting the idempotent autosizing heuristic (through the different initial scale),
and ensures that this method of text autosizing actually remains idempotent.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::resetIdempotentTextAutosizingIfNeeded):
(WebKit::WebPage::viewportConfigurationChanged):
LayoutTests:
Add a layout test to verify that on a page with a single line of text, if idempotent text autosizing is enabled,
the text size is boosted by idempotent autosizing, but we don't end up getting into a state where the computed
font size flickers between multiple values.
* fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state-expected.txt: Added.
* fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (260146 => 260147)
--- trunk/LayoutTests/ChangeLog 2020-04-15 20:20:21 UTC (rev 260146)
+++ trunk/LayoutTests/ChangeLog 2020-04-15 20:22:45 UTC (rev 260147)
@@ -1,3 +1,18 @@
+2020-04-15 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [iPadOS] Some pages indefinitely zoom in and out due to idempotent text autosizing
+ https://bugs.webkit.org/show_bug.cgi?id=210551
+ <rdar://problem/56820674>
+
+ Reviewed by Tim Horton.
+
+ Add a layout test to verify that on a page with a single line of text, if idempotent text autosizing is enabled,
+ the text size is boosted by idempotent autosizing, but we don't end up getting into a state where the computed
+ font size flickers between multiple values.
+
+ * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state-expected.txt: Added.
+ * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state.html: Added.
+
2020-04-15 Diego Pino Garcia <dp...@igalia.com>
[GTK] Gardening of flaky failures
Added: trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state-expected.txt (0 => 260147)
--- trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state-expected.txt 2020-04-15 20:22:45 UTC (rev 260147)
@@ -0,0 +1,17 @@
+Verifies that idempotent text autosizing does not cause computed font size to thrash indefinitely. To run the test manually, open this page on an iPad.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS computedSizeAfterLoad is >= 13
+PASS computedSizeAfterLoad is computedSizeAfterWaitingFor100Milliseconds
+PASS computedSizeAfterLoad is computedSizeAfterWaitingFor200Milliseconds
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+The quick brown fox jumped over the lazy dog. The quick brown fox jumped over the lazy dog. The quick brown fox jumped over the lazy dog. The quick brown fox jumped over the lazy dog. The quick brown fox jumped over the lazy dog.
+
+
+
+
Added: trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state.html (0 => 260147)
--- trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state.html (rev 0)
+++ trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state.html 2020-04-15 20:22:45 UTC (rev 260147)
@@ -0,0 +1,43 @@
+<!DOCTYPE html><!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<script src=""
+<script src=""
+<script>
+if (window.internals) {
+ internals.settings.setTextAutosizingEnabled(true);
+ internals.settings.setTextAutosizingUsesIdempotentMode(true);
+ jsTestIsAsync = true;
+}
+</script>
+<style>
+body {
+ font-size: 12px;
+ font-family: monospace;
+ white-space: pre;
+ margin-bottom: 200vh;
+}
+</style>
+</head>
+<body>
+<span>The quick brown fox jumped over the lazy dog. The quick brown fox jumped over the lazy dog. The quick brown fox jumped over the lazy dog. The quick brown fox jumped over the lazy dog. The quick brown fox jumped over the lazy dog.</span>
+<script>
+addEventListener("load", async () => {
+ description("Verifies that idempotent text autosizing does not cause computed font size to thrash indefinitely. To run the test manually, open this page on an iPad.");
+
+ computedSizeAfterLoad = getComputedStyle(document.querySelector("span")).fontSize;
+
+ await UIHelper.delayFor(100);
+ computedSizeAfterWaitingFor100Milliseconds = getComputedStyle(document.querySelector("span")).fontSize;
+
+ await UIHelper.delayFor(100);
+ computedSizeAfterWaitingFor200Milliseconds = getComputedStyle(document.querySelector("span")).fontSize;
+
+ shouldBeGreaterThanOrEqual("computedSizeAfterLoad", "13");
+ shouldBe("computedSizeAfterLoad", "computedSizeAfterWaitingFor100Milliseconds");
+ shouldBe("computedSizeAfterLoad", "computedSizeAfterWaitingFor200Milliseconds");
+ finishJSTest();
+});
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (260146 => 260147)
--- trunk/Source/WebCore/ChangeLog 2020-04-15 20:20:21 UTC (rev 260146)
+++ trunk/Source/WebCore/ChangeLog 2020-04-15 20:22:45 UTC (rev 260147)
@@ -1,3 +1,25 @@
+2020-04-15 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [iPadOS] Some pages indefinitely zoom in and out due to idempotent text autosizing
+ https://bugs.webkit.org/show_bug.cgi?id=210551
+ <rdar://problem/56820674>
+
+ Reviewed by Tim Horton.
+
+ Rename m_initialScale and initialScale() on Page to m_initialScaleIgnoringContentSize and
+ initialScaleIgnoringContentSize(), respectively. See WebKit/ChangeLog for more details.
+
+ Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-reaches-stable-state.html
+
+ * page/Page.cpp:
+ (WebCore::Page::setInitialScaleIgnoringContentSize):
+ (WebCore::Page::setInitialScale): Deleted.
+ * page/Page.h:
+ (WebCore::Page::initialScaleIgnoringContentSize const):
+ (WebCore::Page::initialScale const): Deleted.
+ * style/StyleAdjuster.cpp:
+ (WebCore::Style::Adjuster::adjustmentForTextAutosizing):
+
2020-04-15 Chris Dumez <cdu...@apple.com>
REGRESSION (r258977): Crash under Document::visibilityStateChanged
Modified: trunk/Source/WebCore/page/Page.cpp (260146 => 260147)
--- trunk/Source/WebCore/page/Page.cpp 2020-04-15 20:20:21 UTC (rev 260146)
+++ trunk/Source/WebCore/page/Page.cpp 2020-04-15 20:22:45 UTC (rev 260147)
@@ -1105,9 +1105,9 @@
pageOverlayController().didChangeDeviceScaleFactor();
}
-void Page::setInitialScale(float initialScale)
+void Page::setInitialScaleIgnoringContentSize(float scale)
{
- m_initialScale = initialScale;
+ m_initialScaleIgnoringContentSize = scale;
}
void Page::setUserInterfaceLayoutDirection(UserInterfaceLayoutDirection userInterfaceLayoutDirection)
Modified: trunk/Source/WebCore/page/Page.h (260146 => 260147)
--- trunk/Source/WebCore/page/Page.h 2020-04-15 20:20:21 UTC (rev 260146)
+++ trunk/Source/WebCore/page/Page.h 2020-04-15 20:22:45 UTC (rev 260147)
@@ -352,8 +352,8 @@
float deviceScaleFactor() const { return m_deviceScaleFactor; }
WEBCORE_EXPORT void setDeviceScaleFactor(float);
- float initialScale() const { return m_initialScale; }
- WEBCORE_EXPORT void setInitialScale(float);
+ float initialScaleIgnoringContentSize() const { return m_initialScaleIgnoringContentSize; }
+ WEBCORE_EXPORT void setInitialScaleIgnoringContentSize(float);
float topContentInset() const { return m_topContentInset; }
WEBCORE_EXPORT void setTopContentInset(float);
@@ -866,7 +866,7 @@
#if ENABLE(TEXT_AUTOSIZING)
float m_textAutosizingWidth { 0 };
#endif
- float m_initialScale { 1.0f };
+ float m_initialScaleIgnoringContentSize { 1.0f };
bool m_suppressScrollbarAnimations { false };
Modified: trunk/Source/WebCore/style/StyleAdjuster.cpp (260146 => 260147)
--- trunk/Source/WebCore/style/StyleAdjuster.cpp 2020-04-15 20:20:21 UTC (rev 260146)
+++ trunk/Source/WebCore/style/StyleAdjuster.cpp 2020-04-15 20:22:45 UTC (rev 260147)
@@ -577,7 +577,7 @@
if (style.textSizeAdjust().isNone())
return adjustmentForTextAutosizing;
- float initialScale = document.page() ? document.page()->initialScale() : 1;
+ float initialScale = document.page() ? document.page()->initialScaleIgnoringContentSize() : 1;
auto adjustLineHeightIfNeeded = [&](auto computedFontSize) {
auto lineHeight = style.specifiedLineHeight();
constexpr static unsigned eligibleFontSize = 12;
Modified: trunk/Source/WebKit/ChangeLog (260146 => 260147)
--- trunk/Source/WebKit/ChangeLog 2020-04-15 20:20:21 UTC (rev 260146)
+++ trunk/Source/WebKit/ChangeLog 2020-04-15 20:22:45 UTC (rev 260147)
@@ -1,3 +1,51 @@
+2020-04-15 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [iPadOS] Some pages indefinitely zoom in and out due to idempotent text autosizing
+ https://bugs.webkit.org/show_bug.cgi?id=210551
+ <rdar://problem/56820674>
+
+ Reviewed by Tim Horton.
+
+ On a non-responsive web page with one or lines of non-wrapping text of a specific length (just under 1920px),
+ it's possible for the combination of idempotent text autosizing heuristics and viewport shrink-to-fit to cause
+ the single line of text to grow and shrink in size indefinitely, and additionally cause the initial scale to
+ thrash between multiple values indefinitely. This manifests in the entire page repeatedly zooming in and out
+ immediately after page load.
+
+ Consider the following scenario:
+
+ (1) A viewport configuration change (e.g. due to parsing the viewport meta tag) schedules the timer to reset
+ idempotent text autosizing. Let's suppose the page has a really long line of non-wrapping 12px text that is
+ below 1920px wide.
+
+ (2) The timer fires, invalidating styles and recomputing text autosizing given the current initial scale. The
+ current initial scale is below 1, since ViewportConfiguration will attempt to shrink to fit the page to
+ avoid horizontal scrolling. This causes text autosizing to boost the long line of text to a larger value
+ (let's say 17px).
+
+ (3) The next time we perform style recomputation and layout, we discover that the content width of the page is
+ now larger than 1920px, which is the maximum width which we'll attempt to shrink to fit; when computing
+ initial scale, we give up trying to shrink down to avoid making the inital scale too small, and instead just
+ keep it at 1.
+
+ (4) This change in viewport configuration then schedules another idempotent text autosizing reset. When this
+ timer fires, it sees that the initial scale is now 1, which means that the text is no longer boosted, so we
+ make the single line of text small again (12px).
+
+ (5) After the next style recomputation and layout, this causes the content width of the page to dip below the
+ 1920px threshold, causing the initial scale to dip below 1 again. As detailed above, this schedules another
+ idempotent text autosizing update, which now boosts font size once again, and the cycle continues.
+
+ To fix this, instead of consulting the initial scale (`ViewportConfiguration::initialScale()`) when computing
+ the boosted font size for idempotent text autosizing, we can instead ask for the initial scale ignoring content
+ size (`ViewportConfiguration::initialScaleIgnoringContentSize()`). This prevents changes in content size due to
+ idempotent autosizing from affecting the idempotent autosizing heuristic (through the different initial scale),
+ and ensures that this method of text autosizing actually remains idempotent.
+
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::resetIdempotentTextAutosizingIfNeeded):
+ (WebKit::WebPage::viewportConfigurationChanged):
+
2020-04-14 Megan Gardner <megan_gard...@apple.com>
Data Detected Actions sheets are presented from odd locations.
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (260146 => 260147)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2020-04-15 20:20:21 UTC (rev 260146)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2020-04-15 20:22:45 UTC (rev 260147)
@@ -3527,10 +3527,10 @@
return;
const float minimumScaleChangeBeforeRecomputingTextAutosizing = 0.01;
- if (std::abs(previousInitialScale - m_page->initialScale()) < minimumScaleChangeBeforeRecomputingTextAutosizing)
+ if (std::abs(previousInitialScale - m_page->initialScaleIgnoringContentSize()) < minimumScaleChangeBeforeRecomputingTextAutosizing)
return;
- if (m_page->initialScale() >= 1 && previousInitialScale >= 1)
+ if (m_page->initialScaleIgnoringContentSize() >= 1 && previousInitialScale >= 1)
return;
if (!m_page->mainFrame().view())
@@ -3646,10 +3646,11 @@
void WebPage::viewportConfigurationChanged(ZoomToInitialScale zoomToInitialScale)
{
double initialScale = m_viewportConfiguration.initialScale();
+ double initialScaleIgnoringContentSize = m_viewportConfiguration.initialScaleIgnoringContentSize();
#if ENABLE(TEXT_AUTOSIZING)
- double previousInitialScale = m_page->initialScale();
- m_page->setInitialScale(initialScale);
- resetIdempotentTextAutosizingIfNeeded(previousInitialScale);
+ double previousInitialScaleIgnoringContentSize = m_page->initialScaleIgnoringContentSize();
+ m_page->setInitialScaleIgnoringContentSize(initialScaleIgnoringContentSize);
+ resetIdempotentTextAutosizingIfNeeded(previousInitialScaleIgnoringContentSize);
if (setFixedLayoutSize(m_viewportConfiguration.layoutSize()))
resetTextAutosizing();
