[iPadOS] Unable to increase zoom level on Google using the Aa menu
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Aug 2019 14:49:25 +0000 (14:49 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Aug 2019 14:49:25 +0000 (14:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200453
<rdar://problem/52278579>

Reviewed by Tim Horton.

Source/WebCore:

Makes a couple of minor adjustments to how layout size scale factor is handled in ViewportConfiguration, to
address some scenarios in which adjusting WKWebView's _viewScale does not have any apparent effect on the page.
See changes below for more detail.

Tests: fast/viewport/ios/non-responsive-viewport-after-changing-view-scale.html
       fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale.html

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

When the page is either zoomed in or zoomed out using _viewScale, let the specified initial scale take
precedence over the scale computed by fitting the content width to the view width, or the scale computed by
fitting the content height to the view height.

This avoids a scenario in which nothing happens when increasing view scale in a responsively designed web page
that has a fixed minimum width. Before this change, when computing the initial scale at a view scale that would
not allow the entire content width of the page to fit within the viewport, the new initial scale would remain
unchanged if the initial scale in the meta viewport is not also set to 1, because a new initial scale would be
computed in ViewportConfiguration::initialScaleFromSize to accomodate for the entire content width.

Our new behavior allows us to zoom into the page, even if doing so would cause horizontal scrolling.

(WebCore::ViewportConfiguration::updateConfiguration):

When the page is either zoomed in or zoomed out using _viewScale and the default viewport configuration has a
fixed width (e.g. on iPhone), then adjust the width of the default viewport configuration to account for the
_viewScale. For example, the default width of a viewport-less web page is 980px on iPhone; at a view scale of 2,
this would become 490px instead, and at 0.5 view scale, it would become 1960px.

This ensures that on iPhone, for web pages without a meta viewport, changing the view scale still changes the
layout and initial scale of the web page.

* page/ViewportConfiguration.h:
(WebCore::ViewportConfiguration::layoutSizeIsExplicitlyScaled const):

LayoutTests:

Adds a couple of layout tests (with device-specific expectations) to verify that the two scenarios targeted by
this change are fixed.

* fast/viewport/ios/non-responsive-viewport-after-changing-view-scale-expected.txt: Added.
* fast/viewport/ios/non-responsive-viewport-after-changing-view-scale.html: Added.

Verifies that, for a page with no viewport meta tag (where we fall back to a fixed 980px viewport on iPhone),
changing view scale still changes page scale and window size.

* fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale-expected.txt: Added.
* fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale.html: Added.

Verifies that, for a page with a responsive meta viewport tag containing a fixed-width element that forces a
minimum width for the page, setting the view scale such that the page scrolls horizontally (2.5) doesn't result
in the initial scale being adjusted back to the maximum scale that would accomodate the full contents of the
page (2).

* platform/ipad/fast/viewport/ios/non-responsive-viewport-after-changing-view-scale-expected.txt: Added.
* platform/ipad/fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale-expected.txt: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/viewport/ios/non-responsive-viewport-after-changing-view-scale-expected.txt [new file with mode: 0644]
LayoutTests/fast/viewport/ios/non-responsive-viewport-after-changing-view-scale.html [new file with mode: 0644]
LayoutTests/fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale-expected.txt [new file with mode: 0644]
LayoutTests/fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale.html [new file with mode: 0644]
LayoutTests/platform/ipad/fast/viewport/ios/non-responsive-viewport-after-changing-view-scale-expected.txt [new file with mode: 0644]
LayoutTests/platform/ipad/fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale-expected.txt [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/ViewportConfiguration.cpp
Source/WebCore/page/ViewportConfiguration.h

index 05cfe6b..3e723f1 100644 (file)
@@ -1,3 +1,31 @@
+2019-08-06  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iPadOS] Unable to increase zoom level on Google using the Aa menu
+        https://bugs.webkit.org/show_bug.cgi?id=200453
+        <rdar://problem/52278579>
+
+        Reviewed by Tim Horton.
+
+        Adds a couple of layout tests (with device-specific expectations) to verify that the two scenarios targeted by
+        this change are fixed.
+
+        * fast/viewport/ios/non-responsive-viewport-after-changing-view-scale-expected.txt: Added.
+        * fast/viewport/ios/non-responsive-viewport-after-changing-view-scale.html: Added.
+
+        Verifies that, for a page with no viewport meta tag (where we fall back to a fixed 980px viewport on iPhone),
+        changing view scale still changes page scale and window size.
+
+        * fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale-expected.txt: Added.
+        * fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale.html: Added.
+
+        Verifies that, for a page with a responsive meta viewport tag containing a fixed-width element that forces a
+        minimum width for the page, setting the view scale such that the page scrolls horizontally (2.5) doesn't result
+        in the initial scale being adjusted back to the maximum scale that would accomodate the full contents of the
+        page (2).
+
+        * platform/ipad/fast/viewport/ios/non-responsive-viewport-after-changing-view-scale-expected.txt: Added.
+        * platform/ipad/fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale-expected.txt: Added.
+
 2019-08-05  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Styles: show @supports CSS groupings
diff --git a/LayoutTests/fast/viewport/ios/non-responsive-viewport-after-changing-view-scale-expected.txt b/LayoutTests/fast/viewport/ios/non-responsive-viewport-after-changing-view-scale-expected.txt
new file mode 100644 (file)
index 0000000..f2f1d13
--- /dev/null
@@ -0,0 +1,17 @@
+setViewScale(1.00)
+window size: [980, 1678]
+zoom scale: 0.33
+
+setViewScale(0.50)
+window size: [1960, 3357]
+zoom scale: 0.16
+
+setViewScale(2.00)
+window size: [490, 839]
+zoom scale: 0.65
+
+setViewScale(1.00)
+window size: [980, 1678]
+zoom scale: 0.33
+
+
diff --git a/LayoutTests/fast/viewport/ios/non-responsive-viewport-after-changing-view-scale.html b/LayoutTests/fast/viewport/ios/non-responsive-viewport-after-changing-view-scale.html
new file mode 100644 (file)
index 0000000..1f9e4b3
--- /dev/null
@@ -0,0 +1,54 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+    <style>
+        #output {
+            width: 100%;
+            height: 100%;
+            overflow: scroll;
+        }
+
+        #bar {
+            width: 980px;
+            height: 100px;
+            background: linear-gradient(to right, red 0%, green 50%, blue 100%);
+        }
+
+        body {
+            margin: 0;
+            width: 100%;
+            height: 100%;
+        }
+    </style>
+    <script src="../../../resources/ui-helper.js"></script>
+    <script>
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+    }
+
+    async function runTest() {
+        const appendOutput = message => {
+            output.appendChild(document.createTextNode(message));
+            output.appendChild(document.createElement("br"));
+        };
+
+        for (const targetScale of [1, 0.5, 2, 1]) {
+            await UIHelper.setViewScale(targetScale);
+            await UIHelper.ensurePresentationUpdate();
+            appendOutput(`setViewScale(${targetScale.toFixed(2)})`);
+            appendOutput(`window size: [${innerWidth}, ${innerHeight}]`);
+            appendOutput(`zoom scale: ${(await UIHelper.zoomScale()).toFixed(2)}`);
+            appendOutput("");
+        }
+
+        testRunner.notifyDone();
+    }
+    </script>
+</head>
+
+<body onload="runTest()">
+    <div id="bar"></div>
+    <pre id="output"></pre>
+</body>
+</html>
diff --git a/LayoutTests/fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale-expected.txt b/LayoutTests/fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale-expected.txt
new file mode 100644 (file)
index 0000000..852d9cd
--- /dev/null
@@ -0,0 +1,17 @@
+setViewScale(1.00)
+window size: [320, 548]
+zoom scale: 1.00
+
+setViewScale(2.00)
+window size: [160, 274]
+zoom scale: 2.00
+
+setViewScale(2.50)
+window size: [128, 219]
+zoom scale: 2.50
+
+setViewScale(1.00)
+window size: [320, 548]
+zoom scale: 1.00
+
+
diff --git a/LayoutTests/fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale.html b/LayoutTests/fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale.html
new file mode 100644 (file)
index 0000000..0448be7
--- /dev/null
@@ -0,0 +1,55 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <style>
+        #output {
+            width: 100%;
+            height: 100%;
+            overflow: scroll;
+        }
+
+        #bar {
+            width: 160px;
+            height: 40px;
+            background: linear-gradient(to right, red 0%, green 50%, blue 100%);
+        }
+
+        body {
+            margin: 0;
+            width: 100%;
+            height: 100%;
+        }
+    </style>
+    <script src="../../../resources/ui-helper.js"></script>
+    <script>
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+    }
+
+    async function runTest() {
+        const appendOutput = message => {
+            output.appendChild(document.createTextNode(message));
+            output.appendChild(document.createElement("br"));
+        };
+
+        for (const targetScale of [1, 2, 2.5, 1]) {
+            await UIHelper.setViewScale(targetScale);
+            await UIHelper.ensurePresentationUpdate();
+            appendOutput(`setViewScale(${targetScale.toFixed(2)})`);
+            appendOutput(`window size: [${innerWidth}, ${innerHeight}]`);
+            appendOutput(`zoom scale: ${(await UIHelper.zoomScale()).toFixed(2)}`);
+            appendOutput("");
+        }
+
+        testRunner.notifyDone();
+    }
+    </script>
+</head>
+
+<body onload="runTest()">
+    <div id="bar"></div>
+    <pre id="output"></pre>
+</body>
+</html>
diff --git a/LayoutTests/platform/ipad/fast/viewport/ios/non-responsive-viewport-after-changing-view-scale-expected.txt b/LayoutTests/platform/ipad/fast/viewport/ios/non-responsive-viewport-after-changing-view-scale-expected.txt
new file mode 100644 (file)
index 0000000..30384f7
--- /dev/null
@@ -0,0 +1,17 @@
+setViewScale(1.00)
+window size: [980, 1281]
+zoom scale: 0.78
+
+setViewScale(0.50)
+window size: [1960, 2562]
+zoom scale: 0.39
+
+setViewScale(2.00)
+window size: [490, 641]
+zoom scale: 1.57
+
+setViewScale(1.00)
+window size: [980, 1281]
+zoom scale: 0.78
+
+
diff --git a/LayoutTests/platform/ipad/fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale-expected.txt b/LayoutTests/platform/ipad/fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale-expected.txt
new file mode 100644 (file)
index 0000000..98ab8b2
--- /dev/null
@@ -0,0 +1,17 @@
+setViewScale(1.00)
+window size: [768, 1004]
+zoom scale: 1.00
+
+setViewScale(2.00)
+window size: [384, 502]
+zoom scale: 2.00
+
+setViewScale(2.50)
+window size: [307, 401]
+zoom scale: 2.50
+
+setViewScale(1.00)
+window size: [768, 1004]
+zoom scale: 1.00
+
+
index 067022d..65e5879 100644 (file)
@@ -1,3 +1,46 @@
+2019-08-06  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iPadOS] Unable to increase zoom level on Google using the Aa menu
+        https://bugs.webkit.org/show_bug.cgi?id=200453
+        <rdar://problem/52278579>
+
+        Reviewed by Tim Horton.
+
+        Makes a couple of minor adjustments to how layout size scale factor is handled in ViewportConfiguration, to
+        address some scenarios in which adjusting WKWebView's _viewScale does not have any apparent effect on the page.
+        See changes below for more detail.
+
+        Tests: fast/viewport/ios/non-responsive-viewport-after-changing-view-scale.html
+               fast/viewport/ios/responsive-viewport-with-minimum-width-after-changing-view-scale.html
+
+        * page/ViewportConfiguration.cpp:
+        (WebCore::ViewportConfiguration::initialScaleFromSize const):
+
+        When the page is either zoomed in or zoomed out using _viewScale, let the specified initial scale take
+        precedence over the scale computed by fitting the content width to the view width, or the scale computed by
+        fitting the content height to the view height.
+
+        This avoids a scenario in which nothing happens when increasing view scale in a responsively designed web page
+        that has a fixed minimum width. Before this change, when computing the initial scale at a view scale that would
+        not allow the entire content width of the page to fit within the viewport, the new initial scale would remain
+        unchanged if the initial scale in the meta viewport is not also set to 1, because a new initial scale would be
+        computed in ViewportConfiguration::initialScaleFromSize to accomodate for the entire content width.
+
+        Our new behavior allows us to zoom into the page, even if doing so would cause horizontal scrolling.
+
+        (WebCore::ViewportConfiguration::updateConfiguration):
+
+        When the page is either zoomed in or zoomed out using _viewScale and the default viewport configuration has a
+        fixed width (e.g. on iPhone), then adjust the width of the default viewport configuration to account for the
+        _viewScale. For example, the default width of a viewport-less web page is 980px on iPhone; at a view scale of 2,
+        this would become 490px instead, and at 0.5 view scale, it would become 1960px.
+
+        This ensures that on iPhone, for web pages without a meta viewport, changing the view scale still changes the
+        layout and initial scale of the web page.
+
+        * page/ViewportConfiguration.h:
+        (WebCore::ViewportConfiguration::layoutSizeIsExplicitlyScaled const):
+
 2019-08-05  Zalan Bujtas  <zalan@apple.com>
 
         [LFC] Remove out-of-flow descendants from Container
index 031e034..0d9e398 100644 (file)
@@ -246,6 +246,18 @@ double ViewportConfiguration::initialScaleFromSize(double width, double height,
 {
     ASSERT(!constraintsAreAllRelative(m_configuration));
 
+    auto clampToMinimumAndMaximumScales = [&] (double initialScale) {
+        return clampTo<double>(initialScale, shouldIgnoreScalingConstraints ? m_defaultConfiguration.minimumScale : m_configuration.minimumScale, m_configuration.maximumScale);
+    };
+
+    if (layoutSizeIsExplicitlyScaled()) {
+        if (m_configuration.initialScaleIsSet)
+            return clampToMinimumAndMaximumScales(m_configuration.initialScale);
+
+        if (m_configuration.width > 0)
+            return clampToMinimumAndMaximumScales(m_viewLayoutSize.width() / m_configuration.width);
+    }
+
     // If the document has specified its own initial scale, use it regardless.
     // This is guaranteed to be sanity checked already, so no need for MIN/MAX.
     if (m_configuration.initialScaleIsSet && !shouldIgnoreScalingConstraints)
@@ -261,7 +273,7 @@ double ViewportConfiguration::initialScaleFromSize(double width, double height,
     if (height > 0 && height * initialScale < m_viewLayoutSize.height() && !shouldIgnoreHorizontalScalingConstraints())
         initialScale = m_viewLayoutSize.height() / height;
 
-    return std::min(std::max(initialScale, shouldIgnoreScalingConstraints ? m_defaultConfiguration.minimumScale : m_configuration.minimumScale), m_configuration.maximumScale);
+    return clampToMinimumAndMaximumScales(initialScale);
 }
 
 double ViewportConfiguration::initialScale() const
@@ -458,6 +470,11 @@ void ViewportConfiguration::updateConfiguration()
     bool viewportArgumentsOverridesWidth;
     bool viewportArgumentsOverridesHeight;
 
+    auto effectiveLayoutScale = effectiveLayoutSizeScaleFactor();
+
+    if (layoutSizeIsExplicitlyScaled())
+        m_configuration.width /= effectiveLayoutScale;
+
     applyViewportArgument(m_configuration.minimumScale, m_viewportArguments.minZoom, minimumViewportArgumentsScaleFactor, maximumViewportArgumentsScaleFactor);
     applyViewportArgument(m_configuration.maximumScale, m_viewportArguments.maxZoom, m_configuration.minimumScale, maximumViewportArgumentsScaleFactor);
     applyViewportArgument(m_configuration.initialScale, viewportArgumentsOverridesInitialScale, m_viewportArguments.zoom, m_configuration.minimumScale, m_configuration.maximumScale);
@@ -486,7 +503,6 @@ void ViewportConfiguration::updateConfiguration()
 
     m_configuration.avoidsUnsafeArea = m_viewportArguments.viewportFit != ViewportFit::Cover;
     m_configuration.initialScaleIgnoringLayoutScaleFactor = m_configuration.initialScale;
-    float effectiveLayoutScale = effectiveLayoutSizeScaleFactor();
     m_configuration.initialScale *= effectiveLayoutScale;
     m_configuration.minimumScale *= effectiveLayoutScale;
     m_configuration.maximumScale *= effectiveLayoutScale;
index cd7c05d..20b2767 100644 (file)
@@ -156,6 +156,11 @@ private:
     void updateDefaultConfiguration();
     bool canOverrideConfigurationParameters() const;
 
+    constexpr bool layoutSizeIsExplicitlyScaled() const
+    {
+        return m_layoutSizeScaleFactor != 1;
+    }
+
     constexpr double forceAlwaysUserScalableMaximumScale() const
     {
         const double forceAlwaysUserScalableMaximumScaleIgnoringLayoutScaleFactor = 5;