Bug 25916 – WebKit should expose files from the pasteboard in drop events

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Eric Seidel (no email)]

WebKit should expose files from the pasteboard in drop events

This would make it possible for applications like gmail to implement
drag and drop of attachments onto email messages.

We will need to follow the HTML5 drag and drop security model:
http://www.w3.org/TR/html5/editing.html#security-risks-in-the-drag-and-drop-model

Internet Explorer shows historical documentation of supporting
getData('File') as a way of exposing files on the pasteboard.  The current version of their docs:
http://msdn.microsoft.com/en-us/library/ms537658(VS.85).aspx
has removed this reference (likely IE never implemented it)
I have a printed copy on my desk from 2008 describing getData('File') in IE.

Mozilla has equivalent file access:
event.dataTransfer.mozGetDataAt("application/x-moz-file", 0);
which also does not return a typed value.
https://developer.mozilla.org/En/DragDrop/Recommended_Drag_Types#Dragging_Files

[Eric Seidel (no email)]

Created attachment 30530 [details]
Expose files in the clipboard in ondrop events

 12 files changed, 314 insertions(+), 3 deletions(-)

[Maciej Stachowiak]

Comment on attachment 30530 [details]
Expose files in the clipboard in ondrop events

Conceptually, I think this is awesome. It would be great to have this feature. And the Mac implementation looks fine. r- for the following reasons:

1) Implementations for other platforms are missing, in fact it looks like it will break all other builds as-is.

2) I think we should bring this up on the whatwg list ASAP, maybe even before comitting anything, to make sure we end up aligned with other browsers on this.

[Eric Seidel (no email)]

Ok. Happy to update this to not break other builds.  I also of course will watch the bots when it lands and fix any errors I missed.  Since WebKit doesn't have try-servers I wont know until I land if this will break things like wx or gtk.

[Eric Seidel (no email)]

Ian is aware of the intent to add this.  I don't personally participate in the what-wg list, but I'm sure others concerned about this issue do.

[Eric Seidel (no email)]

Created attachment 31064 [details]
Updated patch to not break other builds.

 22 files changed, 381 insertions(+), 9 deletions(-)

[Adam Barth]

What's the status of this bug?  Do we need to talk more with whatwg or should we review this patch?

[Adam Barth]

(In reply to comment #6)
> What's the status of this bug?  Do we need to talk more with whatwg or should
> we review this patch?

For the record, I think we should go ahead with this patch.  :)

[Eric Seidel (no email)]

Btw, HTMLInputElement.files was added in http://trac.webkit.org/changeset/34702.  input.files does not seem to be covered by any spec:
http://www.w3.org/TR/file-upload/
http://www.w3.org/TR/html5/forms.html#the-input-element

But I'm glad we have it all the same. :)

[Eric Seidel (no email)]

Actually  input.files is a moz invention:
https://developer.mozilla.org/En/NsIDOMFileList

[Eric Seidel (no email)]

Adam has kindly posted to the WhatWG:
http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2009-June/020302.html

Still looking for content review... (Although my sense from before was that the patch was fine it just was gonna break builds).

[Maciej Stachowiak]

To be clear, we don't need a full official draft spec to land this. I brought it up mainly to make sure we didn't skip that step, and that developers of other browser engines had a chance to bring up concerns with the specifics of the API. Seems like that was done sufficiently.

What's the behavior on platforms where the API is not implemented? Will 0 get safely turned into null? The code looks ok to me on quick overview but since I'm on vacation it's probably best to have someone give the official stamp of approval. I would suggest Sam, Adele or Oliver as a good reviewer.

[Eric Seidel (no email)]

Yes, 0 is safely turned into null. (This is only ever called from the wrapping bindings.)  The Mac code used to return 0 on permission failure until I changed it to return an empty list.  (I can change all the others to return an empty list instead of 0 as well.)

[Maciej Stachowiak]

Comment on attachment 31064 [details]
Updated patch to not break other builds.

I'm going to say r=me with the following caveats:

1) Please file follow-up bugs for platforms that don't implement the back end for this yet, at least for ports that have a public buildbot.

2) I think for platforms that don't support file drop yet, it might be better to avoid having a "files" property at all on the Clipboard object, instead of having the property and always returning null. So I think it would be good to give the feature an ENABLE define.

[Eric Seidel (no email)]

(In reply to comment #13)
> (From update of attachment 31064 [details] [review])
> I'm going to say r=me with the following caveats:
> 
> 1) Please file follow-up bugs for platforms that don't implement the back end
> for this yet, at least for ports that have a public buildbot.

Sure thing!  I've filed bug 25916 bug 25917 and bug 25918 for the necessary ports.

> 2) I think for platforms that don't support file drop yet, it might be better
> to avoid having a "files" property at all on the Clipboard object, instead of
> having the property and always returning null. So I think it would be good to
> give the feature an ENABLE define.

Hum.  I disagree.  I don't think this should be guarded by ENABLE defines.  That's a lot of extra work to hide this property while the other ports implement the necessary plumbing.

Returning null makes it pretty clear that files are not available.  It doesn't make it clear that it's due to a missing feature of those ports, but I think that given this is an experimental DOM feature of WebKit to begin with, it's OK that the null is ambiguous.  We can add ENABLE defines if it becomes clear later that some port wants to ship with this disabled.  Our current build system potpourri makes it very heavy-weight to add ENABLEs w/o good cause. :)

[Eric Seidel (no email)]

Sorry.  Bug 26710 bug 26711 and bug 26712

[Eric Seidel (no email)]

s/without good cause/without strong need/

[Eric Seidel (no email)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/http/tests/security/clipboard/clipboard-file-access-expected.txt
	A	LayoutTests/http/tests/security/clipboard/clipboard-file-access.html
	A	LayoutTests/http/tests/security/clipboard/resources/TEMPLATE.html
	A	LayoutTests/http/tests/security/clipboard/resources/apple.gif
	A	LayoutTests/http/tests/security/clipboard/resources/clipboard-file-access.js
	A	LayoutTests/http/tests/security/clipboard/resources/mozilla.gif
	M	WebCore/ChangeLog
	M	WebCore/dom/Clipboard.h
	M	WebCore/dom/Clipboard.idl
	M	WebCore/platform/chromium/ClipboardChromium.cpp
	M	WebCore/platform/chromium/ClipboardChromium.h
	M	WebCore/platform/gtk/ClipboardGtk.cpp
	M	WebCore/platform/gtk/ClipboardGtk.h
	M	WebCore/platform/mac/ClipboardMac.h
	M	WebCore/platform/mac/ClipboardMac.mm
	M	WebCore/platform/qt/ClipboardQt.cpp
	M	WebCore/platform/qt/ClipboardQt.h
	M	WebCore/platform/win/ClipboardWin.cpp
	M	WebCore/platform/win/ClipboardWin.h
	M	WebCore/platform/wx/ClipboardWx.cpp
	M	WebCore/platform/wx/ClipboardWx.h
Committed r45168
http://trac.webkit.org/changeset/45168

[Dimitri Glazkov (Google)]

Chromium build fix landed as http://trac.webkit.org/changeset/45176.