Bug 26699 – [Win] HTML5 Drag and drop, dragend is not fired when pressing Esc

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Erik Arvidsson]

A page that has HTML5 drag and drop support and listens to the dragend event will not get a callback if the user presses the escape key while dragging and the cursor is outside the current window.

This makes cleaning up from drag operations very hard.

This happens on Safari 4 and Chrome on Windows 7. It works fine for Safari 4 on Mac.

TC coming.

[Erik Arvidsson]

Created attachment 31818 [details]
Test case

Try dragging one of the images. Drag it outside the current browser window and press the escape key. Note that we do not get a dragend event in this case.

[Erik Arvidsson]

The TC exposes another related issue. If I press Esc over the current page I get a spurious dragstart event.

[Erik Arvidsson]

One more bug in the same spirit.

1. Drag an image out of the window.
2. Press Esc (but don't let go of the mouse button)
3. Move back over the window and the cancelled drag operation is resurrected with a new dragstart :'(

[Erik Arvidsson]

Created attachment 32104 [details]
Patch

Calls dragSourceEndedAt when fEscapePressed is true in WebDropSource::QueryContinueDrag so we can fire the dragend event correctly.

When we end the drag and drop we set m_mouseDownMayStartDrag to false so that the mouse move after the dnd has ended does not start a new dnd session.

[Adam Roben (:aroben)]

Comment on attachment 32104 [details]
Patch

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 45412)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,13 @@
> [phone]  Erik Arvidsson  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Fixes issue where escape did not cancel the drag and drop operation on windows.

You should add the URL and title of this bug to your ChangeLogs.

> Property changes on: WebCore/manual-tests/drag-escape.html
> ___________________________________________________________________
> Added: svn:executable
>    + *

Please remove the executable bit. It would also be good to set svn:eol-style to native.

> 
> Index: WebCore/page/EventHandler.cpp
> ===================================================================
> --- WebCore/page/EventHandler.cpp	(revision 45393)
> +++ WebCore/page/EventHandler.cpp	(working copy)
> @@ -2102,6 +2102,7 @@ void EventHandler::dragSourceEndedAt(con
>      }
>      freeClipboard();
>      dragState().m_dragSrc = 0;
> +    m_mouseDownMayStartDrag = false;
>  }

It's probably worth adding a comment here about why this is needed (i.e., to handle situations where the drag is cancelled while the mouse button is still down).

>  STDMETHODIMP WebDropSource::QueryContinueDrag(BOOL fEscapePressed, DWORD grfKeyState)
>  {
> -    if(fEscapePressed)
> -        return DRAGDROP_S_CANCEL;
> -
> -    if(!(grfKeyState & (MK_LBUTTON|MK_RBUTTON))) {
> -        m_dropped = true;
> +    if(fEscapePressed || !(grfKeyState & (MK_LBUTTON|MK_RBUTTON))) {

Please add a space after "if" while you're modifying this line.

> +        m_dropped = !fEscapePressed;
>          if (Page* page = m_webView->page())
>              if (Frame* frame = page->mainFrame()) 
>                  //FIXME: We need to figure out how to find out what actually happened in the drag <rdar://problem/5015961>
>                  frame->eventHandler()->dragSourceEndedAt(generateMouseEvent(m_webView.get(), false), DragOperationCopy);

It doesn't seem right to pass DragOperationCopy if Escape was pressed.

> -        return DRAGDROP_S_DROP;
> +		return fEscapePressed? DRAGDROP_S_CANCEL : DRAGDROP_S_DROP;

Tabs! Please use 4-space indents.

Thanks for looking at this!

[Eric Seidel (no email)]

> Property changes on: WebCore/manual-tests/drag-escape.html
> ___________________________________________________________________
> Added: svn:executable
>    + *

Please remove the executable bit. It would also be good to set svn:eol-style to
native.

We need some sort of script to remove these for you.  Windows SVN adds them *all the time*.  Windows SVN also gets the mime types of added images wrong too.

I've filed https://bugs.webkit.org/show_bug.cgi?id=26895 to track creation of such a script.

[Erik Arvidsson]

Created attachment 32133 [details]
Fixed Adam's comments

[Adam Roben (:aroben)]

Comment on attachment 32133 [details]
Fixed Adam's comments

r=me

[Eric Seidel (no email)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	A	WebCore/manual-tests/drag-escape.html
	M	WebCore/page/EventHandler.cpp
	M	WebKit/win/ChangeLog
	M	WebKit/win/WebDropSource.cpp
Committed r45515
	M	WebKit/win/ChangeLog
	M	WebKit/win/WebDropSource.cpp
	M	WebCore/ChangeLog
	M	WebCore/page/EventHandler.cpp
	A	WebCore/manual-tests/drag-escape.html
r45515 = e9a38090ac268a71d0631db21ea0212e575d7316 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/45515