REGRESSION (r181879): Scrolling order on pages with focused iframe is broken.
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Jun 2015 17:53:24 +0000 (17:53 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Jun 2015 17:53:24 +0000 (17:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145637
<rdar://problem/20635581>

Reviewed by Zalan Bujtas.

Source/WebCore:

Test: platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe.html

This page revealed a bug in our RenderBox code caused by the mismatch between
our scrolling units, which are all integrally snapped, with our client height
and widths, which are not snapped at all.

In certain cases, the client height would have a small subpixel difference compared
to the scroll height, which would cause WebKit to believe it was scrollable. When
this happened, it would get stuck latched to this element and block scrolling events.

* page/Frame.cpp:
(WebCore::Frame::scrollOverflowLayer): Use roundToInt for clientWidth and clientHeight,
rather than integer truncation.
* rendering/RenderBox.cpp:
(WebCore::RenderBox::canBeScrolledAndHasScrollableArea): Need to round clientWidth
and clientHeight to compare with scrollWidth/scrollHeight.
* rendering/RenderBox.h:
(WebCore::RenderBox::hasScrollableOverflowX): Ditto.
(WebCore::RenderBox::hasScrollableOverflowY): Ditto.
* rendering/RenderMarquee.cpp:
(WebCore::RenderMarquee::computePosition): Use roundToInt for clientWidth and
clientHeight, rather than integer truncation.

LayoutTests:

* platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe-expected.txt: Added.
* platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe.html: Added.
* platform/mac-wk2/tiled-drawing/scrolling/resources/inner_content.html: Added.
* platform/mac-wk2/tiled-drawing/scrolling/resources/testContent.html: Added.
* platform/mac-wk2/tiled-drawing/scrolling/resources/testImage.png: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe.html [new file with mode: 0644]
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/inner_content.html [new file with mode: 0644]
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/testContent.html [new file with mode: 0644]
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/testImage.png [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/Frame.cpp
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderBox.h
Source/WebCore/rendering/RenderMarquee.cpp

index 7a1a5b4..f64a616 100644 (file)
@@ -1,3 +1,17 @@
+2015-06-03  Brent Fulgham  <bfulgham@apple.com>
+
+        REGRESSION (r181879): Scrolling order on pages with focused iframe is broken.
+        https://bugs.webkit.org/show_bug.cgi?id=145637
+        <rdar://problem/20635581>
+
+        Reviewed by Zalan Bujtas.
+
+        * platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe-expected.txt: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe.html: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/resources/inner_content.html: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/resources/testContent.html: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/resources/testImage.png: Added.
+
 2015-06-04  Zalan Bujtas  <zalan@apple.com>
 
         css3/filters/backdrop/backdrop-filter-with-mask.html is missing the mask layer.
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe-expected.txt b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe-expected.txt
new file mode 100644 (file)
index 0000000..a3dae06
--- /dev/null
@@ -0,0 +1,17 @@
+Inner Frame:
+
+Tests that iframe doesn't pass wheel events to main frame when scrolling inside iframe
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS Page did not receive wheel events.
+PASS iframe did not receive wheel events.
+PASS iframe received wheel events.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe.html b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe.html
new file mode 100644 (file)
index 0000000..6de4e14
--- /dev/null
@@ -0,0 +1,114 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <title>iFrame in iFrame Test</title>
+        <style>
+        * {
+            box-sizing: border-box;
+        }
+
+        .container {
+            width:100%;
+            overflow:auto;
+            height:auto;
+        }
+
+        .innercontainer {
+            height:100%;
+            width:50%;
+        }
+        </style>
+        <script src="../../../../resources/js-test-pre.js"></script>
+    </head>
+    <body>
+<script>
+
+var iframeTarget;
+var innerIFrameTarget;
+var pageScrollPositionBefore;
+var iFrameScrollPositionBefore;
+var continueCount = 5;
+
+function checkForScroll()
+{
+    // The IFrame should not have scrolled at all.
+    var pageScrollPositionAfter = document.body.scrollTop;
+    var iFrameScrollPositionAfter = window.frames['target'].document.body.scrollTop;
+    var innerIFrameScrollPositionAfter = iframeTarget.contentWindow.frames['target'].document.body.scrollTop;
+
+    if (pageScrollPositionBefore != pageScrollPositionAfter)
+        testFailed("Page received wheel events.");
+    else
+        testPassed("Page did not receive wheel events.");
+
+    if (iFrameScrollPositionBefore != iFrameScrollPositionAfter)
+        testFailed("iframe received wheel events.");
+    else
+        testPassed("iframe did not receive wheel events.");
+
+    if (innerIFrameScrollPositionBefore != innerIFrameScrollPositionAfter)
+        testPassed("iframe received wheel events.");
+    else
+        testFailed("iframe did not receive wheel events.");
+
+    finishJSTest();
+    testRunner.notifyDone();
+}
+
+function scrollTest()
+{
+    pageScrollPositionBefore = document.body.scrollTop;
+
+    iframeTarget = document.getElementById('target');
+
+    var iFrameBody = window.frames['target'].document.body;
+    iFrameScrollPositionBefore = iFrameBody.scrollTop;
+
+    innerIFrameTarget = iframeTarget.contentWindow.frames['target'].document.body;
+    innerIFrameScrollPositionBefore = innerIFrameTarget.scrollTop;
+
+    // Scroll the #source until we reach the #target.
+    var startPosX = Math.round(iframeTarget.offsetLeft) + 20;
+    var startPosY = Math.round(iframeTarget.offsetTop) + 80;
+    eventSender.mouseMoveTo(startPosX, startPosY);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'began', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'begin');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'none', 'continue');
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end');
+    eventSender.callAfterScrollingCompletes(checkForScroll);
+}
+
+function setupTopLevel()
+{
+    if (window.eventSender) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+
+        eventSender.monitorWheelEvents();
+        setTimeout(scrollTest, 0);
+    }
+}
+</script>
+        <div class="container">
+            <div class="innercontainer">
+                <div style="width:100%;">
+                    <div>Inner Frame:</div>
+                    <div style="height:92%;">
+                        <iframe id="target" name="target" src="resources/testContent.html" onload="setupTopLevel();"></iframe>
+                    </div>
+                </div>
+            </div>
+        </div>
+        <div id="console"></div>
+        <script>
+        description("Tests that iframe doesn't pass wheel events to main frame when scrolling inside iframe");
+        </script>
+        <script src="../../../../resources/js-test-post.js"></script>
+    </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/inner_content.html b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/inner_content.html
new file mode 100644 (file)
index 0000000..af796cd
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html lang="en-US">
+    <head>
+        <title>Inner iFrame Example</title>
+        <meta charset="utf-8">
+        <style>
+        img {
+            display:block;
+            max-width:100%;
+        }
+        </style>
+    </head>
+    <body style="position: relative; min-height: 100%; top: 0px;">
+        <div style="overflow:auto;">
+            <img src="testImage.png">
+            <div>TEST CASE TEST CASE TEST CASE TEST CASE</div>
+        </div>
+        <div style="overflow:auto;">
+            <h1>TEST HEADING</h1>
+            <p>Test paragraph.</p>
+            <div>TEST BUTTON 1</div>
+            <div>TEST BUTTON 2</div>      
+        </div>
+    </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/testContent.html b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/testContent.html
new file mode 100644 (file)
index 0000000..abc0f11
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<body>
+
+<p>An iframe where scrollbars are always shown:</p>
+<iframe id="target" name="target" src="./inner_content.html" width="200" height="200" scrolling="yes">
+  <p>Your browser does not support iframes.</p>
+</iframe>
+
+<p>An iframe where scrollbars are never shown:</p>
+<iframe src="./inner_content.html" width="200" height="200" scrolling="no">
+  <p>Your browser does not support iframes.</p>
+</iframe>
+
+<p>The scrolling attribute is not supported in HTML5. Use CSS instead.</p>
+
+</body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/testImage.png b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/testImage.png
new file mode 100644 (file)
index 0000000..8c79978
Binary files /dev/null and b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/resources/testImage.png differ
index f9cc082..8308b58 100644 (file)
@@ -1,3 +1,34 @@
+2015-06-03  Brent Fulgham  <bfulgham@apple.com>
+
+        REGRESSION (r181879): Scrolling order on pages with focused iframe is broken.
+        https://bugs.webkit.org/show_bug.cgi?id=145637
+        <rdar://problem/20635581>
+
+        Reviewed by Zalan Bujtas.
+
+        Test: platform/mac-wk2/tiled-drawing/scrolling/iframe_in_iframe.html
+
+        This page revealed a bug in our RenderBox code caused by the mismatch between
+        our scrolling units, which are all integrally snapped, with our client height
+        and widths, which are not snapped at all. 
+        
+        In certain cases, the client height would have a small subpixel difference compared
+        to the scroll height, which would cause WebKit to believe it was scrollable. When
+        this happened, it would get stuck latched to this element and block scrolling events. 
+
+        * page/Frame.cpp:
+        (WebCore::Frame::scrollOverflowLayer): Use roundToInt for clientWidth and clientHeight,
+        rather than integer truncation.
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::canBeScrolledAndHasScrollableArea): Need to round clientWidth
+        and clientHeight to compare with scrollWidth/scrollHeight.
+        * rendering/RenderBox.h:
+        (WebCore::RenderBox::hasScrollableOverflowX): Ditto.
+        (WebCore::RenderBox::hasScrollableOverflowY): Ditto.
+        * rendering/RenderMarquee.cpp:
+        (WebCore::RenderMarquee::computePosition): Use roundToInt for clientWidth and
+        clientHeight, rather than integer truncation.
+
 2015-06-04  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [Cocoa] Clean up m_font inside FontPlatformData
index b25c40e..4fc1ff8 100644 (file)
@@ -501,7 +501,7 @@ void Frame::scrollOverflowLayer(RenderLayer* layer, const IntRect& visibleRect,
     int x = layer->scrollXOffset();
     int exposeLeft = exposeRect.x();
     int exposeRight = exposeLeft + exposeRect.width();
-    int clientWidth = box->clientWidth();
+    int clientWidth = roundToInt(box->clientWidth());
     if (exposeLeft <= 0)
         x = std::max(0, x + exposeLeft - clientWidth / 2);
     else if (exposeRight >= clientWidth)
@@ -510,7 +510,7 @@ void Frame::scrollOverflowLayer(RenderLayer* layer, const IntRect& visibleRect,
     int y = layer->scrollYOffset();
     int exposeTop = exposeRect.y();
     int exposeBottom = exposeTop + exposeRect.height();
-    int clientHeight = box->clientHeight();
+    int clientHeight = roundToInt(box->clientHeight());
     if (exposeTop <= 0)
         y = std::max(0, y + exposeTop - clientHeight / 2);
     else if (exposeBottom >= clientHeight)
index eb1a49d..df1987a 100644 (file)
@@ -876,7 +876,7 @@ bool RenderBox::logicalScroll(ScrollLogicalDirection direction, ScrollGranularit
 
 bool RenderBox::canBeScrolledAndHasScrollableArea() const
 {
-    return canBeProgramaticallyScrolled() && (scrollHeight() != clientHeight() || scrollWidth() != clientWidth());
+    return canBeProgramaticallyScrolled() && (scrollHeight() != roundToInt(clientHeight()) || scrollWidth() != roundToInt(clientWidth()));
 }
 
 bool RenderBox::isScrollableOrRubberbandableBox() const
index 4bc1490..408008f 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
- * Copyright (C) 2003, 2006, 2007 Apple Inc. All rights reserved.
+ * Copyright (C) 2003, 2006, 2007, 2015 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -477,8 +477,8 @@ public:
     bool scrollsOverflow() const { return scrollsOverflowX() || scrollsOverflowY(); }
     bool scrollsOverflowX() const { return hasOverflowClip() && (style().overflowX() == OSCROLL || hasHorizontalScrollbarWithAutoBehavior()); }
     bool scrollsOverflowY() const { return hasOverflowClip() && (style().overflowY() == OSCROLL || hasVerticalScrollbarWithAutoBehavior()); }
-    bool hasScrollableOverflowX() const { return scrollsOverflowX() && scrollWidth() != clientWidth(); }
-    bool hasScrollableOverflowY() const { return scrollsOverflowY() && scrollHeight() != clientHeight(); }
+    bool hasScrollableOverflowX() const { return scrollsOverflowX() && scrollWidth() != roundToInt(clientWidth()); }
+    bool hasScrollableOverflowY() const { return scrollsOverflowY() && scrollHeight() != roundToInt(clientHeight()); }
 
     bool usesCompositedScrolling() const;
     
index 26e30e4..ab6cf54 100644 (file)
@@ -135,7 +135,7 @@ int RenderMarquee::computePosition(EMarqueeDirection dir, bool stopAtContentEdge
     }
     else {
         int contentHeight = box->layoutOverflowRect().maxY() - box->borderTop() + box->paddingBottom();
-        int clientHeight = box->clientHeight();
+        int clientHeight = roundToInt(box->clientHeight());
         if (dir == MUP) {
             if (stopAtContentEdge)
                  return std::min(contentHeight - clientHeight, 0);
@@ -271,7 +271,7 @@ void RenderMarquee::timerFired()
             addIncrement = !addIncrement;
         }
         bool positive = range > 0;
-        int clientSize = (isHorizontal() ? m_layer->renderBox()->clientWidth() : m_layer->renderBox()->clientHeight());
+        int clientSize = (isHorizontal() ? roundToInt(m_layer->renderBox()->clientWidth()) : roundToInt(m_layer->renderBox()->clientHeight()));
         int increment = abs(intValueForLength(m_layer->renderer().style().marqueeIncrement(), clientSize));
         int currentPos = (isHorizontal() ? m_layer->scrollXOffset() : m_layer->scrollYOffset());
         newPos =  currentPos + (addIncrement ? increment : -increment);