Gracefully handle drag cancellation during drag start.

Gracefully handle drag cancellation during drag start.

Should the dispatching of a dragstart cause the drag to be reset, take
that into account when trying to handle follow-on drag operations.

The test is imported from https://bugs.webkit.org/show_bug.cgi?id=115296

R=dcheng@chromium.org
BUG=405442
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180646

[sof, 2014-08-20 09:36:09]

Please take a look.

[dcheng, 2014-08-20 10:56:24]

https://codereview.chromium.org/488923003/diff/1/Source/core/page/EventHandle...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/488923003/diff/1/Source/core/page/EventHandle...
Source/core/page/EventHandler.cpp:3266: &&
!m_frame->selection().isInPasswordField() && dragState().m_dragSrc;
I wonder if it's worth adding a comment about why we need to check if there's
still a drag source after dispatching the event. The drag code is already fairly
unintuitive as it is, and this can't "normally" happen.

[dcheng, 2014-08-20 10:57:11]

LGTM btw (not sure why my last message didn't include this).

[sof, 2014-08-20 11:06:43]

https://codereview.chromium.org/488923003/diff/1/Source/core/page/EventHandle...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/488923003/diff/1/Source/core/page/EventHandle...
Source/core/page/EventHandler.cpp:3266: &&
!m_frame->selection().isInPasswordField() && dragState().m_dragSrc;
On 2014/08/20 10:56:24, dcheng (OOO) wrote:
> I wonder if it's worth adding a comment about why we need to check if there's
> still a drag source after dispatching the event. The drag code is already
fairly
> unintuitive as it is, and this can't "normally" happen.

Good idea, added one - does it clarify ?

[dcheng, 2014-08-20 11:16:07]

https://codereview.chromium.org/488923003/diff/20001/Source/core/page/EventHa...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/488923003/diff/20001/Source/core/page/EventHa...
Source/core/page/EventHandler.cpp:3266: // If dispatching dragstart brings about
a mouse down from deep within its
I might reword this to include the specific circumstance. It's quite unusual
this would occur since normal web content can't synthesize "real" mouse events.
Maybe something like:
"Cancel dragging if another mouse down event arrives during drag start
processing, since the new mouse down will reset a lot of dragging state. This
can happen if the user sets a breakpoint in the dragstart handler and then
clicks on the suspended page."

[sof, 2014-08-20 11:30:13]

On 2014/08/20 11:16:07, dcheng (OOO) wrote:
>
https://codereview.chromium.org/488923003/diff/20001/Source/core/page/EventHa...
> File Source/core/page/EventHandler.cpp (right):
> 
>
https://codereview.chromium.org/488923003/diff/20001/Source/core/page/EventHa...
> Source/core/page/EventHandler.cpp:3266: // If dispatching dragstart brings
about
> a mouse down from deep within its
> I might reword this to include the specific circumstance. It's quite unusual
> this would occur since normal web content can't synthesize "real" mouse
events.
> Maybe something like:
> "Cancel dragging if another mouse down event arrives during drag start
> processing, since the new mouse down will reset a lot of dragging state. This
> can happen if the user sets a breakpoint in the dragstart handler and then
> clicks on the suspended page."

Thanks, expanded comment to include when it might happen.

[sof, 2014-08-20 11:30:20]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-08-20 11:31:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/488923003/40001

[commit-bot: I haz the power, 2014-08-20 12:28:40]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/23628)

[commit-bot: I haz the power, 2014-08-20 13:52:18]

Committed patchset #3 (40001) as 180646