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

Issue 716429 link

Starred by 2 users
Status: Verified
Owner:
est...@chromium.org
Closed: May 2017
Cc:
█ gkihumba@chromium.org
█ yoshiki@chromium.org
edcourtney@chromium.org
█ gkihumba@google.com
est...@chromium.org
yhanada@chromium.org
peter@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression

Blocked on:
issue 718956
issue 719407
issue 719414


Sign in to add a comment

Border keeps the initial position even during swiping popup notification

Project Member Reported by yoshiki@chromium.org, Apr 28 2017 
Chrome Version: R58

What steps will reproduce the problem?
(1) Show popup notification
(2) Touch it and try to swiping it to the left
(3) See that the notification is moved by swipe

What is the expected result?
The notification border moves with the notification

What happens instead?
The notification border keeps the initial position

Please use labels and text to provide additional information.


For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.

Comment 1 by yoshiki@google.com, Apr 28 2017 

Repro video
IMG_9302.MOV
23.4 MB Download

Comment 2 by yoshiki@chromium.org, Apr 28 2017

Cc: yoshiki@chromium.org
Owner: est...@chromium.org
As I did bisect, the regression starts from https://chromium.googlesource.com/chromium/src/+/36f01b0163a66ba5174589f0e766ff80f28ae748. This CL added the shadow, but the shadow doesn't move with swipe.

Evan, do you have any good way to fix this?

Comment 3 by est...@chromium.org, Apr 28 2017 

I'll fix it.

Comment 4 by est...@chromium.org, Apr 28 2017

Labels: -Type-Bug -Pri-2 Pri-1 Type-Bug-Regression
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, May 1 2017 

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

commit 3d7851ff607c370234e4f93ecf66727a8f8498d9
Author: estade <estade@chromium.org>
Date: Mon May 01 17:15:47 2017

CrOS: Fix appearance of notification toasts when sliding out via gesture

This converts SlideOutView to SlideOutController, which provides all the
same functionality but can be added to any view rather than just those
that extend it.

It allows the target view to control the layer that's sliding out. For
toasts, that is the widget's layer. For notifications inside the message
center, that will continue to be the NotificationView itself.

BUG= 716429 

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

[modify] https://crrev.com/3d7851ff607c370234e4f93ecf66727a8f8498d9/ui/message_center/BUILD.gn
[modify] https://crrev.com/3d7851ff607c370234e4f93ecf66727a8f8498d9/ui/message_center/views/custom_notification_view_unittest.cc
[modify] https://crrev.com/3d7851ff607c370234e4f93ecf66727a8f8498d9/ui/message_center/views/message_center_view_unittest.cc
[modify] https://crrev.com/3d7851ff607c370234e4f93ecf66727a8f8498d9/ui/message_center/views/message_view.cc
[modify] https://crrev.com/3d7851ff607c370234e4f93ecf66727a8f8498d9/ui/message_center/views/message_view.h
[modify] https://crrev.com/3d7851ff607c370234e4f93ecf66727a8f8498d9/ui/message_center/views/message_view_factory.cc
[modify] https://crrev.com/3d7851ff607c370234e4f93ecf66727a8f8498d9/ui/message_center/views/notification_view_unittest.cc
[add] https://crrev.com/3d7851ff607c370234e4f93ecf66727a8f8498d9/ui/message_center/views/slide_out_controller.cc
[add] https://crrev.com/3d7851ff607c370234e4f93ecf66727a8f8498d9/ui/message_center/views/slide_out_controller.h
[modify] https://crrev.com/3d7851ff607c370234e4f93ecf66727a8f8498d9/ui/views/BUILD.gn
[delete] https://crrev.com/1f02534fb0fb3a45f76c16d06da2b46c0625a668/ui/views/controls/slide_out_view.cc
[delete] https://crrev.com/1f02534fb0fb3a45f76c16d06da2b46c0625a668/ui/views/controls/slide_out_view.h
Project Member

Comment 6 by bugdroid1@chromium.org, May 1 2017 

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

commit ba5c53c9efbb48cd5f54de19e996b24e63f511db
Author: sky <sky@chromium.org>
Date: Mon May 01 20:15:52 2017

Revert of CrOS: Fix appearance of notification toasts when sliding out via gesture (patchset #4 id:60001 of https://codereview.chromium.org/2849523005/ )

Reason for revert:
This appears to have broken message_center_unittests on chromium.win/Win10 Tests x64. See https://uberchromegw.corp.google.com/i/chromium.win/builders/Win10%20Tests%20x64/builds/11112/steps/message_center_unittests%20on%20Windows-10-10586 for one example.

Original issue's description:
> CrOS: Fix appearance of notification toasts when sliding out via gesture
>
> This converts SlideOutView to SlideOutController, which provides all the
> same functionality but can be added to any view rather than just those
> that extend it.
>
> It allows the target view to control the layer that's sliding out. For
> toasts, that is the widget's layer. For notifications inside the message
> center, that will continue to be the NotificationView itself.
>
> BUG= 716429 
>
> Review-Url: https://codereview.chromium.org/2849523005
> Cr-Commit-Position: refs/heads/master@{#468339}
> Committed: https://chromium.googlesource.com/chromium/src/+/3d7851ff607c370234e4f93ecf66727a8f8498d9

TBR=yoshiki@chromium.org,estade@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 716429 

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

[modify] https://crrev.com/ba5c53c9efbb48cd5f54de19e996b24e63f511db/ui/message_center/BUILD.gn
[modify] https://crrev.com/ba5c53c9efbb48cd5f54de19e996b24e63f511db/ui/message_center/views/custom_notification_view_unittest.cc
[modify] https://crrev.com/ba5c53c9efbb48cd5f54de19e996b24e63f511db/ui/message_center/views/message_center_view_unittest.cc
[modify] https://crrev.com/ba5c53c9efbb48cd5f54de19e996b24e63f511db/ui/message_center/views/message_view.cc
[modify] https://crrev.com/ba5c53c9efbb48cd5f54de19e996b24e63f511db/ui/message_center/views/message_view.h
[modify] https://crrev.com/ba5c53c9efbb48cd5f54de19e996b24e63f511db/ui/message_center/views/message_view_factory.cc
[modify] https://crrev.com/ba5c53c9efbb48cd5f54de19e996b24e63f511db/ui/message_center/views/notification_view_unittest.cc
[delete] https://crrev.com/b042a7e565df1572caa21084289f6e37db216684/ui/message_center/views/slide_out_controller.cc
[delete] https://crrev.com/b042a7e565df1572caa21084289f6e37db216684/ui/message_center/views/slide_out_controller.h
[modify] https://crrev.com/ba5c53c9efbb48cd5f54de19e996b24e63f511db/ui/views/BUILD.gn
[add] https://crrev.com/ba5c53c9efbb48cd5f54de19e996b24e63f511db/ui/views/controls/slide_out_view.cc
[add] https://crrev.com/ba5c53c9efbb48cd5f54de19e996b24e63f511db/ui/views/controls/slide_out_view.h
Project Member

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

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

commit d3c675e53e8b9c17d77e47107d04df8d0baae069
Author: estade <estade@chromium.org>
Date: Wed May 03 14:27:34 2017

reland "CrOS: Fix appearance of notification toasts when sliding out via gesture"

This re-lands 3d7851ff607c370234e4f93ecf6
original review: https://codereview.chromium.org/2849523005

This converts SlideOutView to SlideOutController, which provides all the
same functionality but can be added to any view rather than just those
that extend it.

It allows the target view to control the layer that's sliding out. For
toasts, that is the widget's layer. For notifications inside the message
center, that will continue to be the NotificationView itself.

BUG= 716429 
Review-Url: https://codereview.chromium.org/2849523005

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

[modify] https://crrev.com/d3c675e53e8b9c17d77e47107d04df8d0baae069/ui/message_center/BUILD.gn
[modify] https://crrev.com/d3c675e53e8b9c17d77e47107d04df8d0baae069/ui/message_center/views/custom_notification_view_unittest.cc
[modify] https://crrev.com/d3c675e53e8b9c17d77e47107d04df8d0baae069/ui/message_center/views/message_center_view_unittest.cc
[modify] https://crrev.com/d3c675e53e8b9c17d77e47107d04df8d0baae069/ui/message_center/views/message_view.cc
[modify] https://crrev.com/d3c675e53e8b9c17d77e47107d04df8d0baae069/ui/message_center/views/message_view.h
[modify] https://crrev.com/d3c675e53e8b9c17d77e47107d04df8d0baae069/ui/message_center/views/message_view_factory.cc
[modify] https://crrev.com/d3c675e53e8b9c17d77e47107d04df8d0baae069/ui/message_center/views/notification_view_unittest.cc
[add] https://crrev.com/d3c675e53e8b9c17d77e47107d04df8d0baae069/ui/message_center/views/slide_out_controller.cc
[add] https://crrev.com/d3c675e53e8b9c17d77e47107d04df8d0baae069/ui/message_center/views/slide_out_controller.h
[modify] https://crrev.com/d3c675e53e8b9c17d77e47107d04df8d0baae069/ui/views/BUILD.gn
[delete] https://crrev.com/e3cfa1285898021bfe5bd4e62ddd94d0e97e6e93/ui/views/controls/slide_out_view.cc
[delete] https://crrev.com/e3cfa1285898021bfe5bd4e62ddd94d0e97e6e93/ui/views/controls/slide_out_view.h

Comment 8 by est...@chromium.org, May 3 2017

Cc: est...@chromium.org
 Issue 718008  has been merged into this issue.

Comment 9 by est...@chromium.org, May 9 2017

Labels: Merge-Request-59
I would like to merge this, although the patch breaks ARC notifications. Do we care about that in m59?
Project Member

Comment 10 by sheriffbot@chromium.org, May 9 2017

Labels: -Merge-Request-59 Merge-Review-59 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the 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 11 by est...@chromium.org, May 10 2017

Cc: gkihumba@chromium.org
+Grace, requesting to merge d3c675e53e8b9c17d77e47107d04df8d0baae069

Comment 12 by gkihumba@google.com, May 11 2017

Labels: Merge-Approved-59

Comment 13 by est...@chromium.org, May 11 2017 

yoshiki, question for you in #9

Comment 14 by yoshiki@chromium.org, May 12 2017

Blockedon: 719414 718956 719407
We need to care about M59 as well. So please wait for your merge until all of these fixes are done.
-  Issue 719414  (done)
- Issue 718956 (done)
-  Issue 719407  (wip)
Project Member

Comment 15 by sheriffbot@chromium.org, May 15 2017

Cc: gkihumba@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

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

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

commit dd91791606f52ac9b6d627b7ed3fe67ab52db50d
Author: Evan Stade <estade@chromium.org>
Date: Fri May 19 15:07:50 2017

reland "CrOS: Fix appearance of notification toasts when sliding out via gesture"

This re-lands 3d7851ff607c370234e4f93ecf6
original review: https://codereview.chromium.org/2849523005

This converts SlideOutView to SlideOutController, which provides all the
same functionality but can be added to any view rather than just those
that extend it.

It allows the target view to control the layer that's sliding out. For
toasts, that is the widget's layer. For notifications inside the message
center, that will continue to be the NotificationView itself.

BUG= 716429 
Review-Url: https://codereview.chromium.org/2849523005

Review-Url: https://codereview.chromium.org/2855763003
Cr-Original-Commit-Position: refs/heads/master@{#468962}
Review-Url: https://codereview.chromium.org/2892183002 .
Cr-Commit-Position: refs/branch-heads/3071@{#630}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/dd91791606f52ac9b6d627b7ed3fe67ab52db50d/ui/message_center/BUILD.gn
[modify] https://crrev.com/dd91791606f52ac9b6d627b7ed3fe67ab52db50d/ui/message_center/views/custom_notification_view_unittest.cc
[modify] https://crrev.com/dd91791606f52ac9b6d627b7ed3fe67ab52db50d/ui/message_center/views/message_center_view_unittest.cc
[modify] https://crrev.com/dd91791606f52ac9b6d627b7ed3fe67ab52db50d/ui/message_center/views/message_view.cc
[modify] https://crrev.com/dd91791606f52ac9b6d627b7ed3fe67ab52db50d/ui/message_center/views/message_view.h
[modify] https://crrev.com/dd91791606f52ac9b6d627b7ed3fe67ab52db50d/ui/message_center/views/message_view_factory.cc
[modify] https://crrev.com/dd91791606f52ac9b6d627b7ed3fe67ab52db50d/ui/message_center/views/notification_view_unittest.cc
[add] https://crrev.com/dd91791606f52ac9b6d627b7ed3fe67ab52db50d/ui/message_center/views/slide_out_controller.cc
[add] https://crrev.com/dd91791606f52ac9b6d627b7ed3fe67ab52db50d/ui/message_center/views/slide_out_controller.h
[modify] https://crrev.com/dd91791606f52ac9b6d627b7ed3fe67ab52db50d/ui/views/BUILD.gn
[delete] https://crrev.com/9ec113cbb773ee79fff67c5e22fbf1b4e12cbc5f/ui/views/controls/slide_out_view.cc
[delete] https://crrev.com/9ec113cbb773ee79fff67c5e22fbf1b4e12cbc5f/ui/views/controls/slide_out_view.h

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

Status: Fixed (was: Started)

Comment 18 by sdantul...@chromium.org, May 25 2017

Status: Verified (was: Fixed)
9460.50.0, 59.0.3071.71)

Comment 19 by gkihumba@google.com, Jun 16 2017

Labels: -Hotlist-Merge-Review -Merge-Review-59
►

Sign in to add a comment