diff options
author | estade@chromium.org <estade@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-21 01:21:43 +0000 |
---|---|---|
committer | estade@chromium.org <estade@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-21 01:21:43 +0000 |
commit | 4b32fe696f8a3b84a4feb16f1ce8dfa5d5f41a9a (patch) | |
tree | 98098331aec7763d892ea1def2a2ec90d09b3e55 | |
parent | 2d8bfca747e386fd929c57e014e227b148ac2142 (diff) | |
download | chromium_src-4b32fe696f8a3b84a4feb16f1ce8dfa5d5f41a9a.zip chromium_src-4b32fe696f8a3b84a4feb16f1ce8dfa5d5f41a9a.tar.gz chromium_src-4b32fe696f8a3b84a4feb16f1ce8dfa5d5f41a9a.tar.bz2 |
Info bubble focus tweaks.
We can't close info bubbles when they lose focus because then we can't use the select dropdown in the bookmark bubble. Thus we have to watch the active window. Since we are doing this, we no longer need the application level grab or the button press handler.
This also fixes the problem that a browser window showing an info bubble will paint inactive (assuming your WM supports active/inactive windows).
This maintains the desirable properties:
- info bubbles close when you click outside them but inside the parent window
- info bubbles close when the parent window closes
- info bubbles don't show up on alternate desktops (even if your WM doesn't support active/inactive windows)
- info bubbles can survive losing focus to a popup window
This breaks the desirable properties:
- clicking on something in the browser while the info bubble is showing both dismisses the info bubble and does the appropriate action in the browser
- the info bubble dismisses when the browser loses focus in a window manager that doesn't support active/inactive state
- on xmonad, if you have the cursor over the browser window but the focus is on the info bubble (this is hard to do, since focus is supposed to follow the mouse, but not impossible), then it requires two clicks to dismiss the info bubble. But I think that this is a problem with xmonad and is present before this patch as well.
So this is not the best of all possible worlds, but it is an improvement over our current state as well as an improvement over the state before r20977.
BUG=17223
TEST=browser doesn't crash when you open the select dropdown in the bookmark bubble
Review URL: http://codereview.chromium.org/155793
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@21141 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/gtk/browser_window_gtk.cc | 6 | ||||
-rw-r--r-- | chrome/browser/gtk/info_bubble_gtk.cc | 40 | ||||
-rw-r--r-- | chrome/browser/gtk/info_bubble_gtk.h | 23 |
3 files changed, 51 insertions, 18 deletions
diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc index d3545c2..bff26ed 100644 --- a/chrome/browser/gtk/browser_window_gtk.cc +++ b/chrome/browser/gtk/browser_window_gtk.cc @@ -41,6 +41,7 @@ #include "chrome/browser/gtk/go_button_gtk.h" #include "chrome/browser/gtk/gtk_theme_provider.h" #include "chrome/browser/gtk/import_dialog_gtk.h" +#include "chrome/browser/gtk/info_bubble_gtk.h" #include "chrome/browser/gtk/infobar_container_gtk.h" #include "chrome/browser/gtk/keyword_editor_view.h" #include "chrome/browser/gtk/nine_box.h" @@ -817,7 +818,10 @@ void BrowserWindowGtk::Observe(NotificationType type, break; const GdkWindow* active_window = Details<const GdkWindow>(details).ptr(); - bool is_active = (GTK_WIDGET(window_)->window == active_window); + const GtkWindow* info_bubble_toplevel = + InfoBubbleGtk::GetToplevelForInfoBubble(active_window); + bool is_active = (GTK_WIDGET(window_)->window == active_window || + window_ == info_bubble_toplevel); bool changed = (is_active != is_active_); is_active_ = is_active; if (changed) { diff --git a/chrome/browser/gtk/info_bubble_gtk.cc b/chrome/browser/gtk/info_bubble_gtk.cc index 8c8cbad..cee6ad3 100644 --- a/chrome/browser/gtk/info_bubble_gtk.cc +++ b/chrome/browser/gtk/info_bubble_gtk.cc @@ -12,6 +12,7 @@ #include "base/gfx/gtk_util.h" #include "base/gfx/rect.h" #include "base/logging.h" +#include "chrome/common/notification_service.h" namespace { @@ -33,6 +34,8 @@ const int kRightMargin = kCornerSize + 6; const GdkColor kBackgroundColor = GDK_COLOR_RGB(0xff, 0xff, 0xff); const GdkColor kFrameColor = GDK_COLOR_RGB(0x63, 0x63, 0x63); +const gchar* kInfoBubbleToplevelKey = "__INFO_BUBBLE_TOPLEVEL__"; + // A small convenience since GdkPoint is a POD without a constructor. GdkPoint MakeGdkPoint(gint x, gint y) { GdkPoint point = {x, y}; @@ -133,7 +136,6 @@ InfoBubbleGtk::InfoBubbleGtk() accel_group_(gtk_accel_group_new()), screen_x_(0), screen_y_(0) { - } InfoBubbleGtk::~InfoBubbleGtk() { @@ -192,8 +194,11 @@ void InfoBubbleGtk::Init(GtkWindow* transient_toplevel, G_CALLBACK(&HandleButtonPressThunk), this); g_signal_connect(window_, "destroy", G_CALLBACK(&HandleDestroyThunk), this); - g_signal_connect(window_, "focus-out-event", - G_CALLBACK(&HandleFocusOutThunk), this); + + // Set some data which helps the browser know whether it should appear + // active. + g_object_set_data(G_OBJECT(window_->window), kInfoBubbleToplevelKey, + transient_toplevel); gtk_widget_show_all(window_); // Make sure our window has focus, is brought to the top, etc. @@ -205,6 +210,30 @@ void InfoBubbleGtk::Init(GtkWindow* transient_toplevel, // keystrokes from your window manager, prevent you from interacting with // other applications, etc. gtk_grab_add(window_); + + registrar_.Add(this, NotificationType::ACTIVE_WINDOW_CHANGED, + NotificationService::AllSources()); +} + +void InfoBubbleGtk::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + DCHECK(type.value == NotificationType::ACTIVE_WINDOW_CHANGED); + + // If we are no longer the active toplevel for whatever reason (whether + // another toplevel gained focus or our browser did), close. + if (window_->window != Details<const GdkWindow>(details).ptr()) + Close(); +} + +// static +GtkWindow* InfoBubbleGtk::GetToplevelForInfoBubble( + const GdkWindow* bubble_window){ + if (!bubble_window) + return NULL; + + return reinterpret_cast<GtkWindow*>( + g_object_get_data(G_OBJECT(bubble_window), kInfoBubbleToplevelKey)); } void InfoBubbleGtk::Close(bool closed_by_escape) { @@ -250,8 +279,3 @@ gboolean InfoBubbleGtk::HandleDestroy() { delete this; return FALSE; // Propagate. } - -gboolean InfoBubbleGtk::HandleFocusOut(GdkEventButton* event) { - Close(); - return FALSE; // Propagate. -} diff --git a/chrome/browser/gtk/info_bubble_gtk.h b/chrome/browser/gtk/info_bubble_gtk.h index ac08aa0..2707187 100644 --- a/chrome/browser/gtk/info_bubble_gtk.h +++ b/chrome/browser/gtk/info_bubble_gtk.h @@ -16,6 +16,7 @@ #include <gtk/gtk.h> #include "base/basictypes.h" +#include "chrome/common/notification_registrar.h" class InfoBubbleGtk; namespace gfx { @@ -33,7 +34,7 @@ class InfoBubbleGtkDelegate { // where it ever returns false, so we always allow you to close via escape. }; -class InfoBubbleGtk { +class InfoBubbleGtk : public NotificationObserver { public: // Show an InfoBubble, pointing at the area |rect| (in screen coordinates). // An infobubble will try to fit on the screen, so it can point to any edge @@ -48,6 +49,16 @@ class InfoBubbleGtk { // so you shouldn't hold a InfoBubbleGtk pointer after calling Close(). void Close() { Close(false); } + // NotificationObserver implementation. + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + + // This returns the toplevel GtkWindow that is the transient parent of + // |bubble_window|, or NULL if |bubble_window| isn't the GdkWindow + // for an InfoBubbleGtk. + static GtkWindow* GetToplevelForInfoBubble(const GdkWindow* bubble_window); + private: InfoBubbleGtk(); virtual ~InfoBubbleGtk(); @@ -103,14 +114,6 @@ class InfoBubbleGtk { } gboolean HandleDestroy(); - static gboolean HandleFocusOutThunk(GtkWidget* widget, - GdkEventButton* event, - gpointer userdata) { - return reinterpret_cast<InfoBubbleGtk*>(userdata)-> - HandleFocusOut(event); - } - gboolean HandleFocusOut(GdkEventButton* event); - // The caller supplied delegate, can be NULL. InfoBubbleGtkDelegate* delegate_; @@ -125,6 +128,8 @@ class InfoBubbleGtk { int screen_x_; int screen_y_; + NotificationRegistrar registrar_; + DISALLOW_COPY_AND_ASSIGN(InfoBubbleGtk); }; |