Summary: | Expand DragController to provide more information about the dragging session | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jon Lee <jonlee> | ||||||||||||||||||
Component: | Forms | Assignee: | Jon Lee <jonlee> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Normal | CC: | darin, dglazkov, gustavo.noronha, gustavo, sam, webkit-bug-importer, webkit.review.bot, xan.lopez | ||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||
Hardware: | Mac | ||||||||||||||||||||
OS: | OS X 10.7 | ||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||
Bug Blocks: | 13897, 71325 | ||||||||||||||||||||
Attachments: |
|
Description
Jon Lee
2011-11-01 14:32:20 PDT
Created attachment 113233 [details]
Patch
Comment on attachment 113233 [details] Patch Attachment 113233 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10258070 Comment on attachment 113233 [details] Patch Attachment 113233 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10259068 Created attachment 113247 [details]
Patch
Comment on attachment 113247 [details] Patch Attachment 113247 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10148074 Created attachment 113254 [details]
Patch
Comment on attachment 113254 [details] Patch Attachment 113254 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10175095 Comment on attachment 113254 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113254&action=review > Source/WebCore/page/DraggingInfo.h:33 > +struct DraggingInfo { I’m usually not such a big fan of “info” in a class name, since everything in a computer is info or data, and that suffix doesn’t add much. > Source/WebCore/page/DraggingInfo.h:39 > + bool mouseOverFileInput; I think a verb in this phrase would help. Like “mouseIsOverFileInput”. Comment on attachment 113254 [details] Patch Attachment 113254 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10259099 (In reply to comment #9) > (From update of attachment 113254 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=113254&action=review > > > Source/WebCore/page/DraggingInfo.h:33 > > +struct DraggingInfo { > > I’m usually not such a big fan of “info” in a class name, since everything in a computer is info or data, and that suffix doesn’t add much. How about DragSession? > > > Source/WebCore/page/DraggingInfo.h:39 > > + bool mouseOverFileInput; > > I think a verb in this phrase would help. Like “mouseIsOverFileInput”. Done. Created attachment 113267 [details]
Patch
Created attachment 113269 [details]
Patch
Comment on attachment 113269 [details] Patch Attachment 113269 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10258127 New failing tests: editing/pasteboard/file-input-files-access.html Comment on attachment 113269 [details] Patch Attachment 113269 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10259202 Created attachment 113354 [details]
Patch
This latest patch changes a layout test. The expectation now is that dragging a file over a disabled file input does nothing-- previously it would load the file being dragged into the web view. Created attachment 113358 [details]
Patch
Created attachment 113376 [details]
Patch
Comment on attachment 113376 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113376&action=review Not sure I love the name DragSession, but I guess it’s OK. > Source/WebCore/page/DragController.cpp:353 > + } else > + // We are not over a file input element. The dragged item(s) will only > + // be loaded into the view the number of dragged items is 1. > + dragSession.numberOfItemsToBeAccepted = numberOfFiles != 1 ? 0 : 1; WebKit coding style says you should use braces here. It’s the number of lines that determines the braces, not the number of statements. > Source/WebCore/page/DragSession.h:37 > + DragSession() > + : operation(DragOperationNone) > + , mouseIsOverFileInput(false) > + , numberOfItemsToBeAccepted(0) { } This is not the formatting we use in WebKit code. That would be: DragSession() : operation(DragOperationNone) , mouseIsOverFileInput(false) , numberOfItemsToBeAccepted(0) { } Also, we normally put the constructor after the data members in a separate paragraph when it’s a struct rather than a class. > Source/WebKit2/UIProcess/WebPageProxy.cpp:186 > + , m_currentDragSession() You should be able to delete this line of code entirely. The default constructor is called if you don’t explicitly initialize the data member. Thanks, I will check the patch in with the changes you mentioned. Committed r99108: <http://trac.webkit.org/changeset/99108> |