New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 689220 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Non-closable ARC notification is removable by pressing close button.

Project Member Reported by yoshiki@chromium.org, Feb 6 2017

Issue description

Non-closable ARC notification can wrongly be removable by pressing close button if the notification is popup (not in the message center). Pressing close button should be just hiding the notification from the desktop and storing the notification into the message center.

This doesn't happen if the notification is non-arc Chrome notification.
 
Components: UI>Notifications
This also fixes another issue: swiping doesn't remove notification.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9739ee62037118f47be3dab9701164c428b02e5b

commit 9739ee62037118f47be3dab9701164c428b02e5b
Author: yoshiki <yoshiki@chromium.org>
Date: Mon Mar 06 02:27:48 2017

Not Remove Non-Closable Arc Popup When Close Button is Pressed

* Fixes the issue:  crbug.com/689220 
  - Adds ArcNotificationDelegate::Close method to handle the closing event
  - Not calls ArcCustomNotificationItem::Close() when the close button is pressed
* Refines the code
  - Uses the same method MessageView::OnCloseButtonPressed() both from custom and normal notification

BUG= 689220 

Review-Url: https://codereview.chromium.org/2668583005
Cr-Commit-Position: refs/heads/master@{#454814}

[modify] https://crrev.com/9739ee62037118f47be3dab9701164c428b02e5b/ui/arc/notification/arc_custom_notification_item.cc
[modify] https://crrev.com/9739ee62037118f47be3dab9701164c428b02e5b/ui/arc/notification/arc_custom_notification_view.cc
[modify] https://crrev.com/9739ee62037118f47be3dab9701164c428b02e5b/ui/message_center/views/custom_notification_view.cc
[modify] https://crrev.com/9739ee62037118f47be3dab9701164c428b02e5b/ui/message_center/views/custom_notification_view.h
[modify] https://crrev.com/9739ee62037118f47be3dab9701164c428b02e5b/ui/message_center/views/message_view.cc
[modify] https://crrev.com/9739ee62037118f47be3dab9701164c428b02e5b/ui/message_center/views/message_view.h
[modify] https://crrev.com/9739ee62037118f47be3dab9701164c428b02e5b/ui/message_center/views/notification_view.cc

Labels: Merge-Request-58 M-58
It looks fixed in the canary. I request a merge.
Labels: -Pri-3 Pri-1
Project Member

Comment 7 by sheriffbot@chromium.org, Mar 16 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 16 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/900e772bd5b5367e86fcb13c315807470696d081

commit 900e772bd5b5367e86fcb13c315807470696d081
Author: yoshiki iguchi <yoshiki@chromium.org>
Date: Thu Mar 16 06:20:49 2017

Not Remove Non-Closable Arc Popup When Close Button is Pressed

* Fixes the issue:  crbug.com/689220 
  - Adds ArcNotificationDelegate::Close method to handle the closing event
  - Not calls ArcCustomNotificationItem::Close() when the close button is pressed
* Refines the code
  - Uses the same method MessageView::OnCloseButtonPressed() both from custom and normal notification

BUG= 689220 

Review-Url: https://codereview.chromium.org/2668583005
Cr-Commit-Position: refs/heads/master@{#454814}
(cherry picked from commit 9739ee62037118f47be3dab9701164c428b02e5b)

Review-Url: https://codereview.chromium.org/2751353002 .
Cr-Commit-Position: refs/branch-heads/3029@{#228}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/900e772bd5b5367e86fcb13c315807470696d081/ui/arc/notification/arc_custom_notification_item.cc
[modify] https://crrev.com/900e772bd5b5367e86fcb13c315807470696d081/ui/arc/notification/arc_custom_notification_view.cc
[modify] https://crrev.com/900e772bd5b5367e86fcb13c315807470696d081/ui/message_center/views/custom_notification_view.cc
[modify] https://crrev.com/900e772bd5b5367e86fcb13c315807470696d081/ui/message_center/views/custom_notification_view.h
[modify] https://crrev.com/900e772bd5b5367e86fcb13c315807470696d081/ui/message_center/views/message_view.cc
[modify] https://crrev.com/900e772bd5b5367e86fcb13c315807470696d081/ui/message_center/views/message_view.h
[modify] https://crrev.com/900e772bd5b5367e86fcb13c315807470696d081/ui/message_center/views/notification_view.cc

Status: Fixed (was: Started)

Sign in to add a comment