Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(711)

Issue 164183: GTK: Make the bookmark bar folder menus dismiss properly.... (Closed)

Created:
11 years, 4 months ago by Evan Stade
Modified:
9 years, 6 months ago
CC:
chromium-reviews_googlegroups.com, Ben Goodger (Google)
Visibility:
Public.

Description

GTK: Make the bookmark bar folder menus dismiss properly. - They will not dismiss when you show and then hide a context menu (unless that context menu initiates certain actions such as "Edit"). - They will dismiss when you click away from them. BUG=18523 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=22820

Patch Set 1 #

Patch Set 2 : better comments #

Patch Set 3 : don't leak #

Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -0 lines) Patch
M chrome/browser/gtk/bookmark_menu_controller_gtk.cc View 1 2 2 chunks +36 lines, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Evan Stade
what I said in the bug report about punting, I decided not to do. It ...
11 years, 4 months ago (2009-08-07 20:51:15 UTC) #1
Elliot Glaysher
11 years, 4 months ago (2009-08-07 20:52:17 UTC) #2
You are crazy. Also, LGTM.

On Fri, Aug 7, 2009 at 1:51 PM, <estade@chromium.org> wrote:
> Reviewers: Elliot Glaysher, willchan,
>
> Message:
> what I said in the bug report about punting, I decided not to do. It
> wasn't that hard to get it right.
>
> Description:
> GTK: Make the bookmark bar folder menus dismiss properly.
>
> - They will not dismiss when you show and then hide a context menu
> (unless that context menu initiates certain actions such as "Edit").
> - They will dismiss when you click away from them.
>
> BUG=3D18523
>
>
> Please review this at http://codereview.chromium.org/164183
>
> SVN Base: svn://chrome-svn/chrome/trunk/src/
>
> Affected files:
> =A0M =A0 =A0 chrome/browser/gtk/bookmark_menu_controller_gtk.cc
>
>
> Index: chrome/browser/gtk/bookmark_menu_controller_gtk.cc
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- chrome/browser/gtk/bookmark_menu_controller_gtk.cc =A0(revision 22744=
)
> +++ chrome/browser/gtk/bookmark_menu_controller_gtk.cc =A0(working copy)
> @@ -48,6 +48,32 @@
> =A0 return const_cast<BookmarkNode*>(node);
> =A0}
>
> +// The context menu has been dismissed, restore the X and application gr=
abs
> +// to whichever menu last had them. (Assuming that menu is still showing=
.)
> +// The event mask in this function is taken from gtkmenu.c.
> +void OnContextMenuHide(GtkWidget* context_menu, GtkWidget* grab_menu) {
> + =A0guint time =3D gtk_get_current_event_time();
> +
> + =A0if (GTK_WIDGET_VISIBLE(grab_menu)) {
> + =A0 =A0if (!gdk_pointer_grab(grab_menu->window, TRUE,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0GdkEventMask(
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0GDK_BUTTON_PRESS_MAS=
K | GDK_BUTTON_RELEASE_MASK |
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0GDK_ENTER_NOTIFY_MAS=
K | GDK_LEAVE_NOTIFY_MASK |
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0GDK_POINTER_MOTION_M=
ASK), NULL, NULL, time) =3D=3D 0)
> {
> + =A0 =A0 =A0return;
> + =A0 =A0}
> + =A0 =A0if (!gdk_keyboard_grab(grab_menu->window, TRUE, time) =3D=3D 0) =
{
> +
> =A0gdk_display_pointer_ungrab(gdk_drawable_get_display(grab_menu->window)=
,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 time);
> + =A0 =A0 =A0return;
> + =A0 =A0}
> + =A0 =A0gtk_grab_add(grab_menu);
> + =A0}
> +
> + =A0// Match the ref we took when connecting this signal.
> + =A0g_object_unref(grab_menu);
> +}
> +
> =A0} =A0// namespace
>
> =A0BookmarkMenuController::BookmarkMenuController(Browser* browser,
> @@ -216,6 +242,14 @@
> =A0 =A0 =A0 =A0 =A0 =A0 sender, controller->profile_, controller->browser=
_,
> =A0 =A0 =A0 =A0 =A0 =A0 controller->page_navigator_, parent, nodes,
> =A0 =A0 =A0 =A0 =A0 =A0 BookmarkContextMenu::BOOKMARK_BAR));
> +
> + =A0 =A0// Our bookmark folder menu loses the grab to the context menu. =
When
> the
> + =A0 =A0// context menu is hidden, re-assert our grab.
> + =A0 =A0GtkWidget* grabbing_menu =3D gtk_grab_get_current();
> + =A0 =A0g_object_ref(grabbing_menu);
> + =A0 =A0g_signal_connect(controller->context_menu_->menu(), "hide",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 G_CALLBACK(OnContextMenuHide), =
grabbing_menu);
> +
> =A0 =A0 controller->context_menu_->PopupAsContext(event->time);
> =A0 =A0 return TRUE;
> =A0 }
>
>
>

Powered by Google App Engine
This is Rietveld 408576698