Attachment 359686 Details for Bug 192303 – patch

[patch] patch

no-subframe-delegate-3.patch (text/plain), 11.65 KB, created by Antti Koivisto on 2019-01-21 06:06:05 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2019-01-21 06:06:05 PST

Size: 11.65 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 240234)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,28 @@
>+2019-01-21  Antti Koivisto  <antti@apple.com>
>+
>+        [iOS] Handle hit testing for subframes
>+        https://bugs.webkit.org/show_bug.cgi?id=192303
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Don't set delegatesScrolling bit for subframes on iOS. It is meant for top level application
>+        controlled scrolling. This fixes coordinate conversions for subframes and makes events work.
>+
>+        Test by Frederic Wang.
>+
>+        Test: fast/scrolling/ios/hit-testing-iframe.html
>+
>+        * platform/ScrollView.cpp:
>+        (WebCore::ScrollView::managesScrollbars const):
>+
>+        Add a function that tells if the scrollview should deal with scrollbars at all
>+        This is always false on iOS and not connected to delegatesScrolling bit.
>+
>+        (WebCore::ScrollView::updateScrollbars):
>+        * platform/ScrollView.h:
>+        * rendering/RenderLayerCompositor.cpp:
>+        (WebCore::RenderLayerCompositor::shouldCompositeOverflowControls const):
>+
> 2019-01-21  Antti Koivisto  <antti@apple.com>
> 
>         Move delegatesScrolling() tests to lower level conversion function in ScrollView
>Index: Source/WebCore/platform/ScrollView.cpp
>===================================================================
>--- Source/WebCore/platform/ScrollView.cpp	(revision 240232)
>+++ Source/WebCore/platform/ScrollView.cpp	(working copy)
>@@ -542,6 +542,19 @@ IntSize ScrollView::overhangAmount() con
>     return stretch;
> }
> 
>+bool ScrollView::managesScrollbars() const
>+{
>+#if PLATFORM(IOS_FAMILY)
>+    return false;
>+#else
>+    if (platformWidget())
>+        return false;
>+    if (delegatesScrolling())
>+        return false;
>+    return true;
>+#endif
>+}
>+
> void ScrollView::updateScrollbars(const ScrollPosition& desiredPosition)
> {
>     LOG_WITH_STREAM(Scrolling, stream << "ScrollView::updateScrollbars " << desiredPosition);
>@@ -549,7 +562,7 @@ void ScrollView::updateScrollbars(const 
>     if (m_inUpdateScrollbars || prohibitsScrolling() || platformWidget())
>         return;
>     
>-    if (delegatesScrolling()) {
>+    if (!managesScrollbars()) {
>         if (scrollOriginChanged()) {
>             ScrollableArea::scrollToOffsetWithoutAnimation(scrollOffsetFromPosition(desiredPosition));
>             resetScrollOriginChanged();
>Index: Source/WebCore/platform/ScrollView.h
>===================================================================
>--- Source/WebCore/platform/ScrollView.h	(revision 240232)
>+++ Source/WebCore/platform/ScrollView.h	(working copy)
>@@ -378,6 +378,8 @@ public:
>     void setAllowsUnclampedScrollPositionForTesting(bool allowsUnclampedScrollPosition) { m_allowsUnclampedScrollPosition = allowsUnclampedScrollPosition; }
>     bool allowsUnclampedScrollPosition() const { return m_allowsUnclampedScrollPosition; }
> 
>+    bool managesScrollbars() const;
>+
> protected:
>     ScrollView();
> 
>Index: Source/WebCore/rendering/RenderLayerCompositor.cpp
>===================================================================
>--- Source/WebCore/rendering/RenderLayerCompositor.cpp	(revision 240232)
>+++ Source/WebCore/rendering/RenderLayerCompositor.cpp	(working copy)
>@@ -3007,10 +3007,7 @@ bool RenderLayerCompositor::shouldCompos
> {
>     auto& frameView = m_renderView.frameView();
> 
>-    if (frameView.platformWidget())
>-        return false;
>-
>-    if (frameView.delegatesScrolling())
>+    if (!frameView.managesScrollbars())
>         return false;
> 
>     if (documentUsesTiledBacking())
>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 240234)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2019-01-21  Antti Koivisto  <antti@apple.com>
>+
>+        [iOS] Handle hit testing for subframes
>+        https://bugs.webkit.org/show_bug.cgi?id=192303
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
>+        (WebKit::WebFrameLoaderClient::transitionToCommittedForNewPage):
>+
>+        Only set delegatesScrolling for the main frame.
>+
> 2019-01-20  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         Unreviewed, fix -Wreturn-type warning
>Index: Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>===================================================================
>--- Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp	(revision 240232)
>+++ Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp	(working copy)
>@@ -1439,7 +1439,8 @@ void WebFrameLoaderClient::transitionToC
>     m_frame->coreFrame()->view()->setViewExposedRect(webPage->drawingArea()->viewExposedRect());
> #endif
> #if PLATFORM(IOS_FAMILY)
>-    m_frame->coreFrame()->view()->setDelegatesScrolling(true);
>+    if (isMainFrame)
>+        m_frame->coreFrame()->view()->setDelegatesScrolling(true);
> #endif
> 
>     if (webPage->scrollPinningBehavior() != DoNotPin)
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 240234)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2019-01-21  Antti Koivisto  <antti@apple.com>
>+
>+        [iOS] Handle hit testing for subframes
>+        https://bugs.webkit.org/show_bug.cgi?id=192303
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test by Frederic Wang.
>+
>+        * fast/scrolling/ios/hit-testing-iframe-expected.html: Added.
>+        * fast/scrolling/ios/hit-testing-iframe.html: Added.
>+
> 2019-01-21  Frederic Wang  <fwang@igalia.com>
> 
>         Add a basic test for scrollable iframe on iOS
>Index: LayoutTests/fast/scrolling/ios/hit-testing-iframe-expected.html
>===================================================================
>--- LayoutTests/fast/scrolling/ios/hit-testing-iframe-expected.html	(nonexistent)
>+++ LayoutTests/fast/scrolling/ios/hit-testing-iframe-expected.html	(working copy)
>@@ -0,0 +1,11 @@
>+<!DOCTYPE html>
>+<html
>+  <head>
>+    <title>Hit testing of iframe</title>
>+  </head>
>+  <body>
>+    <p>This test passes if you see a green rectangle.</p>
>+    <div style="position: absolute; top: 3em; width: 300px; height: 200px; background: green;">
>+    </div>
>+</body>
>+</html>
>Index: LayoutTests/fast/scrolling/ios/hit-testing-iframe.html
>===================================================================
>--- LayoutTests/fast/scrolling/ios/hit-testing-iframe.html	(nonexistent)
>+++ LayoutTests/fast/scrolling/ios/hit-testing-iframe.html	(working copy)
>@@ -0,0 +1,108 @@
>+<!DOCTYPE html>
>+<html>
>+  <head>
>+    <title>Hit testing of iframe</title>
>+    <meta name="viewport" content="width=device-width, initial-scale=1">
>+    <script src="../../../resources/basic-gestures.js"></script>
>+    <script type="text/javascript">
>+        if (window.testRunner)
>+            testRunner.waitUntilDone();
>+        if (window.internals)
>+            internals.settings.setAsyncFrameScrollingEnabled(true);
>+
>+      function frameBox(id)
>+      {
>+          return document.getElementById(id).getBoundingClientRect();
>+      }
>+
>+      function waitPromise(delay)
>+      {
>+          return new Promise((resolve) => { setTimeout(resolve, delay); });
>+      }
>+
>+      async function runTest() {
>+         if (!window.testRunner || !testRunner.runUIScript)
>+             return;
>+
>+         // This verifies whether the iframe handles clicks inside or ouside its content box.
>+         var p = frameBox("clickInContentBox");
>+         await tapAtPoint(p.left + 7, p.top + 7);
>+         p = frameBox("clickInPaddingBoxOutOfContentBox");
>+         await tapAtPoint(p.left + 3, p.top + 3);
>+
>+         // This verifies that a click event is consumed by the element on top the frame.
>+         p = frameBox("clickElementAboveFrame");
>+         await tapAtPoint(p.left + 50, p.top + 50);
>+
>+         // This verifies that a click event is consumed by an element inside the frame.
>+         p = frameBox("clickElementInsideFrame");
>+         await tapAtPoint(p.left + 7 + 10, p.top + 7 + 10);
>+
>+         // This verifies that a click event is consumed by an element inside the frame, after a user scroll.
>+         p = frameBox("clickElementInsideFrameAfterUserScroll");
>+         await touchAndDragFromPointToPoint(p.left + 7, p.top + 7, p.left + 7, p.top - 150);
>+         await liftUpAtPoint(p.left + 7, p.top - 150);
>+         await waitPromise(1500); // Wait for scrolling to stabilize and for scrollbars to disappear.
>+         await tapAtPoint(p.left + 7 + 10, p.bottom - 7);
>+         await waitPromise(100);
>+
>+         testRunner.notifyDone();
>+       }
>+
>+       var frameToLoadCount = 5;
>+       function newFrameLoaded() {
>+           frameToLoadCount--;
>+           if (frameToLoadCount == 0)
>+               runTest();
>+       }
>+    </script>
>+    <style>
>+        iframe {
>+            position: absolute;
>+            height: 90px;
>+            width: 90px;
>+            overflow: none;
>+            margin: 0;
>+            border: 0;
>+            padding: 5px;
>+        }
>+    </style>
>+  </head>
>+  <body>
>+    <p>This test passes if you see a green rectangle.</p>
>+    <div style="position: absolute; top: 3em; width: 300px; height: 200px; background: green;">
>+      <iframe id="clickInContentBox" style="left: 0px; top: 0px;" scrolling="yes" onclick="this.style.background='red'" srcdoc="
>+          <body style='margin: 0; width: 200px; height: 200px'>
>+              <div style='position: absolute; width: 100px; height: 100px; background: red;'
>+                   onclick='this.style.background=&quot;green&quot;'></div>
>+          </body>" onload="newFrameLoaded()">
>+      </iframe>
>+      <iframe id="clickInPaddingBoxOutOfContentBox" style="left: 100px; top: 0px; background: red;" onclick="this.style.background='green'" scrolling="yes" srcdoc="
>+          <body style='margin: 0; width: 200px; height: 200px'>
>+              <div style='position: absolute; width: 100px; height: 100px; background: green;'
>+                   onclick='this.style.background=&quot;red&quot;'></div>
>+          </body>" onload="newFrameLoaded()">
>+      </iframe>
>+      <iframe id="clickElementAboveFrame" style="left: 200px; top: 0px;" scrolling="yes" onclick="this.style.background='red'" srcdoc="
>+          <body style='margin: 0; width: 200px; height: 200px; background: green;'>
>+              <div style='position: absolute; width: 100px; height: 100px; background: green;'
>+                   onclick='this.style.background=&quot;red&quot;'></div>
>+          </body>" onload="newFrameLoaded()">
>+      </iframe>
>+      <div style="position: absolute; width: 50px; height: 50px; left: 225px; top: 25px; background: red" onclick="this.style.background='green';"></div>
>+      <iframe id="clickElementInsideFrame" style="left: 0px; top: 100px;" scrolling="yes" onclick="this.style.background='red'" srcdoc="
>+          <body style='margin: 0; width: 200px; height: 200px; background: green;'>
>+              <div style='position: absolute; left: 10px; top: 10px; width: 50px; height: 50px; background: red;'
>+                   onclick='this.style.background=&quot;green&quot;'></div>
>+          </body>" onload="newFrameLoaded()">
>+      </iframe>
>+      <iframe id="clickElementInsideFrameAfterUserScroll" style="left: 100px; top: 100px;" scrolling="yes" onclick="this.style.background='red'" srcdoc="
>+          <body style='margin: 0; width: 200px; height: 200px; background: green;'>
>+             <div style='position: absolute; width: 75px; height: 75px; background: red;'></div>
>+              <div style='position: absolute; left: 0px; top: 150px; width: 50px; height: 50px; background: red;'
>+                   onclick='this.style.background=&quot;green&quot;'></div>
>+          </body>" onload="newFrameLoaded()">
>+      </iframe>
>+    </div>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 192303: 356365 | 359686