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

Issue 719407 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression

Blocking:
issue 716429



Sign in to add a comment

Swipe to close no longer works for ARC notifications

Project Member Reported by edcourtney@chromium.org, May 8 2017

Issue description

Swipe to close no longer works for ARC notifications
 
Cc: osh...@chromium.org
As chatted with Oshima-san, he kindly helps to fix this.
Blocking: 716429
Project Member

Comment 3 by bugdroid1@chromium.org, May 19 2017

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

commit 870290f623b47e436d231e354c84b8723bb7000a
Author: edcourtney <edcourtney@chromium.org>
Date: Fri May 19 05:15:08 2017

[Notifications] Fix swipe to close for ARC notifications.

Change ArcCustomNotificationView::EventForwarder to not directly invoke On*Event methods of ArcCustomNotificationView (which then call up to the parent). Instead, re-dispatch them to the containing widget, which should handle the event propagation, letting SlideOutController get events as it should. A consequence of this is that I had to add a delegate method to SlideOutController to update ArcCustomNotificationView::SlideHelper. This is because ArcCustomNotificationView will no longer get the gesture events, since it is no longer being called directly from EventForwarder.

BUG= 719407 

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

[modify] https://crrev.com/870290f623b47e436d231e354c84b8723bb7000a/ui/arc/notification/arc_custom_notification_view.cc
[modify] https://crrev.com/870290f623b47e436d231e354c84b8723bb7000a/ui/arc/notification/arc_custom_notification_view.h
[modify] https://crrev.com/870290f623b47e436d231e354c84b8723bb7000a/ui/message_center/views/custom_notification_content_view_delegate.h
[modify] https://crrev.com/870290f623b47e436d231e354c84b8723bb7000a/ui/message_center/views/custom_notification_view.cc
[modify] https://crrev.com/870290f623b47e436d231e354c84b8723bb7000a/ui/message_center/views/custom_notification_view.h
[modify] https://crrev.com/870290f623b47e436d231e354c84b8723bb7000a/ui/message_center/views/custom_notification_view_unittest.cc
[modify] https://crrev.com/870290f623b47e436d231e354c84b8723bb7000a/ui/message_center/views/message_view.cc
[modify] https://crrev.com/870290f623b47e436d231e354c84b8723bb7000a/ui/message_center/views/message_view.h
[modify] https://crrev.com/870290f623b47e436d231e354c84b8723bb7000a/ui/message_center/views/slide_out_controller.cc
[modify] https://crrev.com/870290f623b47e436d231e354c84b8723bb7000a/ui/message_center/views/slide_out_controller.h

Labels: Merge-Request-59 M-59
Requesting merge to M59. Thanks~!

Comment 5 by est...@chromium.org, May 19 2017

Labels: ReleaseBlock-Beta
I believe all the preliminary patches that needed merging first have now made it over to M59 (3071).
Project Member

Comment 6 by sheriffbot@chromium.org, May 20 2017

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

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

Comment 7 by gkihumba@google.com, May 21 2017

Please go ahead and merge this to M59, thanks.
Project Member

Comment 8 by bugdroid1@chromium.org, May 22 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e33f895d0e3263a25c8e7cd114bfb793ea99fe83

commit e33f895d0e3263a25c8e7cd114bfb793ea99fe83
Author: yoshiki iguchi <yoshiki@chromium.org>
Date: Mon May 22 05:52:20 2017

[Notifications] Fix swipe to close for ARC notifications.

Change ArcCustomNotificationView::EventForwarder to not directly invoke On*Event methods of ArcCustomNotificationView (which then call up to the parent). Instead, re-dispatch them to the containing widget, which should handle the event propagation, letting SlideOutController get events as it should. A consequence of this is that I had to add a delegate method to SlideOutController to update ArcCustomNotificationView::SlideHelper. This is because ArcCustomNotificationView will no longer get the gesture events, since it is no longer being called directly from EventForwarder.

BUG= 719407 

Review-Url: https://codereview.chromium.org/2873553002
Cr-Commit-Position: refs/heads/master@{#473094}
(cherry picked from commit 870290f623b47e436d231e354c84b8723bb7000a)

Review-Url: https://codereview.chromium.org/2897903002 .
Cr-Commit-Position: refs/branch-heads/3071@{#647}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/e33f895d0e3263a25c8e7cd114bfb793ea99fe83/ui/arc/notification/arc_custom_notification_view.cc
[modify] https://crrev.com/e33f895d0e3263a25c8e7cd114bfb793ea99fe83/ui/arc/notification/arc_custom_notification_view.h
[modify] https://crrev.com/e33f895d0e3263a25c8e7cd114bfb793ea99fe83/ui/message_center/views/custom_notification_content_view_delegate.h
[modify] https://crrev.com/e33f895d0e3263a25c8e7cd114bfb793ea99fe83/ui/message_center/views/custom_notification_view.cc
[modify] https://crrev.com/e33f895d0e3263a25c8e7cd114bfb793ea99fe83/ui/message_center/views/custom_notification_view.h
[modify] https://crrev.com/e33f895d0e3263a25c8e7cd114bfb793ea99fe83/ui/message_center/views/custom_notification_view_unittest.cc
[modify] https://crrev.com/e33f895d0e3263a25c8e7cd114bfb793ea99fe83/ui/message_center/views/message_view.cc
[modify] https://crrev.com/e33f895d0e3263a25c8e7cd114bfb793ea99fe83/ui/message_center/views/message_view.h
[modify] https://crrev.com/e33f895d0e3263a25c8e7cd114bfb793ea99fe83/ui/message_center/views/slide_out_controller.cc
[modify] https://crrev.com/e33f895d0e3263a25c8e7cd114bfb793ea99fe83/ui/message_center/views/slide_out_controller.h

Status: Fixed (was: Started)
Thanks for merging that for me Yoshiki-san.
Status: Verified (was: Fixed)
9460.50.0, 59.0.3071.71

Sign in to add a comment