Notification popup behavior broken when notifications are dismissed by touch |
||||
Issue descriptionChrome Version: 66.0.3330.0 OS: Chrome OS What steps will reproduce the problem? (1) Use Notification Galore! to generate three notification popups on the right bottom. https://chrome.google.com/webstore/detail/notifications-galore/gclcddgeeaknflkijpcbplmhbkonmlij (2) Dismiss them by touch slide (3) Create another notification by Notification Galore! What is the expected result? The new notification should also be shown as a popup. What happens instead? It does not show up.
,
Mar 6 2018
,
Mar 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5529bc6356b7dcbabea9003673eb2b0a994f38c0 commit 5529bc6356b7dcbabea9003673eb2b0a994f38c0 Author: Tetsui Ohkubo <tetsui@chromium.org> Date: Tue Mar 13 01:35:53 2018 Add workaround for MessagePopupCollection bug This CL adds workaround to MessageView::SlideOut() for a bug that it did not work with MessagePopupCollection. If |by_user| of RemoveNotification is true in SlideOut(), IncrementDeferCounter() in MessagePopupCollection::OnNotificationRemoved is called but corresponding DecrementDeferCounter() won't be called. MessagePopupCollection will be completely rewritten. TEST=message_center_unittests BUG= 805208 Change-Id: I1a637cef8bdcb25be45647f487f1d29ab3952583 Reviewed-on: https://chromium-review.googlesource.com/952822 Reviewed-by: Evan Stade <estade@chromium.org> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> Cr-Commit-Position: refs/heads/master@{#542686} [modify] https://crrev.com/5529bc6356b7dcbabea9003673eb2b0a994f38c0/ui/message_center/views/message_popup_collection_unittest.cc [modify] https://crrev.com/5529bc6356b7dcbabea9003673eb2b0a994f38c0/ui/message_center/views/message_view.cc [modify] https://crrev.com/5529bc6356b7dcbabea9003673eb2b0a994f38c0/ui/message_center/views/message_view.h [modify] https://crrev.com/5529bc6356b7dcbabea9003673eb2b0a994f38c0/ui/message_center/views/slide_out_controller.cc [modify] https://crrev.com/5529bc6356b7dcbabea9003673eb2b0a994f38c0/ui/message_center/views/slide_out_controller.h
,
Mar 13 2018
,
Mar 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c8e13b10359ee185b83a616c5f585dbf7f9b764 commit 5c8e13b10359ee185b83a616c5f585dbf7f9b764 Author: Evan Stade <estade@chromium.org> Date: Wed Mar 21 21:10:11 2018 Partial revert of 5529bc6356b7dcbabea9003673eb2b0a99 Using false for |by_user| for slide out breaks a lot of NotificationDelegates. We'll have to find another fix. Bug: 805208 , 822248 Change-Id: Ic56783427354b96f74ac393b7bcd07380eadfda5 Reviewed-on: https://chromium-review.googlesource.com/966891 Commit-Queue: Evan Stade <estade@chromium.org> Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Cr-Commit-Position: refs/heads/master@{#544842} [modify] https://crrev.com/5c8e13b10359ee185b83a616c5f585dbf7f9b764/ui/message_center/views/message_popup_collection_unittest.cc [modify] https://crrev.com/5c8e13b10359ee185b83a616c5f585dbf7f9b764/ui/message_center/views/message_view.cc
,
Mar 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/39296e726ce046a4b94719ba131a0714c3ce4e5e commit 39296e726ce046a4b94719ba131a0714c3ce4e5e Author: Evan Stade <estade@chromium.org> Date: Thu Mar 22 20:43:49 2018 Message center popups: remove defer timer and related functionality. The intended behavior as described in code comments as: // Denotes a mode when user is clicking the Close button of toasts in a // sequence, w/o moving the mouse. We reposition the toasts so the next one // happens to be right under the mouse, and the user can just dispose of // multipel toasts by clicking. The mode ends when defer_timer_ expires. In practice, it didn't work if consecutive toasts were not the same height, and wasn't necessary if consecutive toasts were the same height. Given that it doesn't work, this code adds needless complexity. It is also suspected of causing bugs like preventing notification toasts from showing. To test on desktop, you may need to pass --disable-features="NativeNotifications" Bug: 814495, 805208 Change-Id: I197c2a154470068ff6f57e8c91bb374885ca9536 Reviewed-on: https://chromium-review.googlesource.com/967094 Commit-Queue: Evan Stade <estade@chromium.org> Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org> Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> Cr-Commit-Position: refs/heads/master@{#545239} [modify] https://crrev.com/39296e726ce046a4b94719ba131a0714c3ce4e5e/ash/system/web_notification/web_notification_tray.cc [modify] https://crrev.com/39296e726ce046a4b94719ba131a0714c3ce4e5e/chrome/browser/ui/views/message_center/popups_only_ui_delegate.cc [modify] https://crrev.com/39296e726ce046a4b94719ba131a0714c3ce4e5e/ui/message_center/views/message_popup_collection.cc [modify] https://crrev.com/39296e726ce046a4b94719ba131a0714c3ce4e5e/ui/message_center/views/message_popup_collection.h [modify] https://crrev.com/39296e726ce046a4b94719ba131a0714c3ce4e5e/ui/message_center/views/message_popup_collection_unittest.cc [modify] https://crrev.com/39296e726ce046a4b94719ba131a0714c3ce4e5e/ui/message_center/views/popup_alignment_delegate.cc [modify] https://crrev.com/39296e726ce046a4b94719ba131a0714c3ce4e5e/ui/message_center/views/toast_contents_view.cc
,
Apr 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/278ee5ec5ef68d11993c955e4e400c41bb6cdd08 commit 278ee5ec5ef68d11993c955e4e400c41bb6cdd08 Author: Evan Stade <estade@chromium.org> Date: Tue Apr 03 19:41:54 2018 Message center popups: remove defer timer and related functionality. The intended behavior as described in code comments as: // Denotes a mode when user is clicking the Close button of toasts in a // sequence, w/o moving the mouse. We reposition the toasts so the next one // happens to be right under the mouse, and the user can just dispose of // multipel toasts by clicking. The mode ends when defer_timer_ expires. In practice, it didn't work if consecutive toasts were not the same height, and wasn't necessary if consecutive toasts were the same height. Given that it doesn't work, this code adds needless complexity. It is also suspected of causing bugs like preventing notification toasts from showing. To test on desktop, you may need to pass --disable-features="NativeNotifications" Bug: 814495, 805208 Change-Id: I197c2a154470068ff6f57e8c91bb374885ca9536 Reviewed-on: https://chromium-review.googlesource.com/967094 Commit-Queue: Evan Stade <estade@chromium.org> Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org> Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#545239}(cherry picked from commit 39296e726ce046a4b94719ba131a0714c3ce4e5e) Reviewed-on: https://chromium-review.googlesource.com/993614 Reviewed-by: Evan Stade <estade@chromium.org> Cr-Commit-Position: refs/branch-heads/3359@{#557} Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276} [modify] https://crrev.com/278ee5ec5ef68d11993c955e4e400c41bb6cdd08/ash/system/web_notification/web_notification_tray.cc [modify] https://crrev.com/278ee5ec5ef68d11993c955e4e400c41bb6cdd08/chrome/browser/ui/views/message_center/popups_only_ui_delegate.cc [modify] https://crrev.com/278ee5ec5ef68d11993c955e4e400c41bb6cdd08/ui/message_center/views/message_popup_collection.cc [modify] https://crrev.com/278ee5ec5ef68d11993c955e4e400c41bb6cdd08/ui/message_center/views/message_popup_collection.h [modify] https://crrev.com/278ee5ec5ef68d11993c955e4e400c41bb6cdd08/ui/message_center/views/message_popup_collection_unittest.cc [modify] https://crrev.com/278ee5ec5ef68d11993c955e4e400c41bb6cdd08/ui/message_center/views/popup_alignment_delegate.cc [modify] https://crrev.com/278ee5ec5ef68d11993c955e4e400c41bb6cdd08/ui/message_center/views/toast_contents_view.cc
,
Aug 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07e94dcfeee4e08e2a41992b9361e52206c3a867 commit 07e94dcfeee4e08e2a41992b9361e52206c3a867 Author: Tetsui Ohkubo <tetsui@chromium.org> Date: Wed Aug 08 02:20:09 2018 Remove bug workaround in MessageView. We had MessagePopupCollection bug workaround in MessageView. As MessagePopupCollection is rewritten, we don't need the workaround anymore. TEST=manual BUG= 805208 , 863366 Change-Id: I93ce95f3cf5648f9bf8f9761a78d08e93d1fa676 Reviewed-on: https://chromium-review.googlesource.com/1163415 Reviewed-by: Evan Stade <estade@chromium.org> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> Cr-Commit-Position: refs/heads/master@{#581444} [modify] https://crrev.com/07e94dcfeee4e08e2a41992b9361e52206c3a867/ui/message_center/views/message_view.cc |
||||
►
Sign in to add a comment |
||||
Comment 1 by tetsui@chromium.org
, Jan 24 2018