[webkit-reviews] review requested: [Bug 40540] [chromium] Add new stubs for querying platform drag-and-drop and copy-and-paste data. : [Attachment 59409] Patch

[bugzilla-daemon at webkit.org]

Daniel Cheng <dcheng at chromium.org> has asked  for review:
Bug 40540: [chromium] Add new stubs for querying platform drag-and-drop and
copy-and-paste data.
https://bugs.webkit.org/show_bug.cgi?id=40540

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

------- Additional Comments from Daniel Cheng <dcheng at chromium.org>
(In reply to comment #53)
> (From update of attachment 59308 [details])
> WebCore/platform/chromium/ChromiumBridge.h:106
>  +	      static HashSet<String>
clipboardReadTypes(PasteboardPrivate::ClipboardBuffer, bool*
containsFilenames);
> clipboardReadTypes might be better named clipboardGetAvailableTypes
> 

Done. See comment below about naming.

> WebKit/chromium/public/WebClipboard.h:57
>  +	      BufferDrag,
> It is probably worth saying something about the meaning of BufferDrag here.
> Mention that it is a readonly buffer corresponding to the last drag-n-drop
> operation or something like that.

Done.

> WebKit/chromium/public/WebClipboard.h:77
>  +	  virtual WebVector<WebString> readTypes(Buffer, bool*
containsFilenames) = 0;
> readTypes -> getAvailableTypes or maybe readAvailableTypes?  what does the
> return value of readData indicate?  please specify.
> 

Done and done. I chose read over get for consistency.

> WebKit/chromium/src/ChromiumBridge.cpp:203
>  +	  WebDragData dragData; // FIXME: Define the conversion from
ClipboardData to WebDragData.
> this would be a good place to call notImplemented()
> 

Done.

> WebKit/chromium/src/ChromiumBridge.cpp:221
>  +	  WebKit::WebString resultData;
> no need for the WebKit:: prefix here

Done.

(In reply to comment #54)
> (From update of attachment 59308 [details])
> WebKit/chromium/public/WebClipboard.h:80
>  +	  virtual WebVector<WebString> readFilenames(Buffer) = 0;
> please provide default implementations of these WebClipboard methods.
> any method to be implemented by the embedder should have a default
> implementation.

Done.