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

Issue 703717 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Sometimes mousing over HTML5 notifications don't display the X to close the notification

Project Member Reported by atwilson@chromium.org, Mar 21 2017

Issue description

1) Display an HTML5 notification
2) Quickly mouse over the notification such that your mouse is where the 'X' close icon would be.
3) See that the notification doesn't display the 'X'.

I'm actually totally not a fan of this UI change in general, because as a user there's no way to tell how to close the notification until you mouse over the notification.

I've seen this behavior on both cros and linux.

Tentatively assigning to Yoshiki as this may be a result of his recent notification changes.
 
Cc: yoshiki@chromium.org
Owner: yhanada@chromium.org
Status: Assigned (was: Untriaged)
Hanada-san, I think this is already fixed, right?
NotificationView uses OnMouseEntered() and OnMouseExited() to update the visibility of the close button.
I noticed that sometime these methods are not called when quickly mouse over the notification, so IMO there is nothing to do in message_center side.
Project Member

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

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

commit 680cdf421c7cf607910ae1608f75541e5f03281f
Author: yhanada <yhanada@chromium.org>
Date: Mon May 29 04:35:46 2017

Call UpdateControlButtonsVisibility() in OnMouseMoved().

This is workaround for the issue that OnMouseEntered() is sometimes
not called.

BUG= 703717 

Change-Id: I80d6c305f2577775cb7a7cb7d960013950a5d201
Reviewed-on: https://chromium-review.googlesource.com/516685
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#475286}
[modify] https://crrev.com/680cdf421c7cf607910ae1608f75541e5f03281f/ui/message_center/views/notification_view.cc
[modify] https://crrev.com/680cdf421c7cf607910ae1608f75541e5f03281f/ui/message_center/views/notification_view.h

Labels: -Pri-3 Merge-Request-59 M-59 Merge-Request-60 OS-Chrome Pri-1
I'm requesting the merge to M-59 and M-60 because there is no way for users to close the notification if the close button doesn't appear.
Status: Started (was: Assigned)
Project Member

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

Labels: -Merge-Request-59 Merge-Review-59 Hotlist-Merge-Review
This bug requires manual review: We are only 7 days from stable.
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
Project Member

Comment 7 by sheriffbot@chromium.org, May 30 2017

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

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

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

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

commit 0f7330516ee00830359f6d3f59fd585e8b1451ab
Author: yoshiki iguchi <yoshiki@chromium.org>
Date: Tue May 30 05:09:16 2017

Call UpdateControlButtonsVisibility() in OnMouseMoved().

This is workaround for the issue that OnMouseEntered() is sometimes
not called.

BUG= 703717 

Change-Id: I80d6c305f2577775cb7a7cb7d960013950a5d201
Reviewed-on: https://chromium-review.googlesource.com/516685
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#475286}
(cherry picked from commit 680cdf421c7cf607910ae1608f75541e5f03281f)

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

[modify] https://crrev.com/0f7330516ee00830359f6d3f59fd585e8b1451ab/ui/message_center/views/notification_view.cc
[modify] https://crrev.com/0f7330516ee00830359f6d3f59fd585e8b1451ab/ui/message_center/views/notification_view.h

Project Member

Comment 9 by bugdroid1@chromium.org, May 30 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c86ea583c31dd452c9322fa99bc9016b566ab8c9

commit c86ea583c31dd452c9322fa99bc9016b566ab8c9
Author: yoshiki iguchi <yoshiki@chromium.org>
Date: Tue May 30 05:11:15 2017

Call UpdateControlButtonsVisibility() in OnMouseMoved().

This is workaround for the issue that OnMouseEntered() is sometimes
not called.

BUG= 703717 

Change-Id: I80d6c305f2577775cb7a7cb7d960013950a5d201
Reviewed-on: https://chromium-review.googlesource.com/516685
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#475286}
(cherry picked from commit 680cdf421c7cf607910ae1608f75541e5f03281f)

Review-Url: https://codereview.chromium.org/2910003003 .
Cr-Commit-Position: refs/branch-heads/3112@{#22}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/c86ea583c31dd452c9322fa99bc9016b566ab8c9/ui/message_center/views/notification_view.cc
[modify] https://crrev.com/c86ea583c31dd452c9322fa99bc9016b566ab8c9/ui/message_center/views/notification_view.h

Status: Fixed (was: Started)
I merged the CL on behalf of yhanada@.
Labels: -Hotlist-Merge-Review -Merge-Review-59

Comment 12 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment