[webkit-reviews] review denied: [Bug 47482] Add webkitGetDataBlob/webkitSetDataBlob to event.dataTransfer : [Attachment 70437] Patch

[bugzilla-daemon at webkit.org]

Jian Li <jianli at chromium.org> has denied Daniel Cheng <dcheng at chromium.org>'s
request for review:
Bug 47482: Add webkitGetDataBlob/webkitSetDataBlob to event.dataTransfer
https://bugs.webkit.org/show_bug.cgi?id=47482

Attachment 70437: Patch
https://bugs.webkit.org/attachment.cgi?id=70437&action=review

------- Additional Comments from Jian Li <jianli at chromium.org>
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?