Bug 33635 - [DnD] effectAllowed and dropEffect can be set to bogus values
: [DnD] effectAllowed and dropEffect can be set to bogus values
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2010-01-13 16:04 PST by
Modified: 2010-02-24 07:23 PST (History)


Attachments
[PATCH] Fix (13.59 KB, patch)
2010-01-13 16:26 PST, Brian Weinstein
bweinstein: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
[PATCH] Fix without breaking Mac (13.60 KB, patch)
2010-01-13 16:34 PST, Brian Weinstein
no flags Review Patch | Details | Formatted Diff | Diff
[PATCH] Fix + Fixed Layout Tests (19.92 KB, patch)
2010-01-14 11:49 PST, Brian Weinstein
no flags Review Patch | Details | Formatted Diff | Diff
Patch (1.44 KB, patch)
2010-01-14 13:06 PST, Brian Weinstein
no flags Review Patch | Details | Formatted Diff | Diff
Patch (1.59 KB, patch)
2010-01-14 13:12 PST, Brian Weinstein
aroben: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-01-13 16:04:16 PST
event.dataTransfer.dropEffect = "bogus"

and

event.dataTransfer.effectAllowed = "bogus"

Both work, and can cause strange behavior in drag and drop behavior.
------- Comment #1 From 2010-01-13 16:26:00 PST -------
Created an attachment (id=46524) [details]
[PATCH] Fix
------- Comment #2 From 2010-01-13 16:30:37 PST -------
Attachment 46524 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/185881
------- Comment #3 From 2010-01-13 16:34:09 PST -------
Created an attachment (id=46526) [details]
[PATCH] Fix without breaking Mac
------- Comment #4 From 2010-01-13 22:56:36 PST -------
(From update of attachment 46526 [details])
r=me
------- Comment #5 From 2010-01-14 10:39:05 PST -------
Looks like this broke 5 tests on Snow Leopard:
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r53270%20(4299)/results.html
------- Comment #6 From 2010-01-14 10:40:06 PST -------
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.
------- Comment #7 From 2010-01-14 10:49:34 PST -------
(From update of attachment 46526 [details])
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.
------- Comment #8 From 2010-01-14 11:49:09 PST -------
Created an attachment (id=46591) [details]
[PATCH] Fix + Fixed Layout Tests
------- Comment #9 From 2010-01-14 12:33:49 PST -------
(From update of attachment 46591 [details])
r=me
------- Comment #10 From 2010-01-14 12:48:30 PST -------
Landed in r53287.
------- Comment #11 From 2010-01-14 13:06:31 PST -------
Created an attachment (id=46601) [details]
Patch
------- Comment #12 From 2010-01-14 13:08:17 PST -------
(From update of attachment 46601 [details])
r=me

Can we make an even stronger assertion: that m_effectAllowed and m_dropEffect are one of the allowed values?
------- Comment #13 From 2010-01-14 13:12:47 PST -------
Created an attachment (id=46602) [details]
Patch
------- Comment #14 From 2010-01-14 13:15:15 PST -------
(From update of attachment 46602 [details])
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!
------- Comment #15 From 2010-01-14 16:21:25 PST -------
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?
------- Comment #16 From 2010-01-14 16:21:53 PST -------
Actually, this broke all debug builds. :(
------- Comment #17 From 2010-01-14 16:23:52 PST -------
(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.
------- Comment #18 From 2010-02-24 07:23:46 PST -------
(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#