Bug 130424 – Make image controls menu work in WK2

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brady Eidson]

Make image controls menu work in WK2

[Brady Eidson]

<rdar://problem/16294412>

[Brady Eidson]

Created attachment 227110 [details]
Patch v1

[Brady Eidson]

Created attachment 227115 [details]
Patch v2 - Build fixes

[Brady Eidson]

Created attachment 227127 [details]
Patch v3 - More build fixes

[WebKit Commit Bot]

Attachment 227127 [details] did not pass style-queue:


ERROR: Source/WebKit2/Platform/unix/SharedMemoryUnix.cpp:57:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

(In reply to comment #5)
> Attachment 227127 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebKit2/Platform/unix/SharedMemoryUnix.cpp:57:  Tab found; better to use spaces  [whitespace/tab] [1]
> Total errors found: 1 in 22 files
> 

Whoops.  Fixed locally.

[Tim Horton]

Comment on attachment 227127 [details]
Patch v3 - More build fixes

View in context: https://bugs.webkit.org/attachment.cgi?id=227127&action=review

> Source/WebKit2/ChangeLog:47
> +        (-[WKSharingServicePickerDelegate WebKit::]):

WebKit:: :|

> Source/WebKit2/Platform/mac/SharedMemoryMac.cpp:57
> +void SharedMemory::Handle::clear()

maybe “release” is a better name than “clear”… I don’t know. More evident that you’re losing something.

> Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.mm:177
> +    // FIXME: What do we do if there are multiple items?

Please remove my silly comment (for now).

> Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.mm:193
> +- (NSRect)sharingService:(NSSharingService *)sharingService sourceFrameOnScreenForShareItem:(id <NSPasteboardWriting>)item
> +{
> +    notImplemented();
> +    return NSZeroRect;
> +}
> +
> +- (NSImage *)sharingService:(NSSharingService *)sharingService transitionImageForShareItem:(id <NSPasteboardWriting>)item contentRect:(NSRect *)contentRect
> +{
> +    notImplemented();
> +    return nil;
> +}

I think we can elide these.

> Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.mm:354
> +    NSMenu* menu = m_servicesMenu ? m_servicesMenu.get() : [m_popup menu];

star on the wrong side

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3088
> +    m_contextMenu->replaceControlledImage(bitmap->createImage());

what happens in createImage if bitmapHandle.isNull()?

[Brady Eidson]

Addressed most feedback in most obvious ways possible.

https://trac.webkit.org/changeset/165856