From 541a89716190c62e5f0f2e3319da61c837c1800d Mon Sep 17 00:00:00 2001 From: "rdevlin.cronin" Date: Tue, 28 Jul 2015 17:17:16 -0700 Subject: [PATCH] [Extensions UI] Fix two bugs in the toolbar actions bar * Don't allow resizing of the toolbar when it's highlighting. The point of the highlight is to show a subset of actions, so adjusting the size is bad. * Update whether or not an overflowed action wants to run when an extension is removed. Review URL: https://codereview.chromium.org/1262643003 Cr-Commit-Position: refs/heads/master@{#340819} --- .../browser/ui/cocoa/extensions/browser_actions_container_view.h | 7 ++++--- .../ui/cocoa/extensions/browser_actions_container_view.mm | 1 + chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc | 9 +++++++++ chrome/browser/ui/toolbar/toolbar_actions_bar.cc | 2 ++ chrome/browser/ui/views/toolbar/browser_actions_container.cc | 6 ++++++ 5 files changed, 22 insertions(+), 3 deletions(-) diff --git a/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.h b/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.h index d775b10a5d9c..daaba44d6062 100644 --- a/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.h +++ b/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.h @@ -77,9 +77,10 @@ class BrowserActionsContainerViewSizeDelegate { // Whether the container is currently being resized by the user. BOOL userIsResizing_; - // Whether the user can resize this at all. Resizing is disabled in incognito - // mode since any changes done in incognito mode are not saved anyway, and - // also to avoid a crash. http://crbug.com/42848 + // Whether the user can resize the container; this is disabled for overflow + // (where it would make no sense) and during highlighting, since this is a + // temporary and entirely browser-driven sequence in order to warn the user + // about potentially dangerous items. BOOL resizable_; // Whether or not the container is the overflow container, and is shown in the diff --git a/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm b/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm index d4372005c8ac..1dc68604baee 100644 --- a/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm +++ b/chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm @@ -169,6 +169,7 @@ const CGFloat kMinimumContainerWidth = 3.0; - (void)setHighlight:(scoped_ptr)highlight { if (highlight || highlight_) { highlight_ = highlight.Pass(); + resizable_ = highlight.get() != nullptr; [self setNeedsDisplay:YES]; } } diff --git a/chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc b/chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc index 5cbba3e209b9..a47a3c882f31 100644 --- a/chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc +++ b/chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc @@ -333,6 +333,15 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsBarRedesignBrowserTest, EXPECT_TRUE(browser_actions_bar()->OverflowedActionButtonWantsToRun()); toolbar_model()->SetVisibleIconCount(4); EXPECT_FALSE(browser_actions_bar()->OverflowedActionButtonWantsToRun()); + + // Adjusting the visible count down should mean an overflowed action wants + // to run again. Removing the action that wants to run should result in + // no overflowed action wanting to run. + toolbar_model()->SetVisibleIconCount(3); + EXPECT_TRUE(browser_actions_bar()->OverflowedActionButtonWantsToRun()); + extension_service()->DisableExtension(page_action_extension->id(), + extensions::Extension::DISABLE_NONE); + EXPECT_FALSE(browser_actions_bar()->OverflowedActionButtonWantsToRun()); } IN_PROC_BROWSER_TEST_F(BrowserActionsBarBrowserTest, diff --git a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc index d720fcb613cd..2a318efd4447 100644 --- a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc +++ b/chrome/browser/ui/toolbar/toolbar_actions_bar.cc @@ -606,6 +606,8 @@ void ToolbarActionsBar::OnToolbarExtensionRemoved( ResizeDelegate(gfx::Tween::EASE_OUT, false); } } + + SetOverflowedActionWantsToRun(); } void ToolbarActionsBar::OnToolbarExtensionMoved( diff --git a/chrome/browser/ui/views/toolbar/browser_actions_container.cc b/chrome/browser/ui/views/toolbar/browser_actions_container.cc index 09d3dc6d4697..fe3626860a30 100644 --- a/chrome/browser/ui/views/toolbar/browser_actions_container.cc +++ b/chrome/browser/ui/views/toolbar/browser_actions_container.cc @@ -646,6 +646,12 @@ bool BrowserActionsContainer::CanStartDragForView(View* sender, } void BrowserActionsContainer::OnResize(int resize_amount, bool done_resizing) { + // We don't allow resize while the toolbar is highlighting a subset of + // actions, since this is a temporary and entirely browser-driven sequence in + // order to warn the user about potentially dangerous items. + if (toolbar_actions_bar_->is_highlighting()) + return; + if (!done_resizing) { resize_amount_ = resize_amount; Redraw(false); -- 2.11.4.GIT