single-file input elements should refuse multi-file drags

• Component: Forms
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

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.

[Rakesh]

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.

[Jon Lee]

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.

[Rakesh]

Created attachment 118960 [details]
Updated patch

Check moved to if...else chain.

[Jon Lee]

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.

[Rakesh]

Created attachment 119069 [details]
Updated Patch

Modified if check.

[Eric Seidel (no email)]

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?

[Rakesh]

(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.

[Eric Seidel (no email)]

Comment on attachment 119069 [details]
Updated Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 119069 [details]
Updated Patch

Clearing flags on attachment: 119069

Committed r106301: <http://trac.webkit.org/changeset/106301>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.