Bug 25913 - single-file input elements should refuse multi-file drags
Summary: single-file input elements should refuse multi-file drags
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Rakesh
URL:
Keywords:
Depends on: 25879 25924
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-21 00:49 PDT by Eric Seidel (no email)
Modified: 2012-01-30 16:36 PST (History)
7 users (show)

See Also:


Attachments
Proposed patch (5.53 KB, patch)
2011-12-12 03:10 PST, Rakesh
no flags Details | Formatted Diff | Diff
Updated patch (5.54 KB, patch)
2011-12-12 23:16 PST, Rakesh
no flags Details | Formatted Diff | Diff
Updated Patch (5.02 KB, patch)
2011-12-13 12:54 PST, Rakesh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-05-21 00:49:42 PDT
single-file input elements should refuse multi-file drags

The current behavior of only accepting the first file is confusing to the user (I would argue).  We should match whatever behavior matches the operating system and update the expected results in:
LayoutTests/editing/pasteboard/resources/file-input-files-access.js
appropriately.
Comment 1 Eric Seidel (no email) 2009-05-21 04:05:47 PDT
The check which needs to change here is in:

bool DragController::canProcessDrag(DragData* dragData) 

    if (dragData->containsFiles() && asFileInput(result.innerNonSharedNode()))
        return true;

probably we should pass the DragData off to the HTMLInputElement and ask it if it can handle the drag.  DragData will need to be extended to be able to return the number of files on the clipboard for HTMLInputElement to be able to make this decision.
Comment 2 Rakesh 2011-12-12 03:10:04 PST
Created attachment 118761 [details]
Proposed patch

Refuse the multiple file drags on single-file input element. Setting the dragSession.operation as DragOperationNone when input element is multiple and number of files are more than 1. I think DragController::canProcessDrag may not be the right place to refuse the drag as if canProcessDrag fails then tryDocumentDrag will fail which may lead to the loading of the file in the browser.
Comment 3 Jon Lee 2011-12-12 08:41:13 PST
Comment on attachment 118761 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=118761&action=review

> Source/WebCore/page/DragController.cpp:358
> +            if (!dragSession.numberOfItemsToBeAccepted || (!m_fileInputElementUnderMouse->multiple() && (numberOfFiles > 1)))

Although this is correct, I think it would be better to move this check into the if..else chain starting in line 351 (as penultimate "else if" check). This way the check for setting the operation to None remains simple and comprehensible.
Comment 4 Rakesh 2011-12-12 23:16:09 PST
Created attachment 118960 [details]
Updated patch

Check moved to if...else chain.
Comment 5 Jon Lee 2011-12-13 09:35:58 PST
Comment on attachment 118960 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=118960&action=review

> Source/WebCore/page/DragController.cpp:355
> +            else if (!m_fileInputElementUnderMouse->multiple() && (numberOfFiles > 1))

The first check is unnecessary. By this point we already know that the file input is not multiple.
Comment 6 Rakesh 2011-12-13 12:54:51 PST
Created attachment 119069 [details]
Updated Patch

Modified if check.
Comment 7 Eric Seidel (no email) 2011-12-14 12:33:37 PST
Comment on attachment 119069 [details]
Updated Patch

Seems good to me.  I believe Oliver has also worked in drag and drop more recently than I.

Did you test against any operating systems?
Comment 8 Rakesh 2011-12-28 06:20:39 PST
(In reply to comment #7)
> (From update of attachment 119069 [details])
> Seems good to me.  I believe Oliver has also worked in drag and drop more recently than I.
> 
> Did you test against any operating systems?

This fix works fine on MAC and GTK. On Windows DragData::numberOfFiles() returns 0, so this fix does not have any effect on Windows.
Comment 9 Eric Seidel (no email) 2012-01-30 16:19:52 PST
Comment on attachment 119069 [details]
Updated Patch

LGTM.
Comment 10 WebKit Review Bot 2012-01-30 16:35:58 PST
Comment on attachment 119069 [details]
Updated Patch

Clearing flags on attachment: 119069

Committed r106301: <http://trac.webkit.org/changeset/106301>
Comment 11 WebKit Review Bot 2012-01-30 16:36:04 PST
All reviewed patches have been landed.  Closing bug.