Attachment 356365 Details for Bug 192303 – Patch (applies on top of bug 173833)

[patch] Patch (applies on top of bug 173833)

0002-Bug-192303-iOS-Handle-hit-testing-for-subframes.patch (text/plain), 7.47 KB, created by Frédéric Wang (:fredw) on 2018-12-03 00:54:48 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Frédéric Wang (:fredw)

Created: 2018-12-03 00:54:48 PST

Size: 7.47 KB

patch

obsolete

>From 575f4823a102f2463413024c32fb1e8f0f5b3410 Mon Sep 17 00:00:00 2001
>From: Frederic Wang <fwang@igalia.com>
>Date: Mon, 3 Dec 2018 09:31:20 +0100
>Subject: [PATCH xserver 2/3] Bug 192303 - [iOS] Handle hit testing for
> subframes
>
>---
> LayoutTests/ChangeLog                         | 10 +++++++
> .../scrolling/ios/hit-testing-iframe.html     | 16 +++++++++++-
> Source/WebCore/ChangeLog                      | 26 +++++++++++++++++++
> Source/WebCore/page/FrameView.cpp             |  5 ++++
> Source/WebCore/page/FrameView.h               |  2 ++
> Source/WebCore/platform/ScrollView.cpp        |  4 +--
> Source/WebCore/platform/ScrollView.h          |  2 ++
> 7 files changed, 62 insertions(+), 3 deletions(-)
>
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 2ac59b2bd78..58c3b4f126b 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-12-03  Frederic Wang  <fwang@igalia.com>
>+
>+        [iOS] Handle hit testing for subframes
>+        https://bugs.webkit.org/show_bug.cgi?id=192303
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/scrolling/ios/hit-testing-iframe.html: Add a test to verify that a click event is
>+        consumed by an element inside a subframe, after a user scroll.
>+
> 2018-11-20  Frederic Wang  <fwang@igalia.com>
> 
>         [iOS] Adjust layer hierarchy to handle frame scrolling
>diff --git a/LayoutTests/fast/scrolling/ios/hit-testing-iframe.html b/LayoutTests/fast/scrolling/ios/hit-testing-iframe.html
>index a2825efe251..aa457020d04 100644
>--- a/LayoutTests/fast/scrolling/ios/hit-testing-iframe.html
>+++ b/LayoutTests/fast/scrolling/ios/hit-testing-iframe.html
>@@ -43,10 +43,17 @@
>          p = frameBox("clickElementInsideFrameAfterProgrammaticScroll");
>          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);
>+
>          testRunner.notifyDone();
>        }
> 
>-       var frameToLoadCount = 5;
>+       var frameToLoadCount = 6;
>        function newFrameLoaded() {
>            frameToLoadCount--;
>            if (frameToLoadCount == 0)
>@@ -100,6 +107,13 @@
>                    onclick='this.style.background=&quot;green&quot;'></div>
>           </body>" onload="newFrameLoaded()">
>       </iframe>
>+      <iframe id="clickElementInsideFrameAfterUserScroll" style="left: 200px; 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>
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c0a8dbf4e20..e2c2c8e153d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-12-03  Frederic Wang  <fwang@igalia.com>
>+
>+        [iOS] Handle hit testing for subframes
>+        https://bugs.webkit.org/show_bug.cgi?id=192303
>+
>+        Hit testing for subframes on iOS is essentially implemented in bug 173833. This commit fixes
>+        a hit testing bug when the subframe has been scrolled by user interaction.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: fast/scrolling/ios/hit-testing-iframe.html
>+
>+        * platform/ScrollView.cpp:
>+        (WebCore::ScrollView::windowToContents const): Use coordinateChangeShouldIgnoreScrollPosition()
>+        instead of delegatesScrolling() so that WebCore::documentPointForWindowPoint takes into account the
>+        scroll position of subframes during hit testing.
>+        (WebCore::ScrollView::contentsToWindow const): Same change for consistency.
>+        * platform/ScrollView.h:
>+        (WebCore::ScrollView::coordinateChangeShouldIgnoreScrollPosition const): For ScrollView make
>+        windowToContents behave the same as the old version.
>+        * page/FrameView.cpp:
>+        (WebCore::FrameView::coordinateChangeShouldIgnoreScrollPosition const): Now also take into
>+        account scroll position of subframes in WebKit2.
>+        * page/FrameView.h: Override coordinateChangeShouldIgnoreScrollPosition to make it more
>+        restrictive.
>+
> 2018-02-16  Frederic Wang  <fwang@igalia.com>
> 
>         [iOS] Adjust layer hierarchy to handle frame scrolling
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index 778b4c56eeb..d3a432fff90 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -1396,6 +1396,11 @@ void FrameView::adjustMediaTypeForPrinting(bool printing)
>     }
> }
> 
>+bool FrameView::coordinateChangeShouldIgnoreScrollPosition() const
>+{
>+    return delegatesScrolling() && (platformWidget() || frame().isMainFrame());
>+}
>+
> bool FrameView::useSlowRepaints(bool considerOverlap) const
> {
>     bool mustBeSlow = hasSlowRepaintObjects() || (platformWidget() && hasViewportConstrainedObjects());
>diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
>index e44977670e6..7a81e80736d 100644
>--- a/Source/WebCore/page/FrameView.h
>+++ b/Source/WebCore/page/FrameView.h
>@@ -679,6 +679,8 @@ private:
> 
>     bool isFrameView() const final { return true; }
> 
>+    bool coordinateChangeShouldIgnoreScrollPosition() const final;
>+
>     friend class RenderWidget;
>     bool useSlowRepaints(bool considerOverlap = true) const;
>     bool useSlowRepaintsIfNotOverlapped() const;
>diff --git a/Source/WebCore/platform/ScrollView.cpp b/Source/WebCore/platform/ScrollView.cpp
>index 98d4ecae887..341865a151e 100644
>--- a/Source/WebCore/platform/ScrollView.cpp
>+++ b/Source/WebCore/platform/ScrollView.cpp
>@@ -922,7 +922,7 @@ IntRect ScrollView::contentsToRootView(const IntRect& contentsRect) const
> 
> IntPoint ScrollView::windowToContents(const IntPoint& windowPoint) const
> {
>-    if (delegatesScrolling())
>+    if (coordinateChangeShouldIgnoreScrollPosition())
>         return convertFromContainingWindow(windowPoint);
> 
>     return viewToContents(convertFromContainingWindow(windowPoint));
>@@ -930,7 +930,7 @@ IntPoint ScrollView::windowToContents(const IntPoint& windowPoint) const
> 
> IntPoint ScrollView::contentsToWindow(const IntPoint& contentsPoint) const
> {
>-    if (delegatesScrolling())
>+    if (coordinateChangeShouldIgnoreScrollPosition())
>         return convertToContainingWindow(contentsPoint);
> 
>     return convertToContainingWindow(contentsToView(contentsPoint));
>diff --git a/Source/WebCore/platform/ScrollView.h b/Source/WebCore/platform/ScrollView.h
>index a8454621972..0d47a950313 100644
>--- a/Source/WebCore/platform/ScrollView.h
>+++ b/Source/WebCore/platform/ScrollView.h
>@@ -381,6 +381,8 @@ public:
> protected:
>     ScrollView();
> 
>+    virtual bool coordinateChangeShouldIgnoreScrollPosition() const { return delegatesScrolling(); }
>+
>     virtual void repaintContentRectangle(const IntRect&);
>     virtual void paintContents(GraphicsContext&, const IntRect& damageRect, SecurityOriginPaintPolicy = SecurityOriginPaintPolicy::AnyOrigin) = 0;
> 
>-- 
>2.19.1
>

Flags:

simon.fraser: review+
simon.fraser: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 192303: 356365 | 359686