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

Issue 834101 link

Starred by 1 user
Status: Fixed
Owner:
finnur@chromium.org
Closed: Apr 2018
Cc:
chengx@chromium.org
finnur@chromium.org
█ bustamante@chromium.org
peter@chromium.org
Components:
EstimatedDays: ----
NextAction: 2018-04-26
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Notification title gets overridden when lateral notification and previous notifications are from same source in Windows Action centre.

Project Member Reported by pbomm...@chromium.org, Apr 17 2018 
Chrome Version: 67.0.3393.4
OS: Win10

What steps will reproduce the problem?
1. Install and launch Chrome 67.0.3393.4 or above 
2. Click on Chrome profile icon on top right side of the screen --> Manage people --> on the dialog click on Add person(profile 2)
3. Launch both profile 1 and profile 2
4. From profile 1 navigate to https://tests.peter.sh/notification-generator/ and make sure that "Title : Short sentence(LTR)
5. Click on "Display the notification"
6. Notification should display at the bottom right of the screen.(with a pinky drunken cat icon in it.)
7. Wait for ~7sec or more so that the notification on desktop disappears.
8. Click on the message icon on extreme right bottom of windows task bar(similar to message icon) to make sure there is a notification under Google Chrome Dev as "Notification title notification content"
9. From profile 2 navigate to https://tests.peter.sh/notification-generator/ and make sure that "Title : Long sentence(LTR)
10 repeat step 5 



What is the expected result?

Profile 1 notification should be  : "Notification title notification content with pinky drunken cat icon in it"
Profile 2 notification should be  : "Hamburgers: the cornerstone of any nutritious breakfast.Ch-cheeseburger notification content with pinky drunken cat icon in it"


What happens instead?
Profile 1 title changed to Profile 2 title.

Comment 1 by peter@chromium.org, Apr 18 2018

Cc: peter@chromium.org
Owner: finnur@chromium.org
We create the IToastNotification* in NotificationPlatformBridgeWin::GetToastNotification() and set two properties that determine uniqueness:

  - put_Group(), hardcoded to the string "Notifications"
  - put_Tag(), which is set to a hash of |notification.id()|

The notification's ID doesn't include the profile information, so in this case we end up posting notifications from the same profile sharing the "group" and "tag" properties. I suspect that's what causing this issue - as it now looks like a replacement to the OS.

Two ideas:

  1) Include the |profile_id|/|incognito| properties in the Tag.
  2) Use the group to carry the profile uniqueness bits.

Finnur, could you please take a look?

Comment 2 by finnur@chromium.org, Apr 23 2018 

For posterity, it is important to note that the installation part (step 1) is crucial to repro'ing this bug, but an alternative to installing is making sure both profiles are completely fresh (if you are reusing an old profile, make sure to reset it).
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 24 2018 

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

commit 2c8854dd178f1a475200c52c254825117a76d010
Author: Finnur Thorarinsson <finnur@chromium.org>
Date: Tue Apr 24 14:59:04 2018

Win Native Notifications: Add profile id and incognito status to hashed tag.

Also fix a parameter mismatch in the Close function.

Bug:  834101 ,  826817 ,  734095 
Change-Id: I94bef0defae6c7179605db23bfc8ff0020433f75
Reviewed-on: https://chromium-review.googlesource.com/1023231
Commit-Queue: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553105}
[modify] https://crrev.com/2c8854dd178f1a475200c52c254825117a76d010/chrome/browser/notifications/notification_platform_bridge_win.cc
[modify] https://crrev.com/2c8854dd178f1a475200c52c254825117a76d010/chrome/browser/notifications/notification_platform_bridge_win.h
[modify] https://crrev.com/2c8854dd178f1a475200c52c254825117a76d010/chrome/browser/notifications/notification_platform_bridge_win_unittest.cc

Comment 4 by finnur@chromium.org, Apr 24 2018

Status: Fixed (was: Assigned)
Project Member

Comment 5 by blocker...@chrome-infra-auth.iam.gserviceaccount.com, Apr 24 2018

Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-67; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-67 label, otherwise remove Merge-TBD label. Thanks.

Comment 6 by gov...@chromium.org, Apr 24 2018 

finnur@, pls request a merge to M67 once CL listed at #3 is well baked/verified in canary?

Comment 7 by gov...@chromium.org, Apr 25 2018

NextAction: 2018-04-26
How is the change listed at #3 looking in canary?

Comment 8 by finnur@chromium.org, Apr 26 2018

Labels: -Merge-TBD -ReleaseBlock-Stable -M-67
It's looking fine, but I had a discussion with Peter and the decision was we don't need this fix in M67.

Comment 9 by monor...@bugs.chromium.org, Apr 26 2018 

The NextAction date has arrived: 2018-04-26
►

Sign in to add a comment