Page temporarily jumps to an excessively small viewport scale while loading usatoday.com
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Sep 2019 22:30:55 +0000 (22:30 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Sep 2019 22:30:55 +0000 (22:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=202224
<rdar://problem/52906640>

Reviewed by Tim Horton.

Source/WebCore:

On some pages (e.g. usatoday.com), the content width of the page temporarily becomes very large during page
load. This causes a couple of viewport scaling behaviors (notably, the existing shrink-to-fit heuristic in
ViewportConfiguration::initialScaleFromSize, as well as the new iPad-specific content-aware shrink-to-fit
heuristic in WebPage::immediatelyShrinkToFitContent) to cause the page to shrink down excessively in an attempt
to fit all the content to the viewport. This causes a very ugly flash as the page appears zoomed out initially
during page load, before zooming back in.

To fix this, we add some sanity checks to these viewport scaling heuristics. In ViewportConfiguration's
initialScaleFromSize method, in the codepath where an initial scale is not specified, we always scale to fit the
contents of the page; instead, detect the case where the content width is enormous (with a threshold arbitrarily
chosen to be 1920) and fall back to the scaling to fit the viewport's width, if such a width has been explicitly
set. This ensures that we avoid excessive shrinking in the case where content is extremely wide, but also that
we do scale the viewport down to fit all the content in the case where the content isn't extremely wide (e.g. on
daringfireball.com).

See WebKit ChangeLog for more detail.

Test: fast/viewport/ios/shrink-to-fit-large-content-width.html

* page/ViewportConfiguration.cpp:
(WebCore::ViewportConfiguration::initialScaleFromSize const):

Source/WebKit:

Tweaks the content-aware shrink-to-fit algorithm to bail in the case where the content width is extremely large,
such that it bails instead of attempting to fit the entire content of the page. See WebCore ChangeLog for more
details.

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::immediatelyShrinkToFitContent):

LayoutTests:

Adds a new layout test to verify that when the content width of the page is excessively large and an explicit
viewport width is specified, we don't attempt to zoom out to fit the larger content width, and instead zoom to
fit the explicit viewport width.

* fast/viewport/ios/shrink-to-fit-large-content-width-expected.txt: Added.
* fast/viewport/ios/shrink-to-fit-large-content-width.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250361 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/viewport/ios/shrink-to-fit-large-content-width-expected.txt [new file with mode: 0644]
LayoutTests/fast/viewport/ios/shrink-to-fit-large-content-width.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/ViewportConfiguration.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index 09fdb79..5a34ce1 100644 (file)
@@ -1,3 +1,18 @@
+2019-09-25  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Page temporarily jumps to an excessively small viewport scale while loading usatoday.com
+        https://bugs.webkit.org/show_bug.cgi?id=202224
+        <rdar://problem/52906640>
+
+        Reviewed by Tim Horton.
+
+        Adds a new layout test to verify that when the content width of the page is excessively large and an explicit
+        viewport width is specified, we don't attempt to zoom out to fit the larger content width, and instead zoom to
+        fit the explicit viewport width.
+
+        * fast/viewport/ios/shrink-to-fit-large-content-width-expected.txt: Added.
+        * fast/viewport/ios/shrink-to-fit-large-content-width.html: Added.
+
 2019-09-25  Zalan Bujtas  <zalan@apple.com>
 
         Adjust tap position to avoid double-tap issue across tests.
diff --git a/LayoutTests/fast/viewport/ios/shrink-to-fit-large-content-width-expected.txt b/LayoutTests/fast/viewport/ios/shrink-to-fit-large-content-width-expected.txt
new file mode 100644 (file)
index 0000000..085740d
--- /dev/null
@@ -0,0 +1,10 @@
+This test verifies that a page with a constant width viewport and a very large content width, the page is scaled such that the constant viewport width spans the width of the viewport. To test manually, load the page and verify that only the top bar spans the full width of the page.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS innerWidth is 1000
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/viewport/ios/shrink-to-fit-large-content-width.html b/LayoutTests/fast/viewport/ios/shrink-to-fit-large-content-width.html
new file mode 100644 (file)
index 0000000..caba6ab
--- /dev/null
@@ -0,0 +1,49 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ shouldIgnoreMetaViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="width=1000">
+<style>
+body, html {
+    margin: 0;
+}
+
+#top-bar {
+    width: 1000px;
+    height: 100px;
+    background: linear-gradient(to right, red 0%, green 50%, blue 100%);
+}
+
+#bottom-bar {
+    width: 8000px;
+    height: 100px;
+    background: linear-gradient(to right, red 0%, green 50%, blue 100%);
+}
+
+#description {
+    width: 300px;
+    overflow: scroll;
+}
+</style>
+<script src="../../../resources/ui-helper.js"></script>
+<script src="../../../resources/js-test.js"></script>
+<script>
+jsTestIsAsync = true;
+
+description("This test verifies that a page with a constant width viewport and a very large content width, the page is scaled such that the constant viewport width spans the width of the viewport. To test manually, load the page and verify that only the top bar spans the full width of the page.");
+
+addEventListener("load", async () => {
+    if (!window.testRunner)
+        return;
+
+    await UIHelper.ensurePresentationUpdate();
+    shouldBe("innerWidth", "1000");
+    finishJSTest();
+});
+</script>
+</head>
+<body>
+<div id="top-bar"></div>
+<div id="bottom-bar"></div>
+<div id="description"></div>
+</body>
+</html>
index 50b326e..3c40640 100644 (file)
@@ -1,5 +1,35 @@
 2019-09-25  Wenson Hsieh  <wenson_hsieh@apple.com>
 
+        Page temporarily jumps to an excessively small viewport scale while loading usatoday.com
+        https://bugs.webkit.org/show_bug.cgi?id=202224
+        <rdar://problem/52906640>
+
+        Reviewed by Tim Horton.
+
+        On some pages (e.g. usatoday.com), the content width of the page temporarily becomes very large during page
+        load. This causes a couple of viewport scaling behaviors (notably, the existing shrink-to-fit heuristic in
+        ViewportConfiguration::initialScaleFromSize, as well as the new iPad-specific content-aware shrink-to-fit
+        heuristic in WebPage::immediatelyShrinkToFitContent) to cause the page to shrink down excessively in an attempt
+        to fit all the content to the viewport. This causes a very ugly flash as the page appears zoomed out initially
+        during page load, before zooming back in.
+
+        To fix this, we add some sanity checks to these viewport scaling heuristics. In ViewportConfiguration's
+        initialScaleFromSize method, in the codepath where an initial scale is not specified, we always scale to fit the
+        contents of the page; instead, detect the case where the content width is enormous (with a threshold arbitrarily
+        chosen to be 1920) and fall back to the scaling to fit the viewport's width, if such a width has been explicitly
+        set. This ensures that we avoid excessive shrinking in the case where content is extremely wide, but also that
+        we do scale the viewport down to fit all the content in the case where the content isn't extremely wide (e.g. on
+        daringfireball.com).
+
+        See WebKit ChangeLog for more detail.
+
+        Test: fast/viewport/ios/shrink-to-fit-large-content-width.html
+
+        * page/ViewportConfiguration.cpp:
+        (WebCore::ViewportConfiguration::initialScaleFromSize const):
+
+2019-09-25  Wenson Hsieh  <wenson_hsieh@apple.com>
+
         [iPadOS] [DataActivation] Focus moves away after focusing input fields on www.att.com
         https://bugs.webkit.org/show_bug.cgi?id=202167
         <rdar://problem/55185021>
index 0d9e398..a26c8ee 100644 (file)
@@ -266,8 +266,13 @@ double ViewportConfiguration::initialScaleFromSize(double width, double height,
     // If not, it is up to us to determine the initial scale.
     // We want a scale small enough to fit the document width-wise.
     double initialScale = 0;
-    if (width > 0 && !shouldIgnoreVerticalScalingConstraints())
-        initialScale = m_viewLayoutSize.width() / width;
+    if (!shouldIgnoreVerticalScalingConstraints()) {
+        static const double maximumContentWidthBeforePreferringExplicitWidthToAvoidExcessiveScaling = 1920;
+        if (width > maximumContentWidthBeforePreferringExplicitWidthToAvoidExcessiveScaling && m_configuration.widthIsSet && 0 < m_configuration.width && m_configuration.width < width)
+            initialScale = m_viewLayoutSize.width() / m_configuration.width;
+        else if (width > 0)
+            initialScale = m_viewLayoutSize.width() / width;
+    }
 
     // Prevent the initial scale from shrinking to a height smaller than our view's minimum height.
     if (height > 0 && height * initialScale < m_viewLayoutSize.height() && !shouldIgnoreHorizontalScalingConstraints())
index c6e8e4d..1e046ef 100644 (file)
@@ -1,3 +1,18 @@
+2019-09-25  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Page temporarily jumps to an excessively small viewport scale while loading usatoday.com
+        https://bugs.webkit.org/show_bug.cgi?id=202224
+        <rdar://problem/52906640>
+
+        Reviewed by Tim Horton.
+
+        Tweaks the content-aware shrink-to-fit algorithm to bail in the case where the content width is extremely large,
+        such that it bails instead of attempting to fit the entire content of the page. See WebCore ChangeLog for more
+        details.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::immediatelyShrinkToFitContent):
+
 2019-09-25  Alex Christensen  <achristensen@webkit.org>
 
         Don't fall back to default session if session can't be found for cookie operations
index e81b50e..10ac4bd 100644 (file)
@@ -3434,6 +3434,7 @@ bool WebPage::immediatelyShrinkToFitContent()
 
     static const int toleratedHorizontalScrollingDistance = 20;
     static const int maximumExpandedLayoutWidth = 1280;
+    static const int maximumContentWidthBeforeAvoidingShrinkToFit = 1920;
 
     auto scaledViewWidth = [&] () -> int {
         return std::round(m_viewportConfiguration.viewLayoutSize().width() / m_viewportConfiguration.initialScale());
@@ -3443,7 +3444,7 @@ bool WebPage::immediatelyShrinkToFitContent()
     int originalViewWidth = scaledViewWidth();
     int originalLayoutWidth = m_viewportConfiguration.layoutWidth();
     int originalHorizontalOverflowAmount = originalContentWidth - originalViewWidth;
-    if (originalHorizontalOverflowAmount <= toleratedHorizontalScrollingDistance || originalLayoutWidth >= maximumExpandedLayoutWidth || originalContentWidth <= originalViewWidth)
+    if (originalHorizontalOverflowAmount <= toleratedHorizontalScrollingDistance || originalLayoutWidth >= maximumExpandedLayoutWidth || originalContentWidth <= originalViewWidth || originalContentWidth > maximumContentWidthBeforeAvoidingShrinkToFit)
         return false;
 
     auto changeMinimumEffectiveDeviceWidth = [this, mainDocument] (int targetLayoutWidth) -> bool {