Bug 25922 - setting event.dataTransfer.dropEffect = 'none' from ondragover breaks, but ondragenter works
Summary: setting event.dataTransfer.dropEffect = 'none' from ondragover breaks, but on...
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Eric Seidel (no email)
Depends on:
Reported: 2009-05-21 03:37 PDT by Eric Seidel (no email)
Modified: 2009-07-08 10:09 PDT (History)
2 users (show)

See Also:

test case which fails (1.34 KB, text/html)
2009-05-21 03:37 PDT, Eric Seidel (no email)
no flags Details
test case with one line commented out, success! (1.34 KB, text/html)
2009-05-21 03:38 PDT, Eric Seidel (no email)
no flags Details
First pass fix (13.39 KB, patch)
2009-06-05 17:51 PDT, Eric Seidel (no email)
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-05-21 03:37:02 PDT
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.
Comment 1 Eric Seidel (no email) 2009-05-21 03:37:30 PDT
Created attachment 30537 [details]
test case which fails
Comment 2 Eric Seidel (no email) 2009-05-21 03:38:15 PDT
Created attachment 30538 [details]
test case with one line commented out, success!
Comment 3 Eric Seidel (no email) 2009-05-21 04:44:05 PDT
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.
Comment 4 Eric Seidel (no email) 2009-05-21 04:45:48 PDT
Acually, the fact that we create a new Clipboard on every event is more likely the cause of bug 25925.
Comment 5 Eric Seidel (no email) 2009-06-05 15:30:11 PDT
I have a fix.  Creating tests now.
Comment 6 Eric Seidel (no email) 2009-06-05 17:51:42 PDT
Created attachment 31021 [details]
First pass fix

 10 files changed, 176 insertions(+), 52 deletions(-)
Comment 7 Eric Seidel (no email) 2009-06-05 17:52:42 PDT
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.
Comment 8 Oliver Hunt 2009-06-23 23:00:57 PDT
Comment on attachment 31021 [details]
First pass fix

Comment 9 Eric Seidel (no email) 2009-06-24 00:08:03 PDT
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
Comment 10 Eric Seidel (no email) 2009-06-24 00:21:13 PDT
Build fix:
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/page/DragController.cpp
Committed r45065
Comment 11 Eric Seidel (no email) 2009-07-08 10:09:49 PDT
This caused bug 26787.