RESOLVED FIXED 170874
Dragging a few items over MiniBrowser has 0 in the red indicator.
https://bugs.webkit.org/show_bug.cgi?id=170874
Summary Dragging a few items over MiniBrowser has 0 in the red indicator.
Per Arne Vollan
Reported 2017-04-15 02:52:16 PDT
When MiniBrowser doesn't accept the drag, the NSDraggingInfo member numberOfValidItemsForDrop should not be set to zero.
Attachments
Patch (1.52 KB, patch)
2017-04-15 02:55 PDT, Per Arne Vollan
no flags
Patch (3.24 KB, patch)
2017-04-18 08:48 PDT, Per Arne Vollan
thorton: review+
buildbot: commit-queue-
Archive of layout-test-results from ews117 for mac-elcapitan (1.68 MB, application/zip)
2017-04-18 10:16 PDT, Build Bot
no flags
Per Arne Vollan
Comment 1 2017-04-15 02:55:03 PDT
Per Arne Vollan
Comment 2 2017-04-15 03:10:56 PDT
David Kilzer (:ddkilzer)
Comment 3 2017-04-15 06:58:40 PDT
Comment on attachment 307196 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307196&action=review r=me > Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:3620 > + if (numberOfValidItemsForDrop <= 0) Or "< 1"?
Per Arne Vollan
Comment 4 2017-04-18 08:48:02 PDT
Per Arne Vollan
Comment 5 2017-04-18 08:53:20 PDT
(In reply to David Kilzer (:ddkilzer) from comment #3) > Comment on attachment 307196 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=307196&action=review > > r=me > > > Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:3620 > > + if (numberOfValidItemsForDrop <= 0) > > Or "< 1"? Thanks for reviewing! My previous patch was incorrect, since it turns out that Page::currentDragNumberOfFilesToBeAccepted() can return zero also when WebKit accepts the drag items. This should be fixed in the latest patch.
Build Bot
Comment 6 2017-04-18 10:16:52 PDT
Comment on attachment 307387 [details] Patch Attachment 307387 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3557973 New failing tests: http/tests/inspector/network/resource-sizes-network.html
Build Bot
Comment 7 2017-04-18 10:16:53 PDT
Created attachment 307391 [details] Archive of layout-test-results from ews117 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
David Kilzer (:ddkilzer)
Comment 8 2017-04-18 12:14:36 PDT
Comment on attachment 307387 [details] Patch r=me
David Kilzer (:ddkilzer)
Comment 9 2017-04-18 15:26:08 PDT
Comment on attachment 307387 [details] Patch Oops, I can't review this as it's WebKit2.
Wenson Hsieh
Comment 10 2017-04-18 16:17:42 PDT
Comment on attachment 307387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307387&action=review r=me! I'm not a WebKit2 reviewer though, so a WebKit2 reviewer will have to r+ as well. > Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:3603 > + m_initialNumberOfValidItemsForDrop = [draggingInfo numberOfValidItemsForDrop]; Nit - I think we prefer dot notation here. > Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:3625 > if (m_page->currentDragIsOverFileInput() && numberOfValidItemsForDrop > 0) If the idea is to revert to the initial number of items when the drag operation is None, should we also apply similar treatment to the dragging formation and revert to the initial dragging formation? Maybe it's not needed for this bug though, since it's only dealing with the badge. > Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:3643 > + [draggingInfo setNumberOfValidItemsForDrop:m_initialNumberOfValidItemsForDrop]; Dot notation here too.
Tim Horton
Comment 11 2017-04-18 16:24:32 PDT
Comment on attachment 307387 [details] Patch wk2r+ with Wenson's comments addressed
Per Arne Vollan
Comment 12 2017-04-18 23:02:01 PDT
Note You need to log in before you can comment on or make changes to this bug.