[Webkit-unassigned] [Bug 191228] Plain text drag in contenteditable is always DragOperationCopy, never DragOperationMove

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=191228

Wenson Hsieh <wenson_hsieh at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andersca at apple.com

--- Comment #2 from Wenson Hsieh <wenson_hsieh at apple.com> ---
(In reply to Jonathan Hammer from comment #0)
> Created attachment 353783 [details]
> Xcode project to reproduce issue
> 
> Dragging plain text inside a contenteditable element always results in the
> text being copied (instead of the text being moved as intended). This only
> happens with legacy WebView. Please see attached Xcode project to reproduce
> the issue.
> 
> I believe the cause of the problem is as follows:
> 
> 1. DragController::startDrag calls `beginDrag`
> 2. `beginDrag` calls `cleanupAfterSystemDrag` after beginning the drag (this
> seems strange)
> 3. `cleanupAfterSystemDrag` calls `dragEnded`
> 4. `dragEnded` sets m_dragInitiator = nullptr
> 5. As the mouse is moved and the drag is updated, `dragEnteredOrUpdated` is
> called, which calls into `tryDocumentDrag`
> 6. `tryDocumentDrag` calls into `dragIsMove`
> 7. `dragIsMove` always return false because m_documentUnderMouse !=
> m_dragInitiator (because m_dragInitiator was set to nullptr after the drag
> was started)
> 
> I think the root problem is in step #2. `beginDrag` should not call
> `cleanupAfterSystemDrag`. Interestingly, that line of code is marked with
> the following comment:
> 
>     // FIXME: This shouldn't be needed.
> 
> Removing that call to `cleanupAfterSystemDrag` fixes the issue for me.

>From a quick look over the code, your assessment sounds reasonable. Since drag state is cleaned up after the dragging session ends, it seems wrong to clear it out prematurely here.

It's also not obvious to me why this is here in the first place — one guess is that it was simply copied over from DragController::doSystemDrag, which uses a different macOS API to start a drag session (i.e. -dragImage:…:, as opposed to the more modern -beginDraggingSessionWithItems:…: that DragController::beginDrag utilizes). However, in the case where -dragImage:…: is used to begin the drag, we'll spin the runloop such that we don't return to DragController::doSystemDrag until the drag session is actually finished. I'll need to double check, but I don't think this is the case for -beginDraggingSessionWithItems:…:.

If that's true, it should be safe to remove this cleanup call, as long as we ensure that drag state cleanup is performed after dragging concludes when using the DragController::beginDrag codepath.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20181103/523e5aff/attachment.html>