WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
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
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Per Arne Vollan
Comment 1
2017-04-15 02:55:03 PDT
Created
attachment 307196
[details]
Patch
Per Arne Vollan
Comment 2
2017-04-15 03:10:56 PDT
rdar://problem/11141060
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
Created
attachment 307387
[details]
Patch
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
Committed <
https://trac.webkit.org/changeset/215504/webkit
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug