Sites that use a device-width viewport but don't have enough height to fill the view...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Mar 2015 21:07:06 +0000 (21:07 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Mar 2015 21:07:06 +0000 (21:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=142664
<rdar://problem/18859470>

Reviewed by Benjamin Poulain.

* page/ViewportConfiguration.cpp:
(WebCore::ViewportConfiguration::shouldIgnoreHorizontalScalingConstraints):
(WebCore::ViewportConfiguration::shouldIgnoreVerticalScalingConstraints):
(WebCore::ViewportConfiguration::shouldIgnoreScalingConstraints):
Split shouldIgnoreScalingConstraints into one for each dimension.

(WebCore::ViewportConfiguration::initialScale):
(WebCore::ViewportConfiguration::minimumScale):
Don't force the initial and minimum scales to cover the whole view if the
page claims to want to lay out to device width but then lays out too big.
This will allow pages that misbehave in this way to scale down further
than they previously could, but will result in a region of empty background
color being exposed at the initial/minimum scale.

(WebCore::ViewportConfiguration::description):
Update the logging to show each dimension separately.

* page/ViewportConfiguration.h:

* UIProcess/ios/WKScrollView.mm:
(-[WKScrollView _rubberBandOffsetForOffset:maxOffset:minOffset:range:outside:]):
Now that the WKContentView can (without pinching) be smaller than the unobscured
region of the WKWebView, we need to take that into account when deciding where
to retarget scrolling.

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

Source/WebCore/ChangeLog
Source/WebCore/page/ViewportConfiguration.cpp
Source/WebCore/page/ViewportConfiguration.h
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/ios/WKScrollView.mm

index 520cbd7..84bc41e 100644 (file)
@@ -1,3 +1,30 @@
+2015-03-13  Timothy Horton  <timothy_horton@apple.com>
+
+        Sites that use a device-width viewport but don't have enough height to fill the view are scaled up
+        https://bugs.webkit.org/show_bug.cgi?id=142664
+        <rdar://problem/18859470>
+
+        Reviewed by Benjamin Poulain.
+
+        * page/ViewportConfiguration.cpp:
+        (WebCore::ViewportConfiguration::shouldIgnoreHorizontalScalingConstraints):
+        (WebCore::ViewportConfiguration::shouldIgnoreVerticalScalingConstraints):
+        (WebCore::ViewportConfiguration::shouldIgnoreScalingConstraints):
+        Split shouldIgnoreScalingConstraints into one for each dimension.
+
+        (WebCore::ViewportConfiguration::initialScale):
+        (WebCore::ViewportConfiguration::minimumScale):
+        Don't force the initial and minimum scales to cover the whole view if the
+        page claims to want to lay out to device width but then lays out too big.
+        This will allow pages that misbehave in this way to scale down further
+        than they previously could, but will result in a region of empty background
+        color being exposed at the initial/minimum scale.
+
+        (WebCore::ViewportConfiguration::description):
+        Update the logging to show each dimension separately.
+
+        * page/ViewportConfiguration.h:
+
 2015-03-13  Mark Lam  <mark.lam@apple.com>
 
         Replace TCSpinLock with a new WTF::SpinLock based on WTF::Atomic.
index 00c6c3a..dbdc521 100644 (file)
@@ -97,7 +97,7 @@ IntSize ViewportConfiguration::layoutSize() const
     return IntSize(layoutWidth(), layoutHeight());
 }
 
-bool ViewportConfiguration::shouldIgnoreScalingConstraints() const
+bool ViewportConfiguration::shouldIgnoreHorizontalScalingConstraints() const
 {
     if (!m_canIgnoreScalingConstraints)
         return false;
@@ -106,16 +106,29 @@ bool ViewportConfiguration::shouldIgnoreScalingConstraints() const
     if (m_viewportArguments.width == ViewportArguments::ValueDeviceWidth)
         return laidOutWiderThanViewport;
 
+    if (m_configuration.initialScaleIsSet && m_configuration.initialScale == 1)
+        return laidOutWiderThanViewport;
+
+    return false;
+}
+
+bool ViewportConfiguration::shouldIgnoreVerticalScalingConstraints() const
+{
+    if (!m_canIgnoreScalingConstraints)
+        return false;
+
     bool laidOutTallerThanViewport = m_contentSize.height() > layoutHeight();
     if (m_viewportArguments.height == ViewportArguments::ValueDeviceHeight)
         return laidOutTallerThanViewport;
 
-    if (m_configuration.initialScaleIsSet && m_configuration.initialScale == 1)
-        return laidOutWiderThanViewport;
-
     return false;
 }
 
+bool ViewportConfiguration::shouldIgnoreScalingConstraints() const
+{
+    return shouldIgnoreHorizontalScalingConstraints() || shouldIgnoreVerticalScalingConstraints();
+}
+
 double ViewportConfiguration::initialScale() const
 {
     ASSERT(!constraintsAreAllRelative(m_configuration));
@@ -130,12 +143,12 @@ double ViewportConfiguration::initialScale() const
     const FloatSize& minimumLayoutSize = m_minimumLayoutSize;
     double width = m_contentSize.width() > 0 ? m_contentSize.width() : layoutWidth();
     double initialScale = 0;
-    if (width > 0)
+    if (width > 0 && !shouldIgnoreVerticalScalingConstraints())
         initialScale = minimumLayoutSize.width() / width;
 
-    // Prevent the intial scale from shrinking to a height smaller than our view's minimum height.
+    // Prevent the initial scale from shrinking to a height smaller than our view's minimum height.
     double height = m_contentSize.height() > 0 ? m_contentSize.height() : layoutHeight();
-    if (height > 0 && height * initialScale < minimumLayoutSize.height())
+    if (height > 0 && height * initialScale < minimumLayoutSize.height() && !shouldIgnoreHorizontalScalingConstraints())
         initialScale = minimumLayoutSize.height() / height;
     return std::min(std::max(initialScale, shouldIgnoreScalingConstraints() ? m_defaultConfiguration.minimumScale : m_configuration.minimumScale), m_configuration.maximumScale);
 }
@@ -151,11 +164,11 @@ double ViewportConfiguration::minimumScale() const
 
     const FloatSize& minimumLayoutSize = m_minimumLayoutSize;
     double contentWidth = m_contentSize.width();
-    if (contentWidth > 0 && contentWidth * minimumScale < minimumLayoutSize.width())
+    if (contentWidth > 0 && contentWidth * minimumScale < minimumLayoutSize.width() && !shouldIgnoreVerticalScalingConstraints())
         minimumScale = minimumLayoutSize.width() / contentWidth;
 
     double contentHeight = m_contentSize.height();
-    if (contentHeight > 0 && contentHeight * minimumScale < minimumLayoutSize.height())
+    if (contentHeight > 0 && contentHeight * minimumScale < minimumLayoutSize.height() && !shouldIgnoreHorizontalScalingConstraints())
         minimumScale = minimumLayoutSize.height() / contentHeight;
 
     minimumScale = std::min(std::max(minimumScale, m_configuration.minimumScale), m_configuration.maximumScale);
@@ -490,7 +503,9 @@ CString ViewportConfiguration::description() const
     ts.writeIndent();
     ts << "(computed layout size " << layoutSize() << ")\n";
     ts.writeIndent();
-    ts << "(ignoring scaling constraints " << (shouldIgnoreScalingConstraints() ? "true" : "false") << ")";
+    ts << "(ignoring horizontal scaling constraints " << (shouldIgnoreHorizontalScalingConstraints() ? "true" : "false") << ")\n";
+    ts.writeIndent();
+    ts << "(ignoring vertical scaling constraints " << (shouldIgnoreVerticalScalingConstraints() ? "true" : "false") << ")";
     ts.decreaseIndent();
 
     ts << ")\n";
index 97385ac..e1acf6e 100644 (file)
@@ -101,7 +101,10 @@ private:
     double viewportArgumentsLength(double length) const;
     int layoutWidth() const;
     int layoutHeight() const;
+
     bool shouldIgnoreScalingConstraints() const;
+    bool shouldIgnoreVerticalScalingConstraints() const;
+    bool shouldIgnoreHorizontalScalingConstraints() const;
 
     Parameters m_configuration;
     Parameters m_defaultConfiguration;
index e5f0496..f01e10e 100644 (file)
@@ -1,3 +1,17 @@
+2015-03-13  Timothy Horton  <timothy_horton@apple.com>
+
+        Sites that use a device-width viewport but don't have enough height to fill the view are scaled up
+        https://bugs.webkit.org/show_bug.cgi?id=142664
+        <rdar://problem/18859470>
+
+        Reviewed by Benjamin Poulain.
+
+        * UIProcess/ios/WKScrollView.mm:
+        (-[WKScrollView _rubberBandOffsetForOffset:maxOffset:minOffset:range:outside:]):
+        Now that the WKContentView can (without pinching) be smaller than the unobscured
+        region of the WKWebView, we need to take that into account when deciding where
+        to retarget scrolling.
+
 2015-03-13  Mark Lam  <mark.lam@apple.com>
 
         Replace TCSpinLock with a new WTF::SpinLock based on WTF::Atomic.
index d75d2f3..639894b 100644 (file)
@@ -176,6 +176,11 @@ static inline bool valuesAreWithinOnePixel(CGFloat a, CGFloat b)
     CGRect bounds = self.bounds;
 
     CGFloat minimalHorizontalRange = bounds.size.width - contentInsets.left - contentInsets.right;
+    CGFloat contentWidthAtMinimumScale = contentSize.width * (self.minimumZoomScale / self.zoomScale);
+    if (contentWidthAtMinimumScale < minimalHorizontalRange) {
+        CGFloat unobscuredEmptyHorizontalMarginAtMinimumScale = minimalHorizontalRange - contentWidthAtMinimumScale;
+        minimalHorizontalRange -= unobscuredEmptyHorizontalMarginAtMinimumScale;
+    }
     if (contentSize.width < minimalHorizontalRange) {
         if (valuesAreWithinOnePixel(minOffset, -contentInsets.left)
             && valuesAreWithinOnePixel(maxOffset, contentSize.width + contentInsets.right - bounds.size.width)
@@ -188,6 +193,11 @@ static inline bool valuesAreWithinOnePixel(CGFloat a, CGFloat b)
     }
 
     CGFloat minimalVerticalRange = bounds.size.height - contentInsets.top - contentInsets.bottom;
+    CGFloat contentHeightAtMinimumScale = contentSize.height * (self.minimumZoomScale / self.zoomScale);
+    if (contentHeightAtMinimumScale < minimalVerticalRange) {
+        CGFloat unobscuredEmptyVerticalMarginAtMinimumScale = minimalVerticalRange - contentHeightAtMinimumScale;
+        minimalVerticalRange -= unobscuredEmptyVerticalMarginAtMinimumScale;
+    }
     if (contentSize.height < minimalVerticalRange) {
         if (valuesAreWithinOnePixel(minOffset, -contentInsets.top)
             && valuesAreWithinOnePixel(maxOffset, contentSize.height + contentInsets.bottom - bounds.size.height)