NotifierCollision: Do not intercept keyboard events on popup collection

Currently keyboard events are intercepted by the tray bubble view (reroute_event_handler_), which breaks inline reply on popup notifications if they are shown with the tray bubble. Thus, we need to make sure that events happen inside the popup collection do not get blocked when handling in reroute_event_handler_. Fixed: b:346641561 Change-Id: I6b9148a0e4f86298399541855257c32a1ced9742 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5798844 Reviewed-by: Jiaming Cheng <[email protected]> Commit-Queue: Andre Le <[email protected]> Cr-Commit-Position: refs/heads/main@{#1344378}
azx1573 · Aug 20, 2024 · 16f03a1 · 16f03a1
1 parent e71ac4e
commit 16f03a1
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 6 deletions.
diff --git a/ash/system/notification_center/ash_message_popup_collection_unittest.cc b/ash/system/notification_center/ash_message_popup_collection_unittest.cc
@@ -20,11 +20,11 @@
 #include "ash/shelf/shelf_layout_manager.h"
 #include "ash/shell.h"
 #include "ash/system/ime_menu/ime_menu_tray.h"
-#include "ash/system/notification_center/views/ash_notification_expand_button.h"
-#include "ash/system/notification_center/views/ash_notification_view.h"
 #include "ash/system/notification_center/message_center_test_util.h"
 #include "ash/system/notification_center/message_popup_animation_waiter.h"
 #include "ash/system/notification_center/notification_center_tray.h"
+#include "ash/system/notification_center/views/ash_notification_expand_button.h"
+#include "ash/system/notification_center/views/ash_notification_view.h"
 #include "ash/system/phonehub/phone_hub_tray.h"
 #include "ash/system/status_area_widget.h"
 #include "ash/system/status_area_widget_test_helper.h"
@@ -46,6 +46,8 @@
 #include "ui/base/ui_base_features.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/screen.h"
+#include "ui/events/event_constants.h"
+#include "ui/events/keycodes/keyboard_codes_posix.h"
 #include "ui/events/test/event_generator.h"
 #include "ui/gfx/animation/linear_animation.h"
 #include "ui/gfx/geometry/rect.h"
@@ -59,6 +61,7 @@
 #include "ui/message_center/views/notification_control_buttons_view.h"
 #include "ui/message_center/views/notification_view_base.h"
 #include "ui/views/controls/button/label_button.h"
+#include "ui/views/controls/textfield/textfield.h"
 #include "ui/wm/core/window_util.h"
 #include "url/gurl.h"
 
@@ -196,10 +199,11 @@ class AshMessagePopupCollectionTest : public AshTestBase,
   gfx::Rect GetWorkArea() { return GetPrimaryPopupCollection()->work_area_; }
 
   std::string AddNotification(bool has_image = false,
-                              const GURL& origin_url = GURL()) {
+                              const GURL& origin_url = GURL(),
+                              bool has_inline_reply = false) {
     std::string id = base::NumberToString(notification_id_++);
     message_center::MessageCenter::Get()->AddNotification(
-        CreateSimpleNotification(id, has_image, origin_url));
+        CreateSimpleNotification(id, has_image, origin_url, has_inline_reply));
     return id;
   }
 
@@ -1637,6 +1641,42 @@ TEST_P(AshMessagePopupCollectionTest, BubbleNotCloseWhenPopupClose) {
   EXPECT_TRUE(phone_hub_tray->GetBubbleView());
 }
 
+// For b/346641561
+TEST_P(AshMessagePopupCollectionTest, InlineReplyTextfield) {
+  if (!IsNotifierCollisionEnabled()) {
+    GTEST_SKIP() << "Popup notifications does not show when notifier collision "
+                    "is enabled";
+  }
+
+  auto* unified_system_tray = GetPrimaryUnifiedSystemTray();
+  unified_system_tray->ShowBubble();
+
+  // Attempt showing a notification when Quick Settings is open.
+  AddNotification(/*has_image=*/false,
+                  /*origin_url=*/GURL(),
+                  /*has_inline_reply=*/true);
+  auto* popup = GetLastPopUpAdded();
+  ASSERT_TRUE(popup);
+
+  AnimateUntilIdle();
+
+  auto* message_view =
+      static_cast<AshNotificationView*>(GetLastPopUpAdded()->message_view());
+  ASSERT_TRUE(message_view);
+
+  LeftClickOn(message_view->GetActionButtonsForTest().front());
+
+  auto* textfield = message_view->GetInlineReplyForTest()->textfield();
+  EXPECT_TRUE(textfield->GetVisible());
+  EXPECT_TRUE(textfield->HasFocus());
+
+  PressAndReleaseKey(ui::VKEY_A, ui::EF_NONE);
+  PressAndReleaseKey(ui::VKEY_A, ui::EF_NONE);
+
+  // Make sure that inline reply textfield can receive keyboard events.
+  EXPECT_EQ(u"aa", textfield->GetText());
+}
+
 class AshMessagePopupCollectionMockTimeTest : public ash::AshTestBase {
  public:
   AshMessagePopupCollectionMockTimeTest()

diff --git a/ash/system/notification_center/message_center_test_util.cc b/ash/system/notification_center/message_center_test_util.cc
@@ -16,7 +16,8 @@ namespace ash {
 std::unique_ptr<message_center::Notification> CreateSimpleNotification(
     const std::string& id,
     bool has_image,
-    const GURL& origin_url) {
+    const GURL& origin_url,
+    bool has_inline_reply) {
   message_center::NotifierId notifier_id;
   notifier_id.profile_id = "[email protected]";
   notifier_id.type = message_center::NotifierType::WEB_PAGE;
@@ -28,6 +29,12 @@ std::unique_ptr<message_center::Notification> CreateSimpleNotification(
       origin_url, notifier_id, message_center::RichNotificationData(),
       new message_center::NotificationDelegate());
 
+  if (has_inline_reply) {
+    message_center::ButtonInfo button_info(u"test button");
+    button_info.placeholder = std::u16string();
+    notification->set_buttons({button_info});
+  }
+
   if (has_image) {
     notification->SetImage(gfx::test::CreateImage(320, 300));
   }

diff --git a/ash/system/notification_center/message_center_test_util.h b/ash/system/notification_center/message_center_test_util.h
@@ -21,7 +21,8 @@ namespace ash {
 std::unique_ptr<message_center::Notification> CreateSimpleNotification(
     const std::string& id,
     bool has_image = false,
-    const GURL& origin_url = GURL());
+    const GURL& origin_url = GURL(),
+    bool has_inline_reply = false);
 
 }  // namespace ash
 

diff --git a/ash/system/notification_center/views/ash_notification_view.cc b/ash/system/notification_center/views/ash_notification_view.cc
@@ -1524,6 +1524,11 @@ views::Label* AshNotificationView::GetTitleRowLabelForTest() {
   return title_row_->title_view();
 }
 
+message_center::NotificationInputContainer*
+AshNotificationView::GetInlineReplyForTest() {
+  return inline_reply();
+}
+
 void AshNotificationView::OnNotificationRemoved(
     const std::string& notification_id,
     bool by_user) {

diff --git a/ash/system/notification_center/views/ash_notification_view.h b/ash/system/notification_center/views/ash_notification_view.h
@@ -152,6 +152,8 @@ class ASH_EXPORT AshNotificationView
 
   views::Label* GetTitleRowLabelForTest();
 
+  message_center::NotificationInputContainer* GetInlineReplyForTest();
+
   // View containing all grouped notifications, propagates size changes
   // to the parent notification view.
   class GroupedNotificationsContainer : public views::BoxLayoutView {

diff --git a/ash/system/tray/tray_bubble_view.cc b/ash/system/tray/tray_bubble_view.cc
@@ -14,10 +14,12 @@
 #include "ash/constants/ash_features.h"
 #include "ash/public/cpp/accelerators.h"
 #include "ash/public/cpp/style/color_provider.h"
+#include "ash/root_window_controller.h"
 #include "ash/shelf/shelf.h"
 #include "ash/shell.h"
 #include "ash/style/ash_color_id.h"
 #include "ash/style/system_shadow.h"
+#include "ash/system/notification_center/notification_center_tray.h"
 #include "ash/system/tray/system_tray_notifier.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/tray/tray_utils.h"
@@ -242,6 +244,25 @@ void TrayBubbleView::RerouteEventHandler::OnKeyEvent(ui::KeyEvent* event) {
     }
   }
 
+  // For shelf pod bubble that is anchored to the shelf corner, popup
+  // notifications and the bubble can be shown at the same time. If the keyboard
+  // happens inside the popup collection, we need to let the keyboard event pass
+  // there to make sure notification popups can receive keyboard events.
+  if (target &&
+      tray_bubble_view_->GetBubbleType() == TrayBubbleType::kShelfPodBubble &&
+      tray_bubble_view_->IsAnchoredToShelfCorner()) {
+    auto* popup_collection = RootWindowController::ForWindow(target)
+                                 ->shelf()
+                                 ->GetStatusAreaWidget()
+                                 ->notification_center_tray()
+                                 ->popup_collection();
+
+    if (popup_collection->popup_collection_bounds().Contains(
+            target->GetActualBoundsInScreen())) {
+      return;
+    }
+  }
+
   // Always consumes key event not to pass it to other widgets. Calling
   // StopPropagation here to make this consistent with
   // MenuController::OnWillDispatchKeyEvent.