shelf: Press the shortcut in auto-hide shelf should do nothing.
changes in this cl:
- Do not process the event in ShelfView::ButtonPressed for auto-hide
shelf. Since for example we don't want to open the app window by tap
the shortcut in auto-hide shelf.
- Keep auto-hide shelf visible if tap inside the shelf when app list is
opened. This is used to make sure press the shortcut in the auto-hide
shelf when app list is opened can still open the corresponding app.
It works before since we can tap the shortcut in the auto-hide shelf
to open the app, but we fixed it in this cl.
Bug: 914189
Change-Id: I5cdc5e0164b8958e3ffdf03c9718ffab0c6a5ab6
Reviewed-on: https://chromium-review.googlesource.com/c/1374147
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616896}
diff --git a/ash/app_list/app_list_presenter_delegate_impl.cc b/ash/app_list/app_list_presenter_delegate_impl.cc
index 089d9676..44e7a42 100644
--- a/ash/app_list/app_list_presenter_delegate_impl.cc
+++ b/ash/app_list/app_list_presenter_delegate_impl.cc
@@ -240,15 +240,19 @@
aura::Window* window = view_->GetWidget()->GetNativeView()->parent();
if (!window->Contains(target) && !presenter_->CloseOpenedPage() &&
!app_list::switches::ShouldNotDismissOnBlur() && !IsTabletMode()) {
- // Dismiss the app list but not the auto-hide shelf if the event happened in
- // the status area. Since tap the tray in the status area should open the
- // corresponding tray bubble.
- aura::Window* status_window =
+ const aura::Window* status_window =
shelf->shelf_widget()->status_area_widget()->GetNativeWindow();
+ const aura::Window* shelf_window = shelf->shelf_widget()->GetNativeWindow();
+ // Don't dismiss the auto-hide shelf if event happened in status area. Then
+ // the event can still be propagated to the status area tray to open the
+ // corresponding tray bubble.
base::Optional<Shelf::ScopedAutoHideLock> auto_hide_lock;
if (status_window && status_window->Contains(target))
auto_hide_lock.emplace(shelf);
- presenter_->Dismiss(event->time_stamp());
+
+ // Keep app list opened if event happened in the shelf area.
+ if (!shelf_window || !shelf_window->Contains(target))
+ presenter_->Dismiss(event->time_stamp());
}
}
diff --git a/ash/app_list/app_list_presenter_delegate_unittest.cc b/ash/app_list/app_list_presenter_delegate_unittest.cc
index b3208c2..6875c94 100644
--- a/ash/app_list/app_list_presenter_delegate_unittest.cc
+++ b/ash/app_list/app_list_presenter_delegate_unittest.cc
@@ -1108,10 +1108,9 @@
GetAppListTestHelper()->CheckState(app_list::AppListViewState::CLOSED);
}
-// Tests that tap the status area of auto-hide shelf with app list opened should
-// open the corresponding tray bubble but dismiss the app list.
-TEST_F(AppListPresenterDelegateTest,
- TapStatusAreaOfAutoHideShelfWithAppListOpened) {
+// Tests that tap the auto-hide shelf with app list opened should dismiss the
+// app list but keep shelf visible.
+TEST_F(AppListPresenterDelegateTest, TapAutoHideShelfWithAppListOpened) {
Shelf* shelf = GetPrimaryShelf();
shelf->SetAutoHideBehavior(SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS);
@@ -1133,6 +1132,22 @@
EXPECT_TRUE(GetPrimaryUnifiedSystemTray()->IsBubbleShown());
GetAppListTestHelper()->CheckVisibility(false);
EXPECT_EQ(SHELF_AUTO_HIDE_SHOWN, shelf->GetAutoHideState());
+
+ // Tap to dismiss the app list and the auto-hide shelf.
+ generator->GestureTapAt(gfx::Point(0, 0));
+ EXPECT_FALSE(GetPrimaryUnifiedSystemTray()->IsBubbleShown());
+ EXPECT_EQ(SHELF_AUTO_HIDE_HIDDEN, shelf->GetAutoHideState());
+ GetAppListTestHelper()->WaitUntilIdle();
+
+ // Tap the auto-hide shelf area with app list opened should keep both app list
+ // and shelf visible.
+ GetAppListTestHelper()->ShowAndRunLoop(GetPrimaryDisplayId());
+ GetAppListTestHelper()->CheckVisibility(true);
+ EXPECT_EQ(SHELF_AUTO_HIDE_SHOWN, shelf->GetAutoHideState());
+ generator->GestureTapAt(
+ shelf->GetShelfViewForTesting()->GetBoundsInScreen().CenterPoint());
+ GetAppListTestHelper()->CheckVisibility(true);
+ EXPECT_EQ(SHELF_AUTO_HIDE_SHOWN, shelf->GetAutoHideState());
}
// Test a variety of behaviors for home launcher (app list in tablet mode).
diff --git a/ash/shelf/shelf_layout_manager_unittest.cc b/ash/shelf/shelf_layout_manager_unittest.cc
index a92139d..bd4fe88 100644
--- a/ash/shelf/shelf_layout_manager_unittest.cc
+++ b/ash/shelf/shelf_layout_manager_unittest.cc
@@ -19,6 +19,7 @@
#include "ash/public/cpp/window_properties.h"
#include "ash/root_window_controller.h"
#include "ash/session/session_controller.h"
+#include "ash/shelf/app_list_button.h"
#include "ash/shelf/shelf.h"
#include "ash/shelf/shelf_constants.h"
#include "ash/shelf/shelf_layout_manager_observer.h"
@@ -2573,6 +2574,31 @@
EXPECT_TRUE(GetPrimaryUnifiedSystemTray()->IsBubbleShown());
}
+// Tests that tap shelf item in auto-hide shelf should do nothing.
+TEST_F(ShelfLayoutManagerTest, TapShelfItemInAutoHideShelf) {
+ Shelf* shelf = GetPrimaryShelf();
+ shelf->SetAutoHideBehavior(SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS);
+
+ // Create a normal unmaximized window; the shelf should be hidden.
+ aura::Window* window = CreateTestWindow();
+ window->SetBounds(gfx::Rect(0, 0, 100, 100));
+ window->Show();
+ GetAppListTestHelper()->CheckVisibility(false);
+ EXPECT_EQ(SHELF_AUTO_HIDE, shelf->GetVisibilityState());
+ EXPECT_EQ(SHELF_AUTO_HIDE_HIDDEN, shelf->GetAutoHideState());
+
+ // Tap app list button should not open the app list and shelf should keep
+ // hidden.
+ gfx::Rect app_list_button_bounds =
+ shelf->GetShelfViewForTesting()->GetAppListButton()->GetBoundsInScreen();
+ gfx::Rect display_bounds =
+ display::Screen::GetScreen()->GetPrimaryDisplay().bounds();
+ app_list_button_bounds.Intersect(display_bounds);
+ GetEventGenerator()->GestureTapAt(app_list_button_bounds.CenterPoint());
+ GetAppListTestHelper()->CheckVisibility(false);
+ EXPECT_EQ(SHELF_AUTO_HIDE_HIDDEN, shelf->GetAutoHideState());
+}
+
class ShelfLayoutManagerKeyboardTest : public AshTestBase {
public:
ShelfLayoutManagerKeyboardTest() = default;
diff --git a/ash/shelf/shelf_view.cc b/ash/shelf/shelf_view.cc
index 74969f6f..38ca433a 100644
--- a/ash/shelf/shelf_view.cc
+++ b/ash/shelf/shelf_view.cc
@@ -521,6 +521,13 @@
void ShelfView::ButtonPressed(views::Button* sender,
const ui::Event& event,
views::InkDrop* ink_drop) {
+ // Press the shelf item in the auto-hide shelf should not open the
+ // corresponding app window. The event can still be received by auto-hide
+ // shelf since we reserved portion of the auto-hide shelf within the screen
+ // bounds.
+ if (!shelf_->IsVisible())
+ return;
+
if (sender == overflow_button_) {
ToggleOverflowBubble();
shelf_button_pressed_metric_tracker_.ButtonPressed(event, sender,
@@ -1790,7 +1797,6 @@
}
void ShelfView::OnGestureEvent(ui::GestureEvent* event) {
-
// Convert the event location from current view to screen, since swiping up on
// the shelf can open the fullscreen app list. Updating the bounds of the app
// list during dragging is based on screen coordinate space.