Bug 25922 – setting event.dataTransfer.dropEffect = 'none' from ondragover breaks, but ondragenter works

• Component: HTML Editing
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

setting event.dataTransfer.dropEffect from ondragover break, but ondragenter works

See attached test case.

The goal of the attached test case is to make the page *not* navigate when a file is dropped on it (as is the default browser behavior.  The page works correctly if you only set event.dataTransfer.dropEffect from within ondragenter.  If you also set dropEffect from ondragover, then the page breaks!  Seems like a bug.

[Eric Seidel (no email)]

Created attachment 30537 [details]
test case which fails

[Eric Seidel (no email)]

Created attachment 30538 [details]
test case with one line commented out, success!

[Eric Seidel (no email)]

I think this may be a combination of two factors.
1.  We create a new Clipboard for every event:
DragOperation DragController::tryDHTMLDrag(DragData* dragData)
    RefPtr<Clipboard> clipboard = dragData->createClipboard(policy);
2.  We use DragOperationNone to indicate that a document won't handle a drag:
DragOperation DragController::dragEnteredOrUpdated(DragData* dragData)
        operation = tryDocumentDrag(dragData, m_dragDestinationAction);
        if (operation == DragOperationNone && (m_dragDestinationAction & DragDestinationActionLoad))
            return operationForLoad(dragData);

tryDocumentDrag needs to return another parameter, which indicates if the document handled the drag or not.

And I don't think we should be creating a new clipboard on every event.

[Eric Seidel (no email)]

Acually, the fact that we create a new Clipboard on every event is more likely the cause of bug 25925.

[Eric Seidel (no email)]

I have a fix.  Creating tests now.

[Eric Seidel (no email)]

Created attachment 31021 [details]
First pass fix

 10 files changed, 176 insertions(+), 52 deletions(-)

[Eric Seidel (no email)]

Comment on attachment 31021 [details]
First pass fix

Btw, I tested this before my fix to make sure that prevent-drag-to-navigate.html failed as expected before the fix.

[Oliver Hunt]

Comment on attachment 31021 [details]
First pass fix

r=me

[Eric Seidel (no email)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/events/drag-to-navigate-expected.txt
	A	LayoutTests/fast/events/drag-to-navigate.html
	A	LayoutTests/fast/events/prevent-drag-to-navigate-expected.txt
	A	LayoutTests/fast/events/prevent-drag-to-navigate.html
	A	LayoutTests/fast/events/resources/file-for-drag-to-navigate.html
	A	LayoutTests/fast/events/resources/file-for-prevent-drag-to-navigate.html
	M	WebCore/ChangeLog
	M	WebCore/page/DragController.cpp
	M	WebCore/page/DragController.h
Committed r45064
http://trac.webkit.org/changeset/45064

[Eric Seidel (no email)]

Build fix:
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/page/DragController.cpp
Committed r45065
http://trac.webkit.org/changeset/45065

[Eric Seidel (no email)]

This caused bug 26787.