Title: [125528] trunk/Source/WebKit2
Revision
125528
Author
carlo...@webkit.org
Date
2012-08-14 02:06:14 -0700 (Tue, 14 Aug 2012)

Log Message

[GTK] Implement smart separators for context menu in WebKit2 GTK+
https://bugs.webkit.org/show_bug.cgi?id=90449

Reviewed by Martin Robinson.

Don't add to the context menu separators that are at the very
beginning or end of the menu. Once the context menu is shown,
monitor menu items visibility to hide or show separators to make
sure they never appear in the context menu unless they are between
two visible items.

* UIProcess/API/gtk/tests/TestContextMenu.cpp:
(testContextMenuSmartSeparators):
(beforeAll):
* UIProcess/gtk/WebContextMenuProxyGtk.cpp:
(WebKit::contextMenuItemVisibilityChanged):
(WebKit):
(WebKit::WebContextMenuProxyGtk::append):
(WebKit::WebContextMenuProxyGtk::populate):

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (125527 => 125528)


--- trunk/Source/WebKit2/ChangeLog	2012-08-14 09:04:29 UTC (rev 125527)
+++ trunk/Source/WebKit2/ChangeLog	2012-08-14 09:06:14 UTC (rev 125528)
@@ -1,3 +1,25 @@
+2012-08-13  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        [GTK] Implement smart separators for context menu in WebKit2 GTK+
+        https://bugs.webkit.org/show_bug.cgi?id=90449
+
+        Reviewed by Martin Robinson.
+
+        Don't add to the context menu separators that are at the very
+        beginning or end of the menu. Once the context menu is shown,
+        monitor menu items visibility to hide or show separators to make
+        sure they never appear in the context menu unless they are between
+        two visible items.
+
+        * UIProcess/API/gtk/tests/TestContextMenu.cpp:
+        (testContextMenuSmartSeparators):
+        (beforeAll):
+        * UIProcess/gtk/WebContextMenuProxyGtk.cpp:
+        (WebKit::contextMenuItemVisibilityChanged):
+        (WebKit):
+        (WebKit::WebContextMenuProxyGtk::append):
+        (WebKit::WebContextMenuProxyGtk::populate):
+
 2012-08-14  Kihong Kwon  <kihong.k...@samsung.com>
 
         [WK2] Add getter for capture attribute of input element

Modified: trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestContextMenu.cpp (125527 => 125528)


--- trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestContextMenu.cpp	2012-08-14 09:04:29 UTC (rev 125527)
+++ trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestContextMenu.cpp	2012-08-14 09:06:14 UTC (rev 125528)
@@ -74,6 +74,24 @@
         quitMainLoop();
     }
 
+    GtkMenu* getPopupMenu()
+    {
+        GOwnPtr<GList> toplevels(gtk_window_list_toplevels());
+        for (GList* iter = toplevels.get(); iter; iter = g_list_next(iter)) {
+            if (!GTK_IS_WINDOW(iter->data))
+                continue;
+
+            GtkWidget* child = gtk_bin_get_child(GTK_BIN(iter->data));
+            if (!GTK_IS_MENU(child))
+                continue;
+
+            if (gtk_menu_get_attach_widget(GTK_MENU(child)) == GTK_WIDGET(m_webView))
+                return GTK_MENU(child);
+        }
+        g_assert_not_reached();
+        return 0;
+    }
+
     bool shouldShowInputMethodsMenu()
     {
         GtkSettings* settings = gtk_widget_get_settings(GTK_WIDGET(m_webView));
@@ -391,24 +409,6 @@
         return false;
     }
 
-    GtkMenu* getPopupMenu()
-    {
-        GOwnPtr<GList> toplevels(gtk_window_list_toplevels());
-        for (GList* iter = toplevels.get(); iter; iter = g_list_next(iter)) {
-            if (!GTK_IS_WINDOW(iter->data))
-                continue;
-
-            GtkWidget* child = gtk_bin_get_child(GTK_BIN(iter->data));
-            if (!GTK_IS_MENU(child))
-                continue;
-
-            if (gtk_menu_get_attach_widget(GTK_MENU(child)) == GTK_WIDGET(m_webView))
-                return GTK_MENU(child);
-        }
-        g_assert_not_reached();
-        return 0;
-    }
-
     GtkMenuItem* getMenuItem(GtkMenu* menu, const gchar* itemLabel)
     {
         GOwnPtr<GList> items(gtk_container_get_children(GTK_CONTAINER(menu)));
@@ -721,6 +721,115 @@
     g_assert(test->m_dismissed);
 }
 
+class ContextMenuSmartSeparatorsTest: public ContextMenuTest {
+public:
+    MAKE_GLIB_TEST_FIXTURE(ContextMenuSmartSeparatorsTest);
+
+    bool contextMenu(WebKitContextMenu* contextMenu, GdkEvent*, WebKitHitTestResult*)
+    {
+        webkit_context_menu_remove_all(contextMenu);
+
+        webkit_context_menu_append(contextMenu, webkit_context_menu_item_new_separator());
+        webkit_context_menu_append(contextMenu, webkit_context_menu_item_new_separator());
+        webkit_context_menu_append(contextMenu, webkit_context_menu_item_new_from_stock_action(WEBKIT_CONTEXT_MENU_ACTION_GO_BACK));
+        webkit_context_menu_append(contextMenu, webkit_context_menu_item_new_from_stock_action(WEBKIT_CONTEXT_MENU_ACTION_GO_FORWARD));
+        webkit_context_menu_append(contextMenu, webkit_context_menu_item_new_separator());
+        webkit_context_menu_append(contextMenu, webkit_context_menu_item_new_separator());
+        webkit_context_menu_append(contextMenu, webkit_context_menu_item_new_from_stock_action(WEBKIT_CONTEXT_MENU_ACTION_COPY));
+        webkit_context_menu_append(contextMenu, webkit_context_menu_item_new_separator());
+        webkit_context_menu_append(contextMenu, webkit_context_menu_item_new_from_stock_action(WEBKIT_CONTEXT_MENU_ACTION_INSPECT_ELEMENT));
+        webkit_context_menu_append(contextMenu, webkit_context_menu_item_new_separator());
+        webkit_context_menu_append(contextMenu, webkit_context_menu_item_new_separator());
+
+        quitMainLoop();
+
+        return false;
+    }
+
+    GtkMenu* showContextMenuAndGetGtkMenu()
+    {
+        showContextMenuAndWaitUntilFinished();
+        return getPopupMenu();
+    }
+};
+
+static void testContextMenuSmartSeparators(ContextMenuSmartSeparatorsTest* test, gconstpointer)
+{
+    test->showInWindowAndWaitUntilMapped();
+
+    test->loadHtml("<html><body>WebKitGTK+ Context menu tests</body></html>", "file:///");
+    test->waitUntilLoadFinished();
+
+    GtkMenu* menu = test->showContextMenuAndGetGtkMenu();
+    g_assert(menu);
+
+    // Leading and trailing separators are not added to the context menu.
+    GOwnPtr<GList> menuItems(gtk_container_get_children(GTK_CONTAINER(menu)));
+    g_assert_cmpuint(g_list_length(menuItems.get()), ==, 6);
+    GtkWidget* item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 0));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 1));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 2));
+    g_assert(GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 3));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 4));
+    g_assert(GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 5));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+
+    // Hiding a menu item between two separators hides the following separator.
+    GtkAction* action = "" 3)));
+    gtk_action_set_visible(action, FALSE);
+    menuItems.set(gtk_container_get_children(GTK_CONTAINER(menu)));
+    g_assert_cmpuint(g_list_length(menuItems.get()), ==, 6);
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 0));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 1));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 2));
+    g_assert(GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 3));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && !gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 4));
+    g_assert(GTK_IS_SEPARATOR_MENU_ITEM(item) && !gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 5));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    gtk_action_set_visible(action, TRUE);
+
+    // Showing an action between two separators shows the hidden separator.
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 0));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 1));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 2));
+    g_assert(GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 3));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 4));
+    g_assert(GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 5));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+
+    // Trailing separators are hidden too.
+    action = "" 5)));
+    gtk_action_set_visible(action, FALSE);
+    menuItems.set(gtk_container_get_children(GTK_CONTAINER(menu)));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 0));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 1));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 2));
+    g_assert(GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 3));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 4));
+    g_assert(GTK_IS_SEPARATOR_MENU_ITEM(item) && !gtk_widget_get_visible(item));
+    item = GTK_WIDGET(g_list_nth_data(menuItems.get(), 5));
+    g_assert(!GTK_IS_SEPARATOR_MENU_ITEM(item) && !gtk_widget_get_visible(item));
+}
+
 void beforeAll()
 {
     ContextMenuDefaultTest::add("WebKitWebView", "default-menu", testContextMenuDefaultMenu);
@@ -729,6 +838,7 @@
     ContextMenuDisabledTest::add("WebKitWebView", "disable-menu", testContextMenuDisableMenu);
     ContextMenuSubmenuTest::add("WebKitWebView", "submenu", testContextMenuSubMenu);
     ContextMenuDismissedTest::add("WebKitWebView", "menu-dismissed", testContextMenuDismissed);
+    ContextMenuSmartSeparatorsTest::add("WebKitWebView", "smart-separators", testContextMenuSmartSeparators);
 }
 
 void afterAll()

Modified: trunk/Source/WebKit2/UIProcess/gtk/WebContextMenuProxyGtk.cpp (125527 => 125528)


--- trunk/Source/WebKit2/UIProcess/gtk/WebContextMenuProxyGtk.cpp	2012-08-14 09:04:29 UTC (rev 125527)
+++ trunk/Source/WebKit2/UIProcess/gtk/WebContextMenuProxyGtk.cpp	2012-08-14 09:06:14 UTC (rev 125528)
@@ -53,22 +53,78 @@
     page->contextMenuItemSelected(item);
 }
 
+static void contextMenuItemVisibilityChanged(GtkAction* action, GParamSpec*, WebContextMenuProxyGtk* contextMenuProxy)
+{
+    GtkMenu* menu = contextMenuProxy->gtkMenu();
+    if (!menu)
+        return;
+
+    GOwnPtr<GList> items(gtk_container_get_children(GTK_CONTAINER(menu)));
+    bool previousVisibleItemIsNotASeparator = false;
+    GtkWidget* lastItemVisibleSeparator = 0;
+    for (GList* iter = items.get(); iter; iter = g_list_next(iter)) {
+        GtkWidget* widget = GTK_WIDGET(iter->data);
+
+        if (GTK_IS_SEPARATOR_MENU_ITEM(widget)) {
+            if (previousVisibleItemIsNotASeparator) {
+                gtk_widget_show(widget);
+                lastItemVisibleSeparator = widget;
+                previousVisibleItemIsNotASeparator = false;
+            } else
+                gtk_widget_hide(widget);
+        } else if (gtk_widget_get_visible(widget)) {
+            lastItemVisibleSeparator = 0;
+            previousVisibleItemIsNotASeparator = true;
+        }
+    }
+
+    if (lastItemVisibleSeparator)
+        gtk_widget_hide(lastItemVisibleSeparator);
+}
+
 void WebContextMenuProxyGtk::append(ContextMenuItem& menuItem)
 {
     GtkAction* action = ""
-
-    if (action && (menuItem.type() == ActionType || menuItem.type() == CheckableActionType)) {
-        g_object_set_data(G_OBJECT(action), gContextMenuActionId, GINT_TO_POINTER(menuItem.action()));
-        g_signal_connect(action, "activate", G_CALLBACK(contextMenuItemActivatedCallback), m_page);
+    if (action) {
+        switch (menuItem.type()) {
+        case ActionType:
+        case CheckableActionType:
+            g_object_set_data(G_OBJECT(action), gContextMenuActionId, GINT_TO_POINTER(menuItem.action()));
+            g_signal_connect(action, "activate", G_CALLBACK(contextMenuItemActivatedCallback), m_page);
+            // Fall through.
+        case SubmenuType:
+            g_signal_connect(action, "notify::visible", G_CALLBACK(contextMenuItemVisibilityChanged), this);
+            break;
+        case SeparatorType:
+            break;
+        }
     }
 
     m_menu.appendItem(menuItem);
 }
 
+// Populate the context menu ensuring that:
+//  - There aren't separators next to each other.
+//  - There aren't separators at the beginning of the menu.
+//  - There aren't separators at the end of the menu.
 void WebContextMenuProxyGtk::populate(Vector<ContextMenuItem>& items)
 {
-    for (size_t i = 0; i < items.size(); i++)
-        append(items.at(i));
+    bool previousIsSeparator = false;
+    bool isEmpty = true;
+    for (size_t i = 0; i < items.size(); i++) {
+        ContextMenuItem& menuItem = items.at(i);
+        if (menuItem.type() == SeparatorType) {
+            previousIsSeparator = true;
+            continue;
+        }
+
+        if (previousIsSeparator && !isEmpty)
+            append(items.at(i - 1));
+        previousIsSeparator = false;
+
+        append(menuItem);
+        isEmpty = false;
+    }
 }
 
 void WebContextMenuProxyGtk::populate(const Vector<WebContextMenuItemData>& items)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to