[webkit-reviews] review granted: [Bug 84766] REGRESSION (r109022) Safari not placing service data on pasteboard : [Attachment 138642] 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 84766: REGRESSION (r109022) Safari not placing service data on pasteboard
https://bugs.webkit.org/show_bug.cgi?id=84766

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

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


> Source/WebCore/editing/mac/EditorMac.mm:33
>  #import "DocumentFragment.h"
> +#import "DOMRangeInternal.h"

This is out of order, style bit should have complained.

> Source/WebCore/editing/mac/EditorMac.mm:50
>  #import "htmlediting.h"
> +#import "WebNSAttributedStringExtras.h"

This too - upper case comes first in ASCII order.

> Source/WebCore/editing/mac/EditorMac.mm:318
> +PassRefPtr<SharedBuffer> Editor::getBufferSelectionForPasteboard(const
String& pasteboardType)

Did this code come from WebKit1? Can it be deleted there now?

This most definitely doesn't look like something we want to maintain in two
places forever.

> Source/WebKit2/UIProcess/WebPageProxy.h:473
> +    PassRefPtr<WebCore::SharedBuffer> getBufferSelectionForPasteboard(const
String& pasteboardType);

I'm not sure if I can parse "buffer selection". Maybe
"PasteboardDataFromSelection"? Or "SerializedAttributedStringFromSelection"?

This applies to the whole cascade of functions, of course, not just this proxy
wrapper.

> Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:-263
> -    if (!isValid())
> -	   return false;

Why is this not necessary any more?

> Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:278
> +    return SharedBuffer::create(static_cast<unsigned char
*>(sharedMemoryBuffer->data()), size);

Style nit: star goes with the type.