Bug 33635 – [DnD] effectAllowed and dropEffect can be set to bogus values

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brian Weinstein]

event.dataTransfer.dropEffect = "bogus"

and

event.dataTransfer.effectAllowed = "bogus"

Both work, and can cause strange behavior in drag and drop behavior.

[Brian Weinstein]

Created attachment 46524 [details]
[PATCH] Fix

[Eric Seidel (no email)]

Attachment 46524 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/185881

[Brian Weinstein]

Created attachment 46526 [details]
[PATCH] Fix without breaking Mac

[Oliver Hunt]

Comment on attachment 46526 [details]
[PATCH] Fix without breaking Mac

r=me

[Eric Seidel (no email)]

Looks like this broke 5 tests on Snow Leopard:
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r53270%20(4299)/results.html

[Eric Seidel (no email)]

And one test on Qt:
http://build.webkit.org/results/Qt%20Linux%20Release/r53270%20(5993)/fast/events/bogus-dropEffect-effectAllowed-pretty-diff.html

I expect Brian is fully aware of the issues. :)  Just noticed and thought I should record them on the off chance he missed them.

[Brian Weinstein]

Comment on attachment 46526 [details]
[PATCH] Fix without breaking Mac

This broke 5 layout tests on Mac, it was landed in r53270, but rolled out in r53272. I will fix the tests, and upload new results for Mac + GTK + Qt.

[Brian Weinstein]

Created attachment 46591 [details]
[PATCH] Fix + Fixed Layout Tests

[Oliver Hunt]

Comment on attachment 46591 [details]
[PATCH] Fix + Fixed Layout Tests

r=me

[Brian Weinstein]

Landed in r53287.

[Brian Weinstein]

Created attachment 46601 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 46601 [details]
Patch

r=me

Can we make an even stronger assertion: that m_effectAllowed and m_dropEffect are one of the allowed values?

[Brian Weinstein]

Created attachment 46602 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 46602 [details]
Patch

This looks OK. r=me. But now these functions always return true. Let's get rid of the boolean return value and just return a DragOperation!

[Eric Seidel (no email)]

This broke Leopard Debug due to a bad ASSERT:
http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r53288%20(9281)/fast/events/drag-and-drop-stderr.txt

ASSERTION FAILED: op == DragOperationCopy || op == DragOperationNone || op == DragOperationLink || op == DragOperationMove
(/Volumes/Big/WebKit-BuildSlave/leopard-intel-debug/build/WebCore/dom/Clipboard.cpp:113 bool WebCore::Clipboard::destinationOperation(WebCore::DragOperation&) const)

Perhaps this was fixed in http://trac.webkit.org/changeset/53296?

[Eric Seidel (no email)]

Actually, this broke all debug builds. :(

[Brian Weinstein]

(In reply to comment #16)
> Actually, this broke all debug builds. :(

I have a fix, but am trying to figure out why dragOpFromIEOp returns DragOperationGeneric instead of DragOperationMove.

[noel gordon]

(In reply to comment #16)
> Actually, this broke all debug builds. :(

Is it still in that state?  And I noticed your reference to a related
change http://trac.webkit.org/changeset/53296, which removed some old
IE compat code.  So does 

  http://www.whatwg.org/demos/2008-sept/dnd/dnd.html

work for you on a webkit nightly?  Broken on chrome-dev for me, works
fine in Chrome4, Firefox 3.5.7/3.6, and IE6.  Other examples:

  http://html5demos.com/drag#