Expose the "Share" menu for text selections on platforms where it's available

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

Expose the "Share" menu for text selections on platforms where it's available

rdar://problem/20034174

[Brady Eidson]

Created attachment 249947 [details]
Patch v1

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/WebPageProxy.cpp:102:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 23 files


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

[Brady Eidson]

Created attachment 249952 [details]
Patch v2

[Brady Eidson]

Created attachment 249981 [details]
Patch v3

[Brady Eidson]

Created attachment 249982 [details]
Patch v4

[Anders Carlsson]

Comment on attachment 249982 [details]
Patch v4

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

> Source/WebKit2/Shared/API/c/WKContextMenuItemTypes.h:124
> +    kWKContextMenuItemTagShareMenu = 5001,

Why does this need to be 5001?

> Source/WebKit2/Shared/API/c/WKSharedAPICast.h:508
> +        if (static_cast<WKContextMenuItemTag>(action) == kWKContextMenuItemTagShareMenu)
> +            return kWKContextMenuItemTagShareMenu;

Instead of this, I think you should add the corresponding WebCore enum.

> Source/WebKit2/Shared/API/c/WKSharedAPICast.h:699
> +    case kWKContextMenuItemTagShareMenu:
> +        return static_cast<WebCore::ContextMenuAction>(tag);

Same comment here.

> Source/WebKit2/Shared/WebContextMenuItemData.cpp:88
> +    m_nativeContextMenuItem = NativeContextMenuItem::create(item);

Is WebContextMenuItemData something we send over the wire? If so, we shouldn't add native items to it.

[Brady Eidson]

(In reply to comment #6)
> Comment on attachment 249982 [details]
> Patch v4
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249982&action=review
> 
> > Source/WebKit2/Shared/API/c/WKContextMenuItemTypes.h:124
> > +    kWKContextMenuItemTagShareMenu = 5001,
> 
> Why does this need to be 5001?

Because the tag is only relevant in WK2-land and I was half heartedly trying to avoid adding it to WebCore also.

So it follows...

> 
> > Source/WebKit2/Shared/API/c/WKSharedAPICast.h:508
> > +        if (static_cast<WKContextMenuItemTag>(action) == kWKContextMenuItemTagShareMenu)
> > +            return kWKContextMenuItemTagShareMenu;
> 
> Instead of this, I think you should add the corresponding WebCore enum.

If I do that, it'll just be another tag.

> > Source/WebKit2/Shared/WebContextMenuItemData.cpp:88
> > +    m_nativeContextMenuItem = NativeContextMenuItem::create(item);
> 
> Is WebContextMenuItemData something we send over the wire? If so, we
> shouldn't add native items to it.

It does go over the wire.

Adding stuff to the WebContextMenuItem directly, however, is going to be a much more invasive change. Which is why I didn't.

But I guess it's The Right Thing To Do™

[Darin Adler]

Comment on attachment 249982 [details]
Patch v4

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

review- because it’s not compiling on GTK and EFL; some comments about possible improvements too.

> Source/WebCore/page/ContextMenuClient.h:66
> +        virtual ContextMenuItem shareSelectedTextMenuItem(const String& selectedText)
> +        {
> +            return ContextMenuItem::shareSelectedTextMenuItem(selectedText);
> +        }

I think it’s better to not have the multi-line function body here in the class definition; makes the class harder to read. We can put it here in the header if we do want it to be inlined. Or we can put it in a .cpp file, maybe Page.cpp might be fine since the page object is where the client lives.

> Source/WebCore/page/ContextMenuController.cpp:920
> +                if (!selectedString.isEmpty() && ContextMenuItem::supportsShareMenu()) {
> +                    ContextMenuItem ShareItem(m_client.shareSelectedTextMenuItem(selectedString));
> +                    appendItem(ShareItem, m_contextMenu.get());
> +                    appendItem(*separatorItem(), m_contextMenu.get());
> +
> +                    m_context.setSelectedText(selectedString);
> +                }

I would consider structuring this differently. I think the best way to do this would be to have the shareSelectedTextMenuItem function return an empty menu item if there is none, and make a helper function that appends the item plus a separator only if the item passed is non-null. We’d also call setSelectedText unconditionally, not just if the selected string is non-empty.

I think it’s strange that we are using these capitalized local variables for the menu items, and this one in particular doesn’t seem to need a local variable at all. It would read better as a one-liner.

> Source/WebCore/platform/ContextMenuItem.h:213
> +#if PLATFORM(COCOA)
> +        static bool supportsShareMenu();
> +        WEBCORE_EXPORT static ContextMenuItem shareSelectedTextMenuItem(const String&);
> +#else
> +        static bool supportsShareMenu() { return false; }
> +#endif

I don’t think we need supportsShareMenu for non-Cocoa platforms. Another way to do this might just to make shareSelectedTextMenuItem return a null ContextMenuItem instead of having a separate function returning a boolean.

If we did need this function for non-Cocoa platforms, I’d rather have this at the bottom of the file:

    #if !PLATFORM(COCOA)

    bool ContextMenuItem::supportsShareMenu()
    {
        return true;
    }

    #endif

And then leave the supportsShareMenu function unconditional inside the class definition. Conditionals are getting little out of hand in this header and I think we should strive to minimize them in the class definition.

It seems this should be PLATFORM(MAC), not PLATFORM(COCOA), by the way. Or maybe USE(APPKIT).

> Source/WebCore/platform/mac/ContextMenuItemMac.mm:106
> +NSMenuItem* ContextMenuItem::getPlatformDescription() const

Wrong formatting of the NSMenuItem* here; move the * please.

> Source/WebCore/platform/mac/ContextMenuItemMac.mm:205
> +    NSMenuItem *nsItem = [NSMenuItem standardShareMenuItemWithItems:@[ (NSString *)selectedText ]];
> +    return ContextMenuItem(nsItem);

I don’t think we need this local variable. Lets write this as a one-liner. Might not even need the explicit construction of ContextMenuItem.

> Source/WebKit2/Shared/NativeContextMenuItem.h:47
> +class NativeContextMenuItem : public RefCounted<NativeContextMenuItem> {

I think this is a single-ownership object, so I suggest we use unique_ptr instead of RefPtr and not do all the RefCounted stuff.

> Source/WebKit2/Shared/NativeContextMenuItem.h:49
> +    static PassRefPtr<NativeContextMenuItem> create(const WebCore::ContextMenuItem& coreItem)

Should return a Ref, not a PassRefPtr.

> Source/WebKit2/Shared/NativeContextMenuItem.h:58
> +#if PLATFORM(COCOA)
> +    NSMenuItem *nsMenuItem() { return m_nsMenuItem.get(); }
> +#endif

This needs to be USE(APPKIT), not PLATFORM(COCOA). There is no NSMenuItem on iOS.

> Source/WebKit2/Shared/NativeContextMenuItem.h:61
> +    NativeContextMenuItem(const WebCore::ContextMenuItem& coreItem);

No need for the argument name here.

> Source/WebKit2/WebProcess/WebCoreSupport/mac/WebContextMenuClientMac.mm:100
> +    return ContextMenuItem(SubmenuType, toImpl(kWKContextMenuItemTagShareMenu), StringImpl::empty());

StringImpl::empty() is not right here; although it will work, I think you just want emptyString().

[Brady Eidson]

(In reply to comment #8)
> Comment on attachment 249982 [details]
> Patch v4
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249982&action=review
> 
> review- because it’s not compiling on GTK and EFL; some comments about
> possible improvements too.
> 
> > Source/WebCore/page/ContextMenuClient.h:66
> > +        virtual ContextMenuItem shareSelectedTextMenuItem(const String& selectedText)
> > +        {
> > +            return ContextMenuItem::shareSelectedTextMenuItem(selectedText);
> > +        }
> 
> I think it’s better to not have the multi-line function body here in the
> class definition; makes the class harder to read. We can put it here in the
> header if we do want it to be inlined. Or we can put it in a .cpp file,
> maybe Page.cpp might be fine since the page object is where the client lives.

I think it should be inlined. I can make it a single line.

> 
> > Source/WebCore/page/ContextMenuController.cpp:920
> > +                if (!selectedString.isEmpty() && ContextMenuItem::supportsShareMenu()) {
> > +                    ContextMenuItem ShareItem(m_client.shareSelectedTextMenuItem(selectedString));
> > +                    appendItem(ShareItem, m_contextMenu.get());
> > +                    appendItem(*separatorItem(), m_contextMenu.get());
> > +
> > +                    m_context.setSelectedText(selectedString);
> > +                }
> 
> I would consider structuring this differently. I think the best way to do
> this would be to have the shareSelectedTextMenuItem function return an empty
> menu item if there is none, and make a helper function that appends the item
> plus a separator only if the item passed is non-null. We’d also call
> setSelectedText unconditionally, not just if the selected string is
> non-empty.

You just described my first swipe at this. Unfortunately, ContextMenuItems have no concept of "null", and they are also such a cross-platform mess that it quickly became an out-of-scope project to clean them up.

I agree this would be the best structure in the future!

> I think it’s strange that we are using these capitalized local variables for
> the menu items, and this one in particular doesn’t seem to need a local
> variable at all. It would read better as a one-liner.

I agree it's strange we use these capitalized local variables, but I would've felt much worse breaking with the established style of the file.

I agree it would read better as a one liner without the local variable, but because appendItem takes a ContextMenuItem&, that doesn't compile:
...Source/WebCore/page/ContextMenuController.cpp:916:32: Non-const lvalue reference to type 'WebCore::ContextMenuItem' cannot bind to a temporary of type 'WebCore::ContextMenuItem'

And that explains why there's all these temporaries in the file in the first place.

> > Source/WebCore/platform/ContextMenuItem.h:213
> > +#if PLATFORM(COCOA)
> > +        static bool supportsShareMenu();
> > +        WEBCORE_EXPORT static ContextMenuItem shareSelectedTextMenuItem(const String&);
> > +#else
> > +        static bool supportsShareMenu() { return false; }
> > +#endif
> 
> I don’t think we need supportsShareMenu for non-Cocoa platforms. Another way
> to do this might just to make shareSelectedTextMenuItem return a null
> ContextMenuItem instead of having a separate function returning a boolean.

Again, refer back to the previous comment about a null ContextMenuItem - Unfortunately there is no such thing, and it's a non-trivial cleanup project to implement one. :(

> If we did need this function for non-Cocoa platforms, I’d rather have this
> at the bottom of the file:
> 
>     #if !PLATFORM(COCOA)
> 
>     bool ContextMenuItem::supportsShareMenu()
>     {
>         return true;
>     }
> 
>     #endif
> 
> And then leave the supportsShareMenu function unconditional inside the class
> definition. Conditionals are getting little out of hand in this header and I
> think we should strive to minimize them in the class definition.

This makes sense and it is now done.


> > Source/WebCore/platform/mac/ContextMenuItemMac.mm:106
> > +NSMenuItem* ContextMenuItem::getPlatformDescription() const
> 
> Wrong formatting of the NSMenuItem* here; move the * please.

That's my tendency to follow established style within a file. Will fix all uses.

> > Source/WebCore/platform/mac/ContextMenuItemMac.mm:205
> > +    NSMenuItem *nsItem = [NSMenuItem standardShareMenuItemWithItems:@[ (NSString *)selectedText ]];
> > +    return ContextMenuItem(nsItem);
> 
> I don’t think we need this local variable. Lets write this as a one-liner.
> Might not even need the explicit construction of ContextMenuItem.

This changed a bit based on Ander's feedback, and needs to be a multi-liner, but a different multi-liner.

> 
> > Source/WebKit2/Shared/NativeContextMenuItem.h:47
> > +class NativeContextMenuItem : public RefCounted<NativeContextMenuItem> {
> 
> I think this is a single-ownership object, so I suggest we use unique_ptr
> instead of RefPtr and not do all the RefCounted stuff.

In the current version of the patch, the NativeContextMenuItem was passed around to multiple different WebContextMenuItemDatas and therefore couldn't be uniqued.

In the new version, it is truly owned by a single WebContextMenuItem and can be uniqued.

> > Source/WebKit2/WebProcess/WebCoreSupport/mac/WebContextMenuClientMac.mm:100
> > +    return ContextMenuItem(SubmenuType, toImpl(kWKContextMenuItemTagShareMenu), StringImpl::empty());
> 
> StringImpl::empty() is not right here; although it will work, I think you
> just want emptyString().

I originally used ASCIILiteral(""), because it was late at night. Guess what the compiler recommended I use instead?  ;)

I wonder if there's a reason the error message encourages StringImpl::empty instead of emptyString()!

[Brady Eidson]

Created attachment 249999 [details]
Patch v5

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.mm:308:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 30 files


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

[Brady Eidson]

Style bot is full of crap.

Attempting build fixes.

[Brady Eidson]

Created attachment 250002 [details]
Patch v6

One attempt to fix EFL/GTK builds

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.mm:308:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 34 files


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

[Brady Eidson]

Created attachment 250003 [details]
Patch v7

At this point I'm going to stop working on the WK2 build failures for the other ports, but this was apparently a boneheaded move in WebCore - So v7 is an attempt to fix that.

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.mm:308:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 34 files


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

[Brady Eidson]

Will fix windows build.

[Brady Eidson]

Created attachment 250005 [details]
Patch v8

[Brady Eidson]

Created attachment 250006 [details]
Patch v8.1

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.mm:308:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 33 files


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

[WebKit Commit Bot]

Comment on attachment 250006 [details]
Patch v8.1

Clearing flags on attachment: 250006

Committed r182293: <http://trac.webkit.org/changeset/182293>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Brent Fulgham]

This broke other bots (as shown by EWS):

FAILED: /usr/lib/ccache/g++-4.8   -DBUILDING_GTK__=1 -DBUILDING_WEBKIT -DBUILDING_WITH_CMAKE=1 -DDATA_DIR=\"share\" -DDEVELOPMENT_BUILD=1 -DENABLE_3D_RENDERING=1 -DENABLE_PLUGIN_PROCESS=1 -DENABLE_PLUGIN_PROCESS_GTK2=1 -DGETTEXT_PACKAGE=\"WebKit2GTK-4.0\" -DHAVE_CONFIG_H=1 -DLIBDIR=\"/usr/local/lib/x86_64-linux-gnu\" -DLIBEXECDIR=\"/usr/local/libexec/webkit2gtk-4.0\" -DMOZ_X11=1 -DPACKAGE_LOCALE_DIR=\"/usr/local/share/locale\" -DUSER_AGENT_GTK_MAJOR_VERSION=601 -DUSER_AGENT_GTK_MINOR_VERSION=1 -DWEBKIT2_COMPILATION -DWEBKITGTK_API_VERSION_STRING=\"4.0\" -DWTF_PLATFORM_X11=1 -DWTF_USE_3D_GRAPHICS=1 -DWTF_USE_EGL=1 -DWTF_USE_GLX=1 -DWTF_USE_GSTREAMER -DWTF_USE_GSTREAMER_MPEGTS -DWTF_USE_LIBNOTIFY=0 -DWTF_USE_OPENGL=1 -DWTF_USE_OPENWEBRTC -DWTF_USE_REDIRECTED_XCOMPOSITE_WINDOW=1 -DWTF_USE_TEXTURE_MAPPER=1 -DWTF_USE_TEXTURE_MAPPER_GL=1 -DWTF_USE_WEBAUDIO_GSTREAMER -DWebKit2_EXPORTS -DXP_UNIX -pipe  -std=c++11 -Wno-unused-parameter -O3 -DNDEBUG -fno-exceptions -fno-strict-aliasing -fno-rtti -fPIC -I../../Source/WebKit2/Platform -I../../Source/WebKit2/Shared -I../../Source/WebKit2/Shared/API/c -I../../Source/WebKit2/UIProcess/API/C -I../../Source/WebKit2/WebProcess/InjectedBundle -I../../Source/WebKit2/WebProcess/InjectedBundle/API/c -IDerivedSources -IDerivedSources/InjectedBundle -IDerivedSources/webkitdom -IDerivedSources/ForwardingHeaders -IDerivedSources/ForwardingHeaders/webkit2gtk -IDerivedSources/ForwardingHeaders/webkit2gtk-webextension -IDerivedSources/webkit2gtk/webkit2 -IDerivedSources/webkit2gtk -I../../Source/JavaScriptCore/llint -I../../Source/WebKit2 -I../../Source/WebKit2/DatabaseProcess -I../../Source/WebKit2/DatabaseProcess/IndexedDB -I../../Source/WebKit2/DatabaseProcess/IndexedDB/sqlite -I../../Source/WebKit2/NetworkProcess -I../../Source/WebKit2/NetworkProcess/FileAPI -I../../Source/WebKit2/NetworkProcess/cache -I../../Source/WebKit2/Platform/IPC -I../../Source/WebKit2/PluginProcess -I../../Source/WebKit2/Shared/API -I../../Source/WebKit2/Shared/Authentication -I../../Source/WebKit2/Shared/CoreIPCSupport -I../../Source/WebKit2/Shared/Databases -I../../Source/WebKit2/Shared/Databases/IndexedDB -I../../Source/WebKit2/Shared/Downloads -I../../Source/WebKit2/Shared/FileAPI -I../../Source/WebKit2/Shared/Network -I../../Source/WebKit2/Shared/Network/CustomProtocols -I../../Source/WebKit2/Shared/Plugins -I../../Source/WebKit2/Shared/Plugins/Netscape -I../../Source/WebKit2/Shared/Plugins/Netscape/x11 -I../../Source/WebKit2/Shared/WebsiteData -I../../Source/WebKit2/UIProcess -I../../Source/WebKit2/UIProcess/API -I../../Source/WebKit2/UIProcess/API/cpp -I../../Source/WebKit2/UIProcess/Authentication -I../../Source/WebKit2/UIProcess/Databases -I../../Source/WebKit2/UIProcess/Downloads -I../../Source/WebKit2/UIProcess/InspectorServer -I../../Source/WebKit2/UIProcess/Launcher -I../../Source/WebKit2/UIProcess/Network -I../../Source/WebKit2/UIProcess/Network/CustomProtocols -I../../Source/WebKit2/UIProcess/Notifications -I../../Source/WebKit2/UIProcess/Plugins -I../../Source/WebKit2/UIProcess/Storage -I../../Source/WebKit2/UIProcess/UserContent -I../../Source/WebKit2/UIProcess/WebsiteData -I../../Source/WebKit2/WebProcess -I../../Source/WebKit2/WebProcess/ApplicationCache -I../../Source/WebKit2/WebProcess/Battery -I../../Source/WebKit2/WebProcess/Cookies -I../../Source/WebKit2/WebProcess/Databases -I../../Source/WebKit2/WebProcess/Databases/IndexedDB -I../../Source/WebKit2/WebProcess/FileAPI -I../../Source/WebKit2/WebProcess/FullScreen -I../../Source/WebKit2/WebProcess/Geolocation -I../../Source/WebKit2/WebProcess/IconDatabase -I../../Source/WebKit2/WebProcess/InjectedBundle/API -I../../Source/WebKit2/WebProcess/InjectedBundle/DOM -I../../Source/WebKit2/WebProcess/Launching -I../../Source/WebKit2/WebProcess/MediaCache -I../../Source/WebKit2/WebProcess/MediaStream -I../../Source/WebKit2/WebProcess/Network -I../../Source/WebKit2/WebProcess/Notifications -I../../Source/WebKit2/WebProcess/OriginData -I../../Source/WebKit2/WebProcess/Plugins -I../../Source/WebKit2/WebProcess/Plugins/Netscape -I../../Source/WebKit2/WebProcess/ResourceCache -I../../Source/WebKit2/WebProcess/Storage -I../../Source/WebKit2/WebProcess/UserContent -I../../Source/WebKit2/WebProcess/WebCoreSupport -I../../Source/WebKit2/WebProcess/WebPage -I../../Source/WebCore -I../../Source/WebCore/Modules/battery -I../../Source/WebCore/Modules/mediastream -I../../Source/WebCore/Modules/networkinfo -I../../Source/WebCore/Modules/notifications -I../../Source/WebCore/Modules/streams -I../../Source/WebCore/Modules/vibration -I../../Source/WebCore/Modules/webdatabase -I../../Source/WebCore/accessibility -I../../Source/WebCore/bindings/js -I../../Source/WebCore/bindings -I../../Source/WebCore/bridge -I../../Source/WebCore/bridge/jsc -I../../Source/WebCore/css -I../../Source/WebCore/dom -I../../Source/WebCore/dom/default -I../../Source/WebCore/editing -I../../Source/WebCore/fileapi -I../../Source/WebCore/history -I../../Source/WebCore/html -I../../Source/WebCore/html/shadow -I../../Source/WebCore/html/track -I../../Source/WebCore/inspector -I../../Source/WebCore/loader -I../../Source/WebCore/loader/archive -I../../Source/WebCore/loader/icon -I../../Source/WebCore/loader/cache -I../../Source/WebCore/page -I../../Source/WebCore/page/animation -I../../Source/WebCore/page/scrolling -I../../Source/WebCore/platform -I../../Source/WebCore/platform/animation -I../../Source/WebCore/platform/audio -I../../Source/WebCore/platform/graphics -I../../Source/WebCore/platform/graphics/filters -I../../Source/WebCore/platform/graphics/harfbuzz -I../../Source/WebCore/platform/graphics/harfbuzz/ng -I../../Source/WebCore/platform/graphics/surfaces -I../../Source/WebCore/platform/graphics/texmap -I../../Source/WebCore/platform/graphics/transforms -I../../Source/WebCore/platform/mediastream -I../../Source/WebCore/platform/network -I../../Source/WebCore/platform/sql -I../../Source/WebCore/platform/text -I../../Source/WebCore/plugins -I../../Source/WebCore/rendering -I../../Source/WebCore/rendering/line -I../../Source/WebCore/rendering/shapes -I../../Source/WebCore/rendering/style -I../../Source/WebCore/storage -I../../Source/WebCore/style -I../../Source/WebCore/svg -I../../Source/WebCore/svg/graphics -I../../Source/WebCore/svg/properties -I../../Source/JavaScriptCore -I../../Source/JavaScriptCore/ForwardingHeaders -I../../Source/JavaScriptCore/API -I../../Source/JavaScriptCore/assembler -I../../Source/JavaScriptCore/bytecode -I../../Source/JavaScriptCore/bytecompiler -I../../Source/JavaScriptCore/collector/handles -I../../Source/JavaScriptCore/dfg -I../../Source/JavaScriptCore/disassembler -I../../Source/JavaScriptCore/heap -I../../Source/JavaScriptCore/interpreter -I../../Source/JavaScriptCore/jit -I../../Source/JavaScriptCore/parser -I../../Source/JavaScriptCore/profiler -I../../Source/JavaScriptCore/runtime -I../../Source/WTF -IDerivedSources/JavaScriptCore -IDerivedSources/WebCore -IDerivedSources/WebKit2 -IDerivedSources/WebKit2/include -I. -I../../Source -I../../Source/ThirdParty/ANGLE -I../../Source/ThirdParty/ANGLE/include/KHR -I../../Source/WebKit2/PluginProcess/unix -I../../Source/WebCore/platform/cairo -I../../Source/WebCore/platform/gtk -I../../Source/WebCore/platform/graphics/cairo -I../../Source/WebCore/platform/graphics/opentype -I../../Source/WebCore/platform/network/soup -I../../Source/WebCore/platform/text/enchant -I../../Source/WebKit2/DatabaseProcess/unix -I../../Source/WebKit2/NetworkProcess/gtk -I../../Source/WebKit2/NetworkProcess/unix -I../../Source/WebKit2/Shared/API/c/gtk -I../../Source/WebKit2/Shared/Network/CustomProtocols/soup -I../../Source/WebKit2/Shared/Downloads/soup -I../../Source/WebKit2/Shared/gtk -I../../Source/WebKit2/Shared/soup -I../../Source/WebKit2/Shared/unix -I../../Source/WebKit2/UIProcess/API/C/cairo -I../../Source/WebKit2/UIProcess/API/C/gtk -I../../Source/WebKit2/UIProcess/API/C/soup -I../../Source/WebKit2/UIProcess/API/cpp/gtk -I../../Source/WebKit2/UIProcess/API/gtk -I../../Source/WebKit2/UIProcess/Network/CustomProtocols/soup -I../../Source/WebKit2/UIProcess/Plugins/gtk -I../../Source/WebKit2/UIProcess/gtk -I../../Source/WebKit2/UIProcess/soup -I../../Source/WebKit2/WebProcess/InjectedBundle/API/gtk -I../../Source/WebKit2/WebProcess/gtk -I../../Source/WebKit2/WebProcess/soup -I../../Source/WebKit2/WebProcess/unix -I../../Source/WebKit2/WebProcess/WebCoreSupport/gtk -I../../Source/WebKit2/WebProcess/WebCoreSupport/soup -I../../Source/WebKit2/WebProcess/WebPage/atk -I../../Source/WebKit2/WebProcess/WebPage/gtk -I../../Source/WTF/wtf/gtk -I../../Source/WTF/wtf/gobject -I../DependenciesGTK/Root/include/cairo -I/usr/include/enchant -I../DependenciesGTK/Root/include/libxml2 -I../DependenciesGTK/Root/include/glib-2.0 -I../DependenciesGTK/Root/lib64/glib-2.0/include -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I../DependenciesGTK/Root/include/harfbuzz -I../DependenciesGTK/Root/include/libsoup-2.4 -I../DependenciesGTK/Root/include/gtk-3.0 -I../DependenciesGTK/Root/include/atk-1.0 -I../DependenciesGTK/Root/include/at-spi2-atk/2.0 -I../DependenciesGTK/Root/include/gio-unix-2.0 -I../DependenciesGTK/Root/include/gdk-pixbuf-2.0 -I../DependenciesGTK/Root/include/freetype2 -I../DependenciesGTK/Root/include/pixman-1 -I/usr/include/pango-1.0 -I/usr/include/libpng12 -I/usr/include/libdrm -I../DependenciesGTK/Root/include/gtk-3.0/unix-print    -Wall -Wextra -Wcast-align -Wformat-security -Wmissing-format-attribute -Wpointer-arith -Wundef -Wwrite-strings  -include /home/slave/webkitgtk/gtk-linux-64-release/build/Source/WebKit2/WebKit2Prefix.h -MMD -MT Source/WebKit2/CMakeFiles/WebKit2.dir/UIProcess/API/gtk/WebKitContextMenuClient.cpp.o -MF "Source/WebKit2/CMakeFiles/WebKit2.dir/UIProcess/API/gtk/WebKitContextMenuClient.cpp.o.d" -o Source/WebKit2/CMakeFiles/WebKit2.dir/UIProcess/API/gtk/WebKitContextMenuClient.cpp.o -c ../../Source/WebKit2/UIProcess/API/gtk/WebKitContextMenuClient.cpp
../../Source/WebKit2/UIProcess/API/gtk/WebKitContextMenuClient.cpp:37:10: error: ‘bool ContextMenuClient::getContextMenuFromProposedMenu(WebKit::WebPageProxy&, const WTF::Vector<WebKit::WebContextMenuItemData>&, WTF::Vector<WebKit::WebContextMenuItemData>&, const WebKit::WebHitTestResult::Data&, API::Object*)’ marked override, but does not override
     bool getContextMenuFromProposedMenu(WebPageProxy&, const Vector<WebContextMenuItemData>& proposedMenu, Vector<WebContextMenuItemData>&, const WebHitTestResult::Data& hitTestResultData, API::Object* userData) override

[WebKit Commit Bot]

Re-opened since this is blocked by bug 143355

[Csaba Osztrogonác]

Relanded in http://trac.webkit.org/changeset/182303.
( But the build is still broken. :( )

[Csaba Osztrogonác]

buildfix landed in http://trac.webkit.org/changeset/182306

[Csaba Osztrogonác]

GTK build is still broken:

../../Source/WebKit2/UIProcess/API/gtk/WebKitContextMenuClient.cpp:37:10: error: ‘bool ContextMenuClient::getContextMenuFromProposedMenu(WebKit::WebPageProxy&, const WTF::Vector<WebKit::WebContextMenuItemData>&, WTF::Vector<WebKit::WebContextMenuItemData>&, const WebKit::WebHitTestResult::Data&, API::Object*)’ marked override, but does not override
     bool getContextMenuFromProposedMenu(WebPageProxy&, const Vector<WebContextMenuItemData>& proposedMenu, Vector<WebContextMenuItemData>&, const WebHitTestResult::Data& hitTestResultData, API::Object* userData) override
          ^

[Philippe Normand]

GTK build fix: http://trac.webkit.org/changeset/182310