Bug 170874 - Dragging a few items over MiniBrowser has 0 in the red indicator.
Summary: Dragging a few items over MiniBrowser has 0 in the red indicator.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-04-15 02:52 PDT by Per Arne Vollan
Modified: 2017-04-18 23:02 PDT (History)
11 users (show)

See Also:


Attachments
Patch (1.52 KB, patch)
2017-04-15 02:55 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (3.24 KB, patch)
2017-04-18 08:48 PDT, Per Arne Vollan
thorton: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
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 Details

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2017-04-15 02:52:16 PDT
When MiniBrowser doesn't accept the drag, the NSDraggingInfo member numberOfValidItemsForDrop should not be set to zero.
Comment 1 Per Arne Vollan 2017-04-15 02:55:03 PDT
Created attachment 307196 [details]
Patch
Comment 2 Per Arne Vollan 2017-04-15 03:10:56 PDT
rdar://problem/11141060
Comment 3 David Kilzer (:ddkilzer) 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"?
Comment 4 Per Arne Vollan 2017-04-18 08:48:02 PDT
Created attachment 307387 [details]
Patch
Comment 5 Per Arne Vollan 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.
Comment 6 Build Bot 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
Comment 7 Build Bot 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
Comment 8 David Kilzer (:ddkilzer) 2017-04-18 12:14:36 PDT
Comment on attachment 307387 [details]
Patch

r=me
Comment 9 David Kilzer (:ddkilzer) 2017-04-18 15:26:08 PDT
Comment on attachment 307387 [details]
Patch

Oops, I can't review this as it's WebKit2.
Comment 10 Wenson Hsieh 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.
Comment 11 Tim Horton 2017-04-18 16:24:32 PDT
Comment on attachment 307387 [details]
Patch

wk2r+ with Wenson's comments addressed
Comment 12 Per Arne Vollan 2017-04-18 23:02:01 PDT
Committed <https://trac.webkit.org/changeset/215504/webkit>