diff options
19 files changed, 125 insertions, 41 deletions
diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc index be40c97..a9c38fd 100644 --- a/ash/accelerators/accelerator_controller.cc +++ b/ash/accelerators/accelerator_controller.cc @@ -203,4 +203,8 @@ bool AcceleratorController::AcceleratorPressed( return false; } +bool AcceleratorController::CanHandleAccelerators() const { + return true; +} + } // namespace ash diff --git a/ash/accelerators/accelerator_controller.h b/ash/accelerators/accelerator_controller.h index c3d83ae..55c3ed5 100644 --- a/ash/accelerators/accelerator_controller.h +++ b/ash/accelerators/accelerator_controller.h @@ -53,6 +53,7 @@ class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget { // Overridden from ui::AcceleratorTarget: virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE; + virtual bool CanHandleAccelerators() const OVERRIDE; void SetScreenshotDelegate(ScreenshotDelegate* screenshot_delegate); diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc index 9ac406e..68a210b 100644 --- a/ash/accelerators/accelerator_controller_unittest.cc +++ b/ash/accelerators/accelerator_controller_unittest.cc @@ -38,6 +38,7 @@ class TestTarget : public ui::AcceleratorTarget { // Overridden from ui::AcceleratorTarget: virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE; + virtual bool CanHandleAccelerators() const OVERRIDE; private: int accelerator_pressed_count_; @@ -48,6 +49,10 @@ bool TestTarget::AcceleratorPressed(const ui::Accelerator& accelerator) { return true; } +bool TestTarget::CanHandleAccelerators() const { + return true; +} + } // namespace class AcceleratorControllerTest : public AuraShellTestBase { diff --git a/chrome/browser/chromeos/login/screen_locker_views.cc b/chrome/browser/chromeos/login/screen_locker_views.cc index 3041876..ec5e8f2 100644 --- a/chrome/browser/chromeos/login/screen_locker_views.cc +++ b/chrome/browser/chromeos/login/screen_locker_views.cc @@ -891,6 +891,10 @@ bool ScreenLockerViews::AcceleratorPressed( return false; } +bool ScreenLockerViews::CanHandleAccelerators() const { + return true; +} + void ScreenLockerViews::OnClientEvent(GtkWidget* widge, GdkEventClient* event) { #if defined(TOOLKIT_USES_GTK) WmIpc::Message msg; diff --git a/chrome/browser/chromeos/login/screen_locker_views.h b/chrome/browser/chromeos/login/screen_locker_views.h index f2b3f99..04c1e09 100644 --- a/chrome/browser/chromeos/login/screen_locker_views.h +++ b/chrome/browser/chromeos/login/screen_locker_views.h @@ -93,6 +93,7 @@ class ScreenLockerViews : public ScreenLockerDelegate, // Overridden from ui::AcceleratorTarget: virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE; + virtual bool CanHandleAccelerators() const OVERRIDE; // Event handler for client-event. CHROMEGTK_CALLBACK_1(ScreenLockerViews, void, OnClientEvent, GdkEventClient*); diff --git a/chrome/browser/external_tab_container_win.cc b/chrome/browser/external_tab_container_win.cc index 99591be..d29bc5e 100644 --- a/chrome/browser/external_tab_container_win.cc +++ b/chrome/browser/external_tab_container_win.cc @@ -1049,6 +1049,10 @@ bool ExternalTabContainer::AcceleratorPressed( return true; } +bool ExternalTabContainer::CanHandleAccelerators() const { + return true; +} + void ExternalTabContainer::Navigate(const GURL& url, const GURL& referrer) { if (!tab_contents_.get()) { NOTREACHED(); diff --git a/chrome/browser/external_tab_container_win.h b/chrome/browser/external_tab_container_win.h index 02f2915..8f41759 100644 --- a/chrome/browser/external_tab_container_win.h +++ b/chrome/browser/external_tab_container_win.h @@ -202,8 +202,9 @@ class ExternalTabContainer : public content::WebContentsDelegate, // Returns NULL if we fail to find the cookie in the map. static scoped_refptr<ExternalTabContainer> RemovePendingTab(uintptr_t cookie); - // Handles the specified |accelerator| being pressed. - bool AcceleratorPressed(const ui::Accelerator& accelerator); + // ui::AcceleratorTarget + bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE; + bool CanHandleAccelerators() const OVERRIDE; bool pending() const { return pending_; diff --git a/chrome/browser/ui/views/constrained_window_views_browsertest.cc b/chrome/browser/ui/views/constrained_window_views_browsertest.cc index ad1fd7c..4b62f54 100644 --- a/chrome/browser/ui/views/constrained_window_views_browsertest.cc +++ b/chrome/browser/ui/views/constrained_window_views_browsertest.cc @@ -119,11 +119,6 @@ IN_PROC_BROWSER_TEST_F(ConstrainedWindowViewTest, FocusTest) { views::FocusManager* focus_manager = window1->GetFocusManager(); ASSERT_TRUE(focus_manager); - // old_target should be the OK button for test_dialog1. - ui::AcceleratorTarget* old_target = - focus_manager->GetCurrentTargetForAccelerator( - ui::Accelerator(ui::VKEY_RETURN, false, false, false)); - ASSERT_TRUE(old_target != NULL); // test_dialog1's text field should be focused. EXPECT_EQ(test_dialog1->GetInitiallyFocusedView(), focus_manager->GetFocusedView()); @@ -137,14 +132,6 @@ IN_PROC_BROWSER_TEST_F(ConstrainedWindowViewTest, FocusTest) { // Should be the same focus_manager. ASSERT_EQ(focus_manager, window2->GetFocusManager()); - // new_target should be the same as old_target since test_dialog2 is still - // hidden. - ui::AcceleratorTarget* new_target = - focus_manager->GetCurrentTargetForAccelerator( - ui::Accelerator(ui::VKEY_RETURN, false, false, false)); - ASSERT_TRUE(new_target != NULL); - EXPECT_EQ(old_target, new_target); - // test_dialog1's text field should still be the view that has focus. EXPECT_EQ(test_dialog1->GetInitiallyFocusedView(), focus_manager->GetFocusedView()); @@ -164,6 +151,21 @@ IN_PROC_BROWSER_TEST_F(ConstrainedWindowViewTest, FocusTest) { EXPECT_EQ(test_dialog2->GetInitiallyFocusedView(), focus_manager->GetFocusedView()); + int tab_with_constrained_window = browser()->active_index(); + + // Create a new tab. + browser()->NewTab(); + + // The constrained dialog should no longer be selected. + EXPECT_NE(test_dialog2->GetInitiallyFocusedView(), + focus_manager->GetFocusedView()); + + browser()->ActivateTabAt(tab_with_constrained_window, false); + + // Activating the previous tab should bring focus to the constrained window. + EXPECT_EQ(test_dialog2->GetInitiallyFocusedView(), + focus_manager->GetFocusedView()); + // Send another VKEY_RETURN, closing test_dialog2 EXPECT_TRUE(focus_manager->ProcessAccelerator( ui::Accelerator(ui::VKEY_RETURN, false, false, false))); diff --git a/chrome/browser/ui/views/dropdown_bar_host.h b/chrome/browser/ui/views/dropdown_bar_host.h index ba809dd..f6fdb6c 100644 --- a/chrome/browser/ui/views/dropdown_bar_host.h +++ b/chrome/browser/ui/views/dropdown_bar_host.h @@ -79,6 +79,7 @@ class DropdownBarHost : public ui::AcceleratorTarget, // Overridden from ui::AcceleratorTarget: virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) = 0; + virtual bool CanHandleAccelerators() const = 0; // ui::AnimationDelegate implementation: virtual void AnimationProgressed(const ui::Animation* animation) OVERRIDE; diff --git a/chrome/browser/ui/views/find_bar_host.cc b/chrome/browser/ui/views/find_bar_host.cc index d6c0851..2324bf2 100644 --- a/chrome/browser/ui/views/find_bar_host.cc +++ b/chrome/browser/ui/views/find_bar_host.cc @@ -188,6 +188,10 @@ bool FindBarHost::AcceleratorPressed(const ui::Accelerator& accelerator) { return true; } +bool FindBarHost::CanHandleAccelerators() const { + return true; +} + //////////////////////////////////////////////////////////////////////////////// // FindBarTesting implementation: diff --git a/chrome/browser/ui/views/find_bar_host.h b/chrome/browser/ui/views/find_bar_host.h index 20d4f98..b3a1834 100644 --- a/chrome/browser/ui/views/find_bar_host.h +++ b/chrome/browser/ui/views/find_bar_host.h @@ -68,6 +68,7 @@ class FindBarHost : public DropdownBarHost, // Overridden from ui::AcceleratorTarget in DropdownBarHost class: virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE; + virtual bool CanHandleAccelerators() const OVERRIDE; // FindBarTesting implementation: virtual bool GetFindBarWindowInfo(gfx::Point* position, diff --git a/ui/base/accelerators/accelerator.h b/ui/base/accelerators/accelerator.h index 5977c8c2..7fa1fbb 100644 --- a/ui/base/accelerators/accelerator.h +++ b/ui/base/accelerators/accelerator.h @@ -103,9 +103,14 @@ class UI_EXPORT Accelerator { // should implement. class UI_EXPORT AcceleratorTarget { public: - // This method should return true if the accelerator was processed. + // Should return true if the accelerator was processed. virtual bool AcceleratorPressed(const Accelerator& accelerator) = 0; + // Should return true if the target can handle the accelerator events. The + // AcceleratorPressed method is inovked only for targets for which + // CanHandleAccelerators returns true. + virtual bool CanHandleAccelerators() const = 0; + protected: virtual ~AcceleratorTarget() {} }; diff --git a/ui/base/accelerators/accelerator_manager.cc b/ui/base/accelerators/accelerator_manager.cc index c675211..86d44ea 100644 --- a/ui/base/accelerators/accelerator_manager.cc +++ b/ui/base/accelerators/accelerator_manager.cc @@ -59,7 +59,8 @@ bool AcceleratorManager::Process(const Accelerator& accelerator) { AcceleratorTargetList targets(map_iter->second); for (AcceleratorTargetList::iterator iter = targets.begin(); iter != targets.end(); ++iter) { - if ((*iter)->AcceleratorPressed(accelerator)) + if ((*iter)->CanHandleAccelerators() && + (*iter)->AcceleratorPressed(accelerator)) return true; } } diff --git a/ui/views/controls/button/custom_button.cc b/ui/views/controls/button/custom_button.cc index 74535ba..ead5583 100644 --- a/ui/views/controls/button/custom_button.cc +++ b/ui/views/controls/button/custom_button.cc @@ -194,9 +194,6 @@ bool CustomButton::OnKeyReleased(const KeyEvent& event) { } bool CustomButton::AcceleratorPressed(const ui::Accelerator& accelerator) { - if (!enabled()) - return false; - SetState(BS_NORMAL); KeyEvent key_event(ui::ET_KEY_RELEASED, accelerator.key_code(), accelerator.modifiers()); diff --git a/ui/views/focus/accelerator_handler_gtk_unittest.cc b/ui/views/focus/accelerator_handler_gtk_unittest.cc index d4db6f5..6ad2697 100644 --- a/ui/views/focus/accelerator_handler_gtk_unittest.cc +++ b/ui/views/focus/accelerator_handler_gtk_unittest.cc @@ -66,6 +66,9 @@ class AcceleratorHandlerGtkTest home_pressed_ = true; return true; } + virtual bool CanHandleAccelerators() const { + return true; + } // WidgetDelegate Implementation. virtual View* GetContentsView() { diff --git a/ui/views/focus/focus_manager_unittest.cc b/ui/views/focus/focus_manager_unittest.cc index 1ec6143..8945a59 100644 --- a/ui/views/focus/focus_manager_unittest.cc +++ b/ui/views/focus/focus_manager_unittest.cc @@ -195,18 +195,29 @@ TEST_F(FocusManagerTest, ContainsView) { class TestAcceleratorTarget : public ui::AcceleratorTarget { public: explicit TestAcceleratorTarget(bool process_accelerator) - : accelerator_count_(0), process_accelerator_(process_accelerator) {} + : accelerator_count_(0), + process_accelerator_(process_accelerator), + can_handle_accelerators_(true) {} - virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) { + virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE { ++accelerator_count_; return process_accelerator_; } + virtual bool CanHandleAccelerators() const OVERRIDE { + return can_handle_accelerators_; + } + int accelerator_count() const { return accelerator_count_; } + void set_can_handle_accelerators(bool can_handle_accelerators) { + can_handle_accelerators_ = can_handle_accelerators; + } + private: int accelerator_count_; // number of times that the accelerator is activated bool process_accelerator_; // return value of AcceleratorPressed + bool can_handle_accelerators_; // return value of CanHandleAccelerators DISALLOW_COPY_AND_ASSIGN(TestAcceleratorTarget); }; @@ -303,6 +314,40 @@ TEST_F(FocusManagerTest, CallsNormalAcceleratorTarget) { EXPECT_EQ(escape_target.accelerator_count(), 1); } +TEST_F(FocusManagerTest, CallsEnabledAcceleratorTargetsOnly) { + FocusManager* focus_manager = GetFocusManager(); + ui::Accelerator return_accelerator(ui::VKEY_RETURN, false, false, false); + + TestAcceleratorTarget return_target1(true); + TestAcceleratorTarget return_target2(true); + + focus_manager->RegisterAccelerator(return_accelerator, &return_target1); + focus_manager->RegisterAccelerator(return_accelerator, &return_target2); + EXPECT_TRUE(focus_manager->ProcessAccelerator(return_accelerator)); + EXPECT_EQ(0, return_target1.accelerator_count()); + EXPECT_EQ(1, return_target2.accelerator_count()); + + // If CanHandleAccelerators() return false, FocusManager shouldn't call + // AcceleratorPressed(). + return_target2.set_can_handle_accelerators(false); + EXPECT_TRUE(focus_manager->ProcessAccelerator(return_accelerator)); + EXPECT_EQ(1, return_target1.accelerator_count()); + EXPECT_EQ(1, return_target2.accelerator_count()); + + // If no accelerator targets are enabled, ProcessAccelerator() should fail. + return_target1.set_can_handle_accelerators(false); + EXPECT_FALSE(focus_manager->ProcessAccelerator(return_accelerator)); + EXPECT_EQ(1, return_target1.accelerator_count()); + EXPECT_EQ(1, return_target2.accelerator_count()); + + // Enabling the target again causes the accelerators to be processed again. + return_target1.set_can_handle_accelerators(true); + return_target2.set_can_handle_accelerators(true); + EXPECT_TRUE(focus_manager->ProcessAccelerator(return_accelerator)); + EXPECT_EQ(1, return_target1.accelerator_count()); + EXPECT_EQ(2, return_target2.accelerator_count()); +} + // Unregisters itself when its accelerator is invoked. class SelfUnregisteringAcceleratorTarget : public ui::AcceleratorTarget { public: @@ -313,12 +358,16 @@ class SelfUnregisteringAcceleratorTarget : public ui::AcceleratorTarget { accelerator_count_(0) { } - virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) { + virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE { ++accelerator_count_; focus_manager_->UnregisterAccelerator(accelerator, this); return true; } + virtual bool CanHandleAccelerators() const OVERRIDE { + return true; + } + int accelerator_count() const { return accelerator_count_; } private: diff --git a/ui/views/view.cc b/ui/views/view.cc index 920939e..d764d2f 100644 --- a/ui/views/view.cc +++ b/ui/views/view.cc @@ -870,6 +870,10 @@ bool View::AcceleratorPressed(const ui::Accelerator& accelerator) { return false; } +bool View::CanHandleAccelerators() const { + return enabled() && IsDrawn() && GetWidget() && GetWidget()->IsVisible(); +} + // Focus ----------------------------------------------------------------------- bool View::HasFocus() const { @@ -1587,10 +1591,6 @@ void View::PropagateVisibilityNotifications(View* start, bool is_visible) { } void View::VisibilityChangedImpl(View* starting_from, bool is_visible) { - if (is_visible) - RegisterPendingAccelerators(); - else - UnregisterAccelerators(true); VisibilityChanged(starting_from, is_visible); } @@ -1929,9 +1929,6 @@ void View::RegisterPendingAccelerators() { #endif return; } - // Only register accelerators if we are visible. - if (!IsDrawn() || !GetWidget()->IsVisible()) - return; for (std::vector<ui::Accelerator>::const_iterator i( accelerators_->begin() + registered_accelerator_count_); i != accelerators_->end(); ++i) { diff --git a/ui/views/view.h b/ui/views/view.h index 1f0cb58..22a98be 100644 --- a/ui/views/view.h +++ b/ui/views/view.h @@ -630,7 +630,8 @@ class VIEWS_EXPORT View : public ui::LayerDelegate, // Sets a keyboard accelerator for that view. When the user presses the // accelerator key combination, the AcceleratorPressed method is invoked. // Note that you can set multiple accelerators for a view by invoking this - // method several times. + // method several times. Note also that AcceleratorPressed is invoked only + // when CanHandleAccelerators() is true. virtual void AddAccelerator(const ui::Accelerator& accelerator); // Removes the specified accelerator for this view. @@ -642,6 +643,11 @@ class VIEWS_EXPORT View : public ui::LayerDelegate, // Overridden from AcceleratorTarget: virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE; + // Returns whether accelerators are enabled for this view. Accelerators are + // enabled if the containing widget is visible and the view is enabled() and + // IsDrawn() + virtual bool CanHandleAccelerators() const OVERRIDE; + // Focus --------------------------------------------------------------------- // Returns whether this view currently has the focus. diff --git a/ui/views/view_unittest.cc b/ui/views/view_unittest.cc index 8d68d2c..739d393 100644 --- a/ui/views/view_unittest.cc +++ b/ui/views/view_unittest.cc @@ -1058,12 +1058,10 @@ TEST_F(ViewTest, HiddenViewWithAccelerator) { ASSERT_TRUE(focus_manager); view->SetVisible(false); - EXPECT_EQ(NULL, - focus_manager->GetCurrentTargetForAccelerator(return_accelerator)); + EXPECT_FALSE(focus_manager->ProcessAccelerator(return_accelerator)); view->SetVisible(true); - EXPECT_EQ(view, - focus_manager->GetCurrentTargetForAccelerator(return_accelerator)); + EXPECT_TRUE(focus_manager->ProcessAccelerator(return_accelerator)); widget->CloseNow(); } @@ -1088,16 +1086,16 @@ TEST_F(ViewTest, ViewInHiddenWidgetWithAccelerator) { FocusManager* focus_manager = widget->GetFocusManager(); ASSERT_TRUE(focus_manager); - EXPECT_EQ(NULL, - focus_manager->GetCurrentTargetForAccelerator(return_accelerator)); + EXPECT_FALSE(focus_manager->ProcessAccelerator(return_accelerator)); + EXPECT_EQ(0, view->accelerator_count_map_[return_accelerator]); widget->Show(); - EXPECT_EQ(view, - focus_manager->GetCurrentTargetForAccelerator(return_accelerator)); + EXPECT_TRUE(focus_manager->ProcessAccelerator(return_accelerator)); + EXPECT_EQ(1, view->accelerator_count_map_[return_accelerator]); widget->Hide(); - EXPECT_EQ(NULL, - focus_manager->GetCurrentTargetForAccelerator(return_accelerator)); + EXPECT_FALSE(focus_manager->ProcessAccelerator(return_accelerator)); + EXPECT_EQ(1, view->accelerator_count_map_[return_accelerator]); widget->CloseNow(); } |