Summary: | [DnD] effectAllowed and dropEffect can be set to bogus values | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brian Weinstein <bweinstein> | ||||||||||||
Component: | WebCore Misc. | Assignee: | Brian Weinstein <bweinstein> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | aroben, dbates, eric, noel.gordon | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Attachments: |
|
Description
Brian Weinstein
2010-01-13 16:04:16 PST
Created attachment 46524 [details]
[PATCH] Fix
Attachment 46524 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/185881 Created attachment 46526 [details]
[PATCH] Fix without breaking Mac
Comment on attachment 46526 [details]
[PATCH] Fix without breaking Mac
r=me
Looks like this broke 5 tests on Snow Leopard: http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r53270%20(4299)/results.html 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 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. Created attachment 46591 [details]
[PATCH] Fix + Fixed Layout Tests
Comment on attachment 46591 [details]
[PATCH] Fix + Fixed Layout Tests
r=me
Landed in r53287. Created attachment 46601 [details]
Patch
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?
Created attachment 46602 [details]
Patch
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!
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? Actually, this broke all debug builds. :( (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. (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# |