Bug 47482 – Add webkitGetDataBlob/webkitSetDataBlob to event.dataTransfer

• Component: Platform
• Status: RESOLVED INVALID

[Daniel Cheng]

For now, just add stubs. It'd be nice to have event.dataTransfer support binary blobs to support operations like copying and pasting images.

mrobinson was interested in this change as well, but for some reason, Bugzilla won't let me CC him.

[Daniel Cheng]

Created attachment 70426 [details]
Patch

[Early Warning System Bot]

Attachment 70426 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4362018

[Daniel Cheng]

Created attachment 70437 [details]
Patch

[Jian Li]

Comment on attachment 70437 [details]
Patch

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

> WebCore/ChangeLog:9
> +        copy-and-paste or drag-and-drop in WebKit.

Can you add a link to the proposal or discussion?

> WebCore/ChangeLog:11
> +        No tests. No functionality changed.

I think we should say that the test will be added when we have the stub implemented.

> WebCore/WebCore.xcodeproj/project.pbxproj:748
> +		2EDEF1F4121B0EFC00726DB2 /* BlobData.h in Headers */ = {isa = PBXBuildFile; fileRef = 2EDEF1EE121B0EFC00726DB2 /* BlobData.h */; settings = {ATTRIBUTES = (Private, ); }; };

Why do we need to change the settings? Do you mean to copy the header file to PrivateHeaders? If needed, please explain in ChangeLog.

> WebCore/dom/Clipboard.h:67
> +        virtual PassRefPtr<Blob> webkitGetDataBlob(const String& type, bool& success) const = 0;

Can we simply return 0 to indicate failure, instead of introducing an extra boolean output parameter?

> WebCore/dom/Clipboard.h:68
> +        virtual bool webkitSetDataBlob(const String& type, PassRefPtr<Blob> data) = 0;

You can omit 'data' argument name.

> WebCore/dom/Clipboard.idl:41
> +        [Custom] void webkitGetDataBlob(in DOMString type)

Do we need Custom attribute? I think probably we can remove it if we remove 'success' parameter Clipboard::webkitGetDataBlob?
Do we consider adding a feature guard to wrap this new feature?

[Daniel Cheng]

Created attachment 70897 [details]
Patch

[Daniel Cheng]

(In reply to comment #4)
> (From update of attachment 70437 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=70437&action=review
> 
> > WebCore/ChangeLog:9
> > +        copy-and-paste or drag-and-drop in WebKit.
> 
> Can you add a link to the proposal or discussion?

There wasn't much discussion, since no one ever replied to the post. But I linked it anyway.

> 
> > WebCore/ChangeLog:11
> > +        No tests. No functionality changed.
> 
> I think we should say that the test will be added when we have the stub implemented.
> 

Done.

> > WebCore/WebCore.xcodeproj/project.pbxproj:748
> > +		2EDEF1F4121B0EFC00726DB2 /* BlobData.h in Headers */ = {isa = PBXBuildFile; fileRef = 2EDEF1EE121B0EFC00726DB2 /* BlobData.h */; settings = {ATTRIBUTES = (Private, ); }; };
> 
> Why do we need to change the settings? Do you mean to copy the header file to PrivateHeaders? If needed, please explain in ChangeLog.
> 

Done.

> > WebCore/dom/Clipboard.h:67
> > +        virtual PassRefPtr<Blob> webkitGetDataBlob(const String& type, bool& success) const = 0;
> 
> Can we simply return 0 to indicate failure, instead of introducing an extra boolean output parameter?
> 

I opted to follow the pattern in the IDL file, but I'm not opposed to changing it.

> > WebCore/dom/Clipboard.h:68
> > +        virtual bool webkitSetDataBlob(const String& type, PassRefPtr<Blob> data) = 0;
> 
> You can omit 'data' argument name.
> 

Done.

> > WebCore/dom/Clipboard.idl:41
> > +        [Custom] void webkitGetDataBlob(in DOMString type)
> 
> Do we need Custom attribute? I think probably we can remove it if we remove 'success' parameter Clipboard::webkitGetDataBlob?

I opted to follow the way existing code did it. I'm not really familiar with the IDL bindings, so it seemed safest to copy the existing pattern in the file.

> Do we consider adding a feature guard to wrap this new feature?

I did not consider this. Who defines these flags when you're building, say, the Chrome port?

[WebKit Review Bot]

Attachment 70897 [details] did not build on win:
Build output: http://queues.webkit.org/results/4410074

[Daniel Cheng]

Created attachment 71825 [details]
Patch

[Dmitry Titov]

Comment on attachment 71825 [details]
Patch

I think there should be a better proposal before this kind of patch can land.

The linked post (http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2010-June/026910.html) only contains a question about what would be the preferred shape of API for exchanging non-text data with clipboard. I think it didn't get responses because context was unclear.

It is not exactly clear what kind of use case these APIs are going to enable, what kind of data are we talkign about, sizes and speeds (if data is big or produced on demand, then perhaps it should not be a synchronous API?) etc.  It is hard to say, based on information in this bug, whether or not these APIs are all right since it's unclear what exact data transfer scenarios they are going to be used for.

If this is a part of a bigger modification, it could be also useful to see what it is (since it mentions non-text data in clipboardData). In short, I think this requires a proposal or design doc.