[macOS] [WK2] Drag location is computed incorrectly when dragging content from subframes
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 20 Jan 2018 07:24:50 +0000 (07:24 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 20 Jan 2018 07:24:50 +0000 (07:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181896
<rdar://problem/35479043>

Reviewed by Tim Horton.

In r218837, I packaged most of the information needed to start a drag into DragItem, which is propagated to the client layer
via the startDrag codepath. However, this introduced a bug in computing the event position and drag location in window
coordinates. Consider the case where we're determining the drag image offset for a dragged element in a subframe:

Before the patch, the drag location (which starts out in the subframe's content coordinates) would be converted to root view
coordinates, which would then be converted to mainframe content coordinates, which would then be converted to window coordinates
using the mainframe's view. After the patch, we carry out the same math until the last step, where we erroneously use the
_subframe's_ view to convert to window coordinates from content coordinates. This results in the position of the iframe relative
to the mainframe being accounted for twice.

To fix this, we simply use the main frame's view to convert from mainframe content coordinates to window coordinates while
computing the drag location. As for the event position in window coordinates, this is currently unused by any codepath in WebKit,
so we can just remove it altogether.

Since this bug only affects drag and drop in the macOS WebKit2 port, there's currently no way to test this. I'll be using
<https://bugs.webkit.org/show_bug.cgi?id=181898> to track adding test support for drag and drop on macOS WebKit2. Manually tested
dragging in both WebKit1 and WebKit2 on macOS. dragLocationInWindowCoordinates isn't used at all for iOS drag and drop.

* page/DragController.cpp:
(WebCore::DragController::doSystemDrag):
* platform/DragItem.h:
(WebCore::DragItem::encode const):
(WebCore::DragItem::decode):

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

Source/WebCore/ChangeLog
Source/WebCore/page/DragController.cpp
Source/WebCore/platform/DragItem.h

index 46b6dcd..0bfca57 100644 (file)
@@ -1,3 +1,35 @@
+2018-01-19  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [macOS] [WK2] Drag location is computed incorrectly when dragging content from subframes
+        https://bugs.webkit.org/show_bug.cgi?id=181896
+        <rdar://problem/35479043>
+
+        Reviewed by Tim Horton.
+
+        In r218837, I packaged most of the information needed to start a drag into DragItem, which is propagated to the client layer
+        via the startDrag codepath. However, this introduced a bug in computing the event position and drag location in window
+        coordinates. Consider the case where we're determining the drag image offset for a dragged element in a subframe:
+
+        Before the patch, the drag location (which starts out in the subframe's content coordinates) would be converted to root view
+        coordinates, which would then be converted to mainframe content coordinates, which would then be converted to window coordinates
+        using the mainframe's view. After the patch, we carry out the same math until the last step, where we erroneously use the
+        _subframe's_ view to convert to window coordinates from content coordinates. This results in the position of the iframe relative
+        to the mainframe being accounted for twice.
+
+        To fix this, we simply use the main frame's view to convert from mainframe content coordinates to window coordinates while
+        computing the drag location. As for the event position in window coordinates, this is currently unused by any codepath in WebKit,
+        so we can just remove it altogether.
+
+        Since this bug only affects drag and drop in the macOS WebKit2 port, there's currently no way to test this. I'll be using
+        <https://bugs.webkit.org/show_bug.cgi?id=181898> to track adding test support for drag and drop on macOS WebKit2. Manually tested
+        dragging in both WebKit1 and WebKit2 on macOS. dragLocationInWindowCoordinates isn't used at all for iOS drag and drop.
+
+        * page/DragController.cpp:
+        (WebCore::DragController::doSystemDrag):
+        * platform/DragItem.h:
+        (WebCore::DragItem::encode const):
+        (WebCore::DragItem::decode):
+
 2018-01-19  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r227235.
index 4eefbab..ac5225b 100644 (file)
@@ -1227,8 +1227,7 @@ void DragController::doSystemDrag(DragImage image, const IntPoint& dragLoc, cons
     auto dragLocationInRootViewCoordinates = frame.view()->contentsToRootView(dragLoc);
     item.eventPositionInContentCoordinates = viewProtector->rootViewToContents(eventPositionInRootViewCoordinates);
     item.dragLocationInContentCoordinates = viewProtector->rootViewToContents(dragLocationInRootViewCoordinates);
-    item.eventPositionInWindowCoordinates = frame.view()->contentsToWindow(item.eventPositionInContentCoordinates);
-    item.dragLocationInWindowCoordinates = frame.view()->contentsToWindow(item.dragLocationInContentCoordinates);
+    item.dragLocationInWindowCoordinates = viewProtector->contentsToWindow(item.dragLocationInContentCoordinates);
     if (auto element = state.source) {
         auto dataTransferImageElement = state.dataTransfer->dragImageElement();
         if (state.type == DragSourceActionDHTML) {
index 84405f6..a02a3ea 100644 (file)
@@ -43,7 +43,6 @@ struct DragItem final {
     DragSourceAction sourceAction { DragSourceActionNone };
     IntPoint eventPositionInContentCoordinates;
     IntPoint dragLocationInContentCoordinates;
-    IntPoint eventPositionInWindowCoordinates;
     IntPoint dragLocationInWindowCoordinates;
     String title;
     URL url;
@@ -62,7 +61,7 @@ void DragItem::encode(Encoder& encoder) const
     // FIXME(173815): We should encode and decode PasteboardWriterData and platform drag image data
     // here too, as part of moving off of the legacy dragging codepath.
     encoder.encodeEnum(sourceAction);
-    encoder << imageAnchorPoint << eventPositionInContentCoordinates << dragLocationInContentCoordinates << eventPositionInWindowCoordinates << dragLocationInWindowCoordinates << title << url << dragPreviewFrameInRootViewCoordinates;
+    encoder << imageAnchorPoint << eventPositionInContentCoordinates << dragLocationInContentCoordinates << dragLocationInWindowCoordinates << title << url << dragPreviewFrameInRootViewCoordinates;
     bool hasIndicatorData = image.hasIndicatorData();
     encoder << hasIndicatorData;
     if (hasIndicatorData)
@@ -81,8 +80,6 @@ bool DragItem::decode(Decoder& decoder, DragItem& result)
         return false;
     if (!decoder.decode(result.dragLocationInContentCoordinates))
         return false;
-    if (!decoder.decode(result.eventPositionInWindowCoordinates))
-        return false;
     if (!decoder.decode(result.dragLocationInWindowCoordinates))
         return false;
     if (!decoder.decode(result.title))