[webkit-reviews] review granted: [Bug 131992] Change Image Controls replacement to use selection and paste. : [Attachment 229893] Patch v1

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 131992: Change Image Controls replacement to use selection and paste.
https://bugs.webkit.org/show_bug.cgi?id=131992

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

------- Additional Comments from Tim Horton <thorton at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=229893&action=review


> Source/WebCore/html/shadow/mac/ImageControlsButtonElementMac.cpp:110
> +	   while (parent) {
> +	       if (parent->isShadowRoot()) {

Surprised there's not already something to do this.

> Source/WebCore/page/ContextMenuController.cpp:-1456
> -void ContextMenuController::replaceControlledImage(PassRefPtr<Image>
newImage)

Yayyyyy!

> Source/WebKit/mac/Misc/WebSharingServicePickerController.mm:135
> +    [pasteboard declareTypes:[NSArray arrayWithObject:NSPasteboardTypeTIFF]
owner:nil];

@[ NSPasteboardTypeTIFF ]

> Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.h:60
> +    WebPageProxy* page() const { return m_page; }

it looks to me like this should return a reference, no?

> Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.mm:235
>      RetainPtr<CGImageSourceRef> source =
adoptCF(CGImageSourceCreateWithData((CFDataRef)[items objectAtIndex:0], NULL));

>      RetainPtr<CGImageRef> image =
adoptCF(CGImageSourceCreateImageAtIndex(source.get(), 0, NULL));

why are we still doing this part? do you really need to make a CGImageRef here
and then not use it?

> Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.mm:241
> +    [pasteboard declareTypes:[NSArray arrayWithObject:NSPasteboardTypeTIFF]
owner:nil];

@[ NSPasteboardTypeTIFF ]