Fix dragging text inside a input element nested in a draggable element.

Fix dragging text inside a input element nested in a draggable element.

I'm not entirely sure why this used to work and was broken by my
DragController refactoring. Manual inspection seemed to indicate that
it shouldn't have worked before the refactoring either, but it was
probably a combination of several different interactions that allowed
it to work. With this change, we (mostly) match what the spec says we
should do, and more importantly, we match Firefox's behavior. As a
bonus, the code is somewhat simpler as well.

BUG=298659
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161750

[dcheng, 2013-11-04 13:20:23]

Two things:

1) I'm not entirely sure why the layout test is giving me a "FAIL" in one of the
test cases. It works as expected when I try it manually in a browser... I've
opted to leave it in for now because I think it's better to have something than
nothing at all. This is something I plan on looking at again...

2) As mentioned in the patch description, I'm not really sure why this used to
work and stopped working with my refactoring change.

[tony, 2013-11-04 18:23:40]

We should try to figure out why the layout test is failing.

https://codereview.chromium.org/50433002/diff/100001/Source/core/page/DragCon...
File Source/core/page/DragController.cpp (right):

https://codereview.chromium.org/50433002/diff/100001/Source/core/page/DragCon...
Source/core/page/DragController.cpp:626: dragType = DragSourceActionSelection;
Is it OK that dragType doesn't always get set?  It looks like it's possible for
it to be uninitialized.

https://codereview.chromium.org/50433002/diff/100001/Source/core/page/EventHa...
File Source/core/page/EventHandler.cpp (left):

https://codereview.chromium.org/50433002/diff/100001/Source/core/page/EventHa...
Source/core/page/EventHandler.cpp:3281: //    cancel the drag.
Do we have a test for this case?

[dcheng, 2013-11-12 00:58:17]

I still can't figure out why the layout test differs from the actual results in
the browser... if the rest of the patch seems OK to you, I think I'd prefer to
land what we have to fix the broken behavior in Chrome and try to understand the
layout test differences later.

https://codereview.chromium.org/50433002/diff/100001/Source/core/page/DragCon...
File Source/core/page/DragController.cpp (right):

https://codereview.chromium.org/50433002/diff/100001/Source/core/page/DragCon...
Source/core/page/DragController.cpp:626: dragType = DragSourceActionSelection;
On 2013/11/04 18:23:40, tony wrote:
> Is it OK that dragType doesn't always get set?  It looks like it's possible
for
> it to be uninitialized.

It can only be uninitialized if dragSrc is set to 0. I've made the code
consistent for now in setting it though.

https://codereview.chromium.org/50433002/diff/100001/Source/core/page/EventHa...
File Source/core/page/EventHandler.cpp (left):

https://codereview.chromium.org/50433002/diff/100001/Source/core/page/EventHa...
Source/core/page/EventHandler.cpp:3281: //    cancel the drag.
On 2013/11/04 18:23:40, tony wrote:
> Do we have a test for this case?

I recall writing one but couldn't find it after looking around. Apparently it
got deleted in
http://src.chromium.org/viewvc/blink?revision=147714&view=revision. Oops.

Incidentally it revealed that my updated logic in
DragController::draggableNode() was incorrect, so I've fixed that and added a
bunch of comments. It's also rather complicated now =(

[tony, 2013-11-12 01:04:54]

LGTM.

In general, I feel like our test coverage for d&d is not great. If we could
improve it, these big changes would be less scary.

[commit-bot: I haz the power, 2013-11-12 01:17:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/50433002/200001

[commit-bot: I haz the power, 2013-11-12 03:37:22]

Change committed as 161750