From 40c9ca7eee49e301a60ea55b299d8e04934096cb Mon Sep 17 00:00:00 2001 From: pelatx Date: Mon, 19 Feb 2018 02:08:38 +0100 Subject: [PATCH 01/10] Checked menu entries working. --- appshell/appshell_extensions_gtk.cpp | 35 +++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/appshell/appshell_extensions_gtk.cpp b/appshell/appshell_extensions_gtk.cpp index f101c513e..98b99dd6b 100644 --- a/appshell/appshell_extensions_gtk.cpp +++ b/appshell/appshell_extensions_gtk.cpp @@ -1100,7 +1100,6 @@ int32 GetMenuItemState(CefRefPtr browser, ExtensionString commandId, int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, bool& enabled, bool& checked) { - // TODO: Implement functionality for checked NativeMenuModel& model = NativeMenuModel::getInstance(getMenuParent(browser)); int tag = model.getTag(command); if (tag == kTagNotFound) { @@ -1108,6 +1107,40 @@ int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, b } GtkWidget* menuItem = (GtkWidget*) model.getOsItem(tag); gtk_widget_set_sensitive(menuItem, enabled); + + // Functionality for checked + gint position; + GtkWidget* checkedMenuItem; + GtkWidget* parent = gtk_widget_get_parent(menuItem); + + if (GTK_IS_CONTAINER(parent)) { + GList* children = gtk_container_get_children(GTK_CONTAINER(parent)); + gint index = 0; + do { + GtkWidget* widget = (GtkWidget*) children->data; + if (widget == menuItem) + position = index + 1; + if (index == position && GTK_IS_CHECK_MENU_ITEM(widget)) + checkedMenuItem = widget; + index++; + } while ((children = g_list_next(children)) != NULL); + g_list_free(children); + } + + if (checked == true) { + const gchar* label = gtk_menu_item_get_label(GTK_MENU_ITEM(menuItem)); + checkedMenuItem = gtk_check_menu_item_new_with_label(label); + gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(checkedMenuItem), true); + gtk_widget_set_sensitive(checkedMenuItem, enabled); + g_signal_connect(checkedMenuItem, "activate", G_CALLBACK(CheckedItemCallbach), menuItem); + gtk_menu_shell_insert(GTK_MENU_SHELL(parent), checkedMenuItem, position); + gtk_widget_show(checkedMenuItem); + gtk_widget_hide(menuItem); + } else { + gtk_widget_show(menuItem); + if (GTK_IS_WIDGET(checkedMenuItem)) + gtk_widget_destroy(checkedMenuItem); + } return NO_ERROR; } From e3f6e007f0e193b51e948050038cf995aa58eb63 Mon Sep 17 00:00:00 2001 From: pelatx Date: Mon, 19 Feb 2018 02:33:39 +0100 Subject: [PATCH 02/10] Forgotten callback for checked entry functionality. --- appshell/appshell_extensions_gtk.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/appshell/appshell_extensions_gtk.cpp b/appshell/appshell_extensions_gtk.cpp index 98b99dd6b..0fcdba125 100644 --- a/appshell/appshell_extensions_gtk.cpp +++ b/appshell/appshell_extensions_gtk.cpp @@ -1098,6 +1098,10 @@ int32 GetMenuItemState(CefRefPtr browser, ExtensionString commandId, return NO_ERROR; } +void CheckedItemCallbach(GtkWidget* checkMenuItem, GtkWidget* originalMenuItem) { + gtk_menu_item_activate(GTK_MENU_ITEM(originalMenuItem)); +} + int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, bool& enabled, bool& checked) { NativeMenuModel& model = NativeMenuModel::getInstance(getMenuParent(browser)); From 651d58780aae36235ad5ff1332d3804b25e1fdd8 Mon Sep 17 00:00:00 2001 From: pelatx Date: Wed, 21 Feb 2018 19:20:50 +0100 Subject: [PATCH 03/10] Fixes post insertion of new menu item between the original menu item and the checked one. --- appshell/appshell_extensions_gtk.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/appshell/appshell_extensions_gtk.cpp b/appshell/appshell_extensions_gtk.cpp index 0fcdba125..909d66aa9 100644 --- a/appshell/appshell_extensions_gtk.cpp +++ b/appshell/appshell_extensions_gtk.cpp @@ -1113,37 +1113,42 @@ int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, b gtk_widget_set_sensitive(menuItem, enabled); // Functionality for checked - gint position; + gint checkedItemPos = -1; GtkWidget* checkedMenuItem; GtkWidget* parent = gtk_widget_get_parent(menuItem); + const gchar* label = gtk_menu_item_get_label(GTK_MENU_ITEM(menuItem)); if (GTK_IS_CONTAINER(parent)) { GList* children = gtk_container_get_children(GTK_CONTAINER(parent)); gint index = 0; do { GtkWidget* widget = (GtkWidget*) children->data; - if (widget == menuItem) - position = index + 1; - if (index == position && GTK_IS_CHECK_MENU_ITEM(widget)) + const gchar* widgetLabel = gtk_menu_item_get_label(GTK_MENU_ITEM(widget)); + if (widget == menuItem) { + checkedItemPos = index + 1; + } + if (index >= checkedItemPos && GTK_IS_CHECK_MENU_ITEM(widget) && strcmp(widgetLabel, label) == 0) { checkedMenuItem = widget; + break; + } index++; } while ((children = g_list_next(children)) != NULL); g_list_free(children); } if (checked == true) { - const gchar* label = gtk_menu_item_get_label(GTK_MENU_ITEM(menuItem)); checkedMenuItem = gtk_check_menu_item_new_with_label(label); gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(checkedMenuItem), true); gtk_widget_set_sensitive(checkedMenuItem, enabled); g_signal_connect(checkedMenuItem, "activate", G_CALLBACK(CheckedItemCallbach), menuItem); - gtk_menu_shell_insert(GTK_MENU_SHELL(parent), checkedMenuItem, position); + gtk_menu_shell_insert(GTK_MENU_SHELL(parent), checkedMenuItem, checkedItemPos); gtk_widget_show(checkedMenuItem); gtk_widget_hide(menuItem); } else { gtk_widget_show(menuItem); - if (GTK_IS_WIDGET(checkedMenuItem)) + if (GTK_IS_CHECK_MENU_ITEM(checkedMenuItem)) { gtk_widget_destroy(checkedMenuItem); + } } return NO_ERROR; } From ca7da2192a6ca698beb54ce57cbc94616f9e85f0 Mon Sep 17 00:00:00 2001 From: pelatx Date: Fri, 23 Feb 2018 16:05:11 +0100 Subject: [PATCH 04/10] Fixes visual malfunction (can be seen unchecked) of the split view items when the one that is ckecked at that moment is clicked. --- appshell/appshell_extensions_gtk.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/appshell/appshell_extensions_gtk.cpp b/appshell/appshell_extensions_gtk.cpp index 909d66aa9..ead82be27 100644 --- a/appshell/appshell_extensions_gtk.cpp +++ b/appshell/appshell_extensions_gtk.cpp @@ -1098,8 +1098,12 @@ int32 GetMenuItemState(CefRefPtr browser, ExtensionString commandId, return NO_ERROR; } -void CheckedItemCallbach(GtkWidget* checkMenuItem, GtkWidget* originalMenuItem) { - gtk_menu_item_activate(GTK_MENU_ITEM(originalMenuItem)); +void CheckedItemCallback(GtkWidget* checkMenuItem, GtkWidget* originalMenuItem) { + if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(checkMenuItem))) { + gtk_menu_item_activate(GTK_MENU_ITEM(originalMenuItem)); + } else { + gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(checkMenuItem), true); + } } int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, bool& enabled, bool& checked) @@ -1140,7 +1144,7 @@ int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, b checkedMenuItem = gtk_check_menu_item_new_with_label(label); gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(checkedMenuItem), true); gtk_widget_set_sensitive(checkedMenuItem, enabled); - g_signal_connect(checkedMenuItem, "activate", G_CALLBACK(CheckedItemCallbach), menuItem); + g_signal_connect(checkedMenuItem, "activate", G_CALLBACK(CheckedItemCallback), menuItem); gtk_menu_shell_insert(GTK_MENU_SHELL(parent), checkedMenuItem, checkedItemPos); gtk_widget_show(checkedMenuItem); gtk_widget_hide(menuItem); From 5e6f8a282d8c2cfcb1124921ad0c78e72e103d54 Mon Sep 17 00:00:00 2001 From: pelatx Date: Sat, 24 Feb 2018 08:42:27 +0100 Subject: [PATCH 05/10] Check menu item is created the first time that SetMenuItemState() is called with checked flag to true and reutilized later. --- appshell/appshell_extensions_gtk.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/appshell/appshell_extensions_gtk.cpp b/appshell/appshell_extensions_gtk.cpp index ead82be27..f91560268 100644 --- a/appshell/appshell_extensions_gtk.cpp +++ b/appshell/appshell_extensions_gtk.cpp @@ -1141,17 +1141,19 @@ int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, b } if (checked == true) { - checkedMenuItem = gtk_check_menu_item_new_with_label(label); - gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(checkedMenuItem), true); + if (GTK_IS_CHECK_MENU_ITEM(checkedMenuItem)) { + checkedMenuItem = gtk_check_menu_item_new_with_label(label); + gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(checkedMenuItem), true); + g_signal_connect(checkedMenuItem, "activate", G_CALLBACK(CheckedItemCallback), menuItem); + gtk_menu_shell_insert(GTK_MENU_SHELL(parent), checkedMenuItem, checkedItemPos); + } gtk_widget_set_sensitive(checkedMenuItem, enabled); - g_signal_connect(checkedMenuItem, "activate", G_CALLBACK(CheckedItemCallback), menuItem); - gtk_menu_shell_insert(GTK_MENU_SHELL(parent), checkedMenuItem, checkedItemPos); gtk_widget_show(checkedMenuItem); gtk_widget_hide(menuItem); } else { gtk_widget_show(menuItem); if (GTK_IS_CHECK_MENU_ITEM(checkedMenuItem)) { - gtk_widget_destroy(checkedMenuItem); + gtk_widget_hide(checkedMenuItem); } } return NO_ERROR; From c5e80e4be4424ac40445d86c36508f20826ca280 Mon Sep 17 00:00:00 2001 From: pelatx Date: Sun, 25 Feb 2018 06:47:16 +0100 Subject: [PATCH 06/10] Typo fix. --- appshell/appshell_extensions_gtk.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appshell/appshell_extensions_gtk.cpp b/appshell/appshell_extensions_gtk.cpp index f91560268..4d7ad4f6f 100644 --- a/appshell/appshell_extensions_gtk.cpp +++ b/appshell/appshell_extensions_gtk.cpp @@ -1141,7 +1141,7 @@ int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, b } if (checked == true) { - if (GTK_IS_CHECK_MENU_ITEM(checkedMenuItem)) { + if (!GTK_IS_CHECK_MENU_ITEM(checkedMenuItem)) { checkedMenuItem = gtk_check_menu_item_new_with_label(label); gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(checkedMenuItem), true); g_signal_connect(checkedMenuItem, "activate", G_CALLBACK(CheckedItemCallback), menuItem); From 24218ded336ea189e352e2ac176ed2e792f6d64a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20A=2E=20Mart=C3=ADnez?= Date: Fri, 2 Mar 2018 09:59:49 +0100 Subject: [PATCH 07/10] New approach that simplifies the code. --- appshell/appshell_extensions_gtk.cpp | 69 ++++++++++++---------------- 1 file changed, 29 insertions(+), 40 deletions(-) diff --git a/appshell/appshell_extensions_gtk.cpp b/appshell/appshell_extensions_gtk.cpp index 4d7ad4f6f..c05965ab5 100644 --- a/appshell/appshell_extensions_gtk.cpp +++ b/appshell/appshell_extensions_gtk.cpp @@ -1098,12 +1098,20 @@ int32 GetMenuItemState(CefRefPtr browser, ExtensionString commandId, return NO_ERROR; } -void CheckedItemCallback(GtkWidget* checkMenuItem, GtkWidget* originalMenuItem) { - if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(checkMenuItem))) { - gtk_menu_item_activate(GTK_MENU_ITEM(originalMenuItem)); - } else { - gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(checkMenuItem), true); - } +int _getMenuItemPosition(GtkWidget* parent, GtkWidget* menuItem) +{ + GList* children = gtk_container_get_children(GTK_CONTAINER(parent)); + int index = 0; + do { + GtkWidget* widget = (GtkWidget*) children->data; + if (widget == menuItem) { + return index; + } + index++; + } while ((children = g_list_next(children)) != NULL); + g_list_free(children); + + return -1; } int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, bool& enabled, bool& checked) @@ -1117,45 +1125,26 @@ int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, b gtk_widget_set_sensitive(menuItem, enabled); // Functionality for checked - gint checkedItemPos = -1; - GtkWidget* checkedMenuItem; GtkWidget* parent = gtk_widget_get_parent(menuItem); + int position = _getMenuItemPosition(parent, menuItem); const gchar* label = gtk_menu_item_get_label(GTK_MENU_ITEM(menuItem)); - if (GTK_IS_CONTAINER(parent)) { - GList* children = gtk_container_get_children(GTK_CONTAINER(parent)); - gint index = 0; - do { - GtkWidget* widget = (GtkWidget*) children->data; - const gchar* widgetLabel = gtk_menu_item_get_label(GTK_MENU_ITEM(widget)); - if (widget == menuItem) { - checkedItemPos = index + 1; - } - if (index >= checkedItemPos && GTK_IS_CHECK_MENU_ITEM(widget) && strcmp(widgetLabel, label) == 0) { - checkedMenuItem = widget; - break; - } - index++; - } while ((children = g_list_next(children)) != NULL); - g_list_free(children); - } - + GtkWidget* newMenuItem; if (checked == true) { - if (!GTK_IS_CHECK_MENU_ITEM(checkedMenuItem)) { - checkedMenuItem = gtk_check_menu_item_new_with_label(label); - gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(checkedMenuItem), true); - g_signal_connect(checkedMenuItem, "activate", G_CALLBACK(CheckedItemCallback), menuItem); - gtk_menu_shell_insert(GTK_MENU_SHELL(parent), checkedMenuItem, checkedItemPos); - } - gtk_widget_set_sensitive(checkedMenuItem, enabled); - gtk_widget_show(checkedMenuItem); - gtk_widget_hide(menuItem); - } else { - gtk_widget_show(menuItem); - if (GTK_IS_CHECK_MENU_ITEM(checkedMenuItem)) { - gtk_widget_hide(checkedMenuItem); - } + newMenuItem = gtk_check_menu_item_new_with_label(label); + gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(newMenuItem), true); + } else if (checked == false){ + newMenuItem = gtk_menu_item_new_with_label(label); } + + gtk_widget_destroy(menuItem); + + InstallMenuHandler(newMenuItem, browser, tag); + model.setOsItem(tag, newMenuItem); + gtk_menu_shell_insert(GTK_MENU_SHELL(parent), newMenuItem, position); + gtk_widget_set_sensitive(newMenuItem, enabled); + gtk_widget_show(newMenuItem); + return NO_ERROR; } From d6834b403ccd65c3bd344e0d677ec79176788b91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20A=2E=20Mart=C3=ADnez?= Date: Sat, 3 Mar 2018 07:00:04 +0100 Subject: [PATCH 08/10] Removed unnecessary call to gtk_widget_set_sensitive(). --- appshell/appshell_extensions_gtk.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/appshell/appshell_extensions_gtk.cpp b/appshell/appshell_extensions_gtk.cpp index c05965ab5..3458971b9 100644 --- a/appshell/appshell_extensions_gtk.cpp +++ b/appshell/appshell_extensions_gtk.cpp @@ -1122,9 +1122,6 @@ int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, b return ERR_NOT_FOUND; } GtkWidget* menuItem = (GtkWidget*) model.getOsItem(tag); - gtk_widget_set_sensitive(menuItem, enabled); - - // Functionality for checked GtkWidget* parent = gtk_widget_get_parent(menuItem); int position = _getMenuItemPosition(parent, menuItem); const gchar* label = gtk_menu_item_get_label(GTK_MENU_ITEM(menuItem)); @@ -1136,7 +1133,6 @@ int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, b } else if (checked == false){ newMenuItem = gtk_menu_item_new_with_label(label); } - gtk_widget_destroy(menuItem); InstallMenuHandler(newMenuItem, browser, tag); From db7e018983348ad7ad989c6bda438da337f6fa00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20A=2E=20Mart=C3=ADnez?= Date: Thu, 8 Mar 2018 00:01:19 +0100 Subject: [PATCH 09/10] Added @nethip hack to avoid seeing the checkbox disabled in `No Split`, etc. And addressing review comments. --- appshell/appshell_extensions_gtk.cpp | 3 +- appshell/appshell_extensions_platform.h | 14 ++++++++ appshell/browser/root_window_gtk.cc | 44 ++++++++++++++++++++++--- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/appshell/appshell_extensions_gtk.cpp b/appshell/appshell_extensions_gtk.cpp index 3458971b9..7f9152972 100644 --- a/appshell/appshell_extensions_gtk.cpp +++ b/appshell/appshell_extensions_gtk.cpp @@ -95,6 +95,8 @@ typedef char s8; +bool StMenuCommandSkipper::sSkipMenuCommand = false; + extern CefRefPtr g_handler; // Supported browsers (order matters): @@ -1109,7 +1111,6 @@ int _getMenuItemPosition(GtkWidget* parent, GtkWidget* menuItem) } index++; } while ((children = g_list_next(children)) != NULL); - g_list_free(children); return -1; } diff --git a/appshell/appshell_extensions_platform.h b/appshell/appshell_extensions_platform.h index b2cea7b51..843bbbc67 100644 --- a/appshell/appshell_extensions_platform.h +++ b/appshell/appshell_extensions_platform.h @@ -108,6 +108,20 @@ class CharSetEncode void operator()(std::string &contents); }; +#if defined(OS_LINUX) +class StMenuCommandSkipper { +public: + + StMenuCommandSkipper() { sSkipMenuCommand = true; } + ~StMenuCommandSkipper() { sSkipMenuCommand = false;} + + static bool GetMenuCmdSkipFlag () { return sSkipMenuCommand;} + +private: + static bool sSkipMenuCommand; +}; +#endif + #if defined(OS_MACOSX) || defined(OS_LINUX) void DecodeContents(std::string &contents, const std::string& encoding); #endif diff --git a/appshell/browser/root_window_gtk.cc b/appshell/browser/root_window_gtk.cc index d566bbdd3..60f66302e 100644 --- a/appshell/browser/root_window_gtk.cc +++ b/appshell/browser/root_window_gtk.cc @@ -601,13 +601,49 @@ void RootWindowGtk::MenubarSizeAllocated(GtkWidget* widget, self->menubar_height_ = allocation->height; } +// Brackets specific change. +// GTK is toggling the menu state just by +// clicking on the menu entry. So posting a task to undo that +// unwanted side effect. This is a @nethip hack. +void DelayedMenuMarkToggle(GtkWidget *menuItem){ + + if (GTK_IS_CHECK_MENU_ITEM(menuItem)) { + // It is important to make sure our MenuItemACtivated + // knows that we are only changing the state and not + // firing any command as such. + StMenuCommandSkipper skipCmd; + + // Get the current state of the menu. + gboolean menuState = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuItem)); + + // Now go about toggling the state. If Brackets is triggering a menu state change, + // that will get executed later than DelayedMenuMarkToggle, as this task would have been + // posted prior to shell call to fire a command, so need not worry about this function + // getting executed after the shell call. + gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuItem), !menuState); + } +} + // static gboolean RootWindowGtk::MenuItemActivated(GtkWidget* widget, RootWindowGtk* self) { - // Retrieve the menu ID set in AddMenuEntry. - int tag = GPOINTER_TO_INT(g_object_get_data(G_OBJECT(widget), kMenuIdKey)); - if (self && self->browser_window_) - self->browser_window_->DispatchCommandToBrowser(self->GetBrowser(), tag); + // Since we have migrated to checked menu items, setting the + // state of the menu is triggering a menu activation. So made + // sure we actually execute a command only when is menu is + // actually activated by the user and not when setting the menu + // state. + if(!StMenuCommandSkipper::GetMenuCmdSkipFlag()){ + + // Post a message to undo the undesired menu state change. + // See the comments above for explanation. + CefPostTask(TID_UI, base::Bind(&DelayedMenuMarkToggle, widget)); + + // Retrieve the menu ID set in AddMenuEntry. + int tag = GPOINTER_TO_INT(g_object_get_data(G_OBJECT(widget), kMenuIdKey)); + if (self && self->browser_window_) + self->browser_window_->DispatchCommandToBrowser(self->GetBrowser(), tag); + } + } // static From 5d821c02ad4d283daa80e6eaa7fec3122c162878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20A=2E=20Mart=C3=ADnez?= Date: Thu, 8 Mar 2018 09:39:59 +0100 Subject: [PATCH 10/10] Added some checks addressing review comments. --- appshell/appshell_extensions_gtk.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/appshell/appshell_extensions_gtk.cpp b/appshell/appshell_extensions_gtk.cpp index 7f9152972..43136c475 100644 --- a/appshell/appshell_extensions_gtk.cpp +++ b/appshell/appshell_extensions_gtk.cpp @@ -1123,8 +1123,17 @@ int32 SetMenuItemState(CefRefPtr browser, ExtensionString command, b return ERR_NOT_FOUND; } GtkWidget* menuItem = (GtkWidget*) model.getOsItem(tag); + if (menuItem == NULL) { + return ERR_UNKNOWN; + } GtkWidget* parent = gtk_widget_get_parent(menuItem); + if (parent == NULL) { + return ERR_UNKNOWN; + } int position = _getMenuItemPosition(parent, menuItem); + if (position < 0) { + return ERR_UNKNOWN; + } const gchar* label = gtk_menu_item_get_label(GTK_MENU_ITEM(menuItem)); GtkWidget* newMenuItem;