Bug 17324 – Remove PLATFORM ifdefs from ContextMenu.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

Remove PLATFORM(GTK) ifdefs from ContextMenu.cpp

Uggggly.

platform/ContextMenu.cpp:49:#if PLATFORM(GTK)
platform/ContextMenu.cpp:81:#if PLATFORM(MAC)
platform/ContextMenu.cpp:88:#if PLATFORM(MAC)
platform/ContextMenu.cpp:93:#if PLATFORM(MAC)
platform/ContextMenu.cpp:100:#if PLATFORM(MAC)
platform/ContextMenu.cpp:151:#if PLATFORM(MAC)
platform/ContextMenu.cpp:166:#if PLATFORM(GTK)
platform/ContextMenu.cpp:299:#if PLATFORM(MAC)
platform/ContextMenu.cpp:323:#if PLATFORM(GTK)
platform/ContextMenu.cpp:333:#if PLATFORM(GTK)
platform/ContextMenu.cpp:367:#if PLATFORM(MAC)
platform/ContextMenu.cpp:372:#if PLATFORM(MAC)
platform/ContextMenu.cpp:379:#if PLATFORM(GTK)
platform/ContextMenu.cpp:457:#if PLATFORM(MAC)
platform/ContextMenu.cpp:463:#if PLATFORM(MAC)
platform/ContextMenu.cpp:472:#if PLATFORM(GTK)
platform/ContextMenu.cpp:495:#if PLATFORM(MAC)
platform/ContextMenu.cpp:501:#if PLATFORM(GTK)
platform/ContextMenu.cpp:590:#if PLATFORM(GTK)
platform/ContextMenu.cpp:652:#if PLATFORM(GTK)

[Alp Toker]

Agreed. ContextMenu.cpp needs to be split up and put into the platform layers, though we still need to allow a mechanism for WebCore to insert menu entries as well.

[Christian Dywan]

Created attachment 21560 [details]
Move webkit code to ContextMenuClient

This patch moves code that needs access to webkit to ContextMenuClientGtk.

[Eric Seidel (no email)]

Comment on attachment 21560 [details]
Move webkit code to ContextMenuClient

Assuming the code still works, the move looks fine.

new WebKit::ContextMenuClient (webView)

seems to have an extra space.

And:
+        WebKitWebView *m_webView;

Is in a WebCore file, but puts the * in the opposite place as the rest of WebCore. :)

It would be nice to land with those to changes, but in general looks fine.

[Adam Roben (:aroben)]

Comment on attachment 21560 [details]
Move webkit code to ContextMenuClient

It seems to me that a better approach would be to move the menu items into ContextMenu{Gtk,Mac,Win,Wx,Qt}.cpp, instead of moving the logic up into WebKit. WebKit's for the most part supposed to be a thin API layer that wraps WebCore, so I don't think moving this kind of logic there is the right direction to go in.

[Christian Dywan]

Landed in r34426, with style fixes as suggested by Eric.

[Christian Dywan]

Comment on attachment 21560 [details]
Move webkit code to ContextMenuClient

Clearing review flag.