[webkit-reviews] review granted: [Bug 77567] Refactor Mac platform implementation of the Pasteboard class : [Attachment 125197] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Enrica Casucci
<enrica at apple.com>'s request for review:
Bug 77567: Refactor Mac platform implementation of the Pasteboard class
https://bugs.webkit.org/show_bug.cgi?id=77567

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

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=125197&action=review


> Source/WebCore/ChangeLog:8
> +	   writeSelectionForTypes has been left to support Mac OSX services.

s/Mac OSX/OS X/

> Source/WebCore/platform/Pasteboard.h:-84
> +    // This is required to support Mac OSX services.
>      void writeSelectionForTypes(NSArray* pasteboardTypes, Range*
selectedRange, bool canSmartCopyOrDelete, Frame*);
> -    void writeURLForTypes(NSArray* types, const KURL&, const String&
titleStr, Frame*);

s/Mac OSX/OS X/

I'm not sure if I fully understand why one of the two methods needed for
services can be moved, but not another.

> Source/WebCore/platform/mac/DragDataMac.mm:168
> +    // FIXME: using the editorClient to call into webkit, for now, since 
> +    // calling webkit_canonicalize from WebCore involves migrating a sizable
amount of 
> +    // helper code that should either be done in a separate patch or figured
out in another way.

s/webkit/WebKit/.

Why do we need WebKit to canonicalize a URL? Normally, KURL does that.

> Source/WebCore/platform/mac/DragDataMac.mm:187
> +	   // FIXME: Maybe it makes more sense to allow multiple files and only
use the first one?

This is something Jon Lee would have an opinion about.

> Source/WebCore/platform/mac/PasteboardMac.mm:380
> -PassRefPtr<DocumentFragment> Pasteboard::documentFragmentWithRtf(Frame*
frame, NSString* pboardType)
> +static PassRefPtr<DocumentFragment> documentFragmentWithRtf(Frame* frame,
NSString* pboardType, NSPasteboard* pasteboard)

You can consider fixing coding style when touching code like this.