Issue metadata
Sign in to add a comment
|
Regression: Clicking on setting button on notification alert does not navigate to chrome://settings page.
Reported by
aiman.an...@etouch.net,
Jan 2 2018
|
||||||||||||||||||||||
Issue descriptionChrome Version: 65.0.3309.0 (Official Build) 7f5c6c7cbfeb43b7cb082829d8b05a0197d9b151-refs/heads/master@{#526409} (64-bit). OS: Mac(10,12,6, 10.13.1, 10.13.2). Test URL: https://tests.peter.sh/notification-generator/#image=1 What steps will reproduce the problem? 1. Launch chrome, navigate to the above URL and click on Allow Button from notification bubble. 2. Scroll down and click on ‘Display the Notification’ button. 3. Once Notification Alerts appear click on Settings button and observe. Actual: Chrome://settings page does not open after clicking on setting button on Notification Alert. Expected: Chrome://settings page should open upon clicking on settings button on Notification Alerts. This is a regression issue, broken in M-65 series, Using the per-revision bisect providing the bisect results, Good Build:65.0.3285.0 Bad Build:65.0.3286.0 You are probably looking for a change made after 521747 (known good), but no later than 521748 (first known bad). CHANGE-LOG URL: The script might not always return single CL as suspect as some perf builds might get missing due to failure. https://chromium.googlesource.com/chromium/src/+log/44e2e5f8caf153b0ad01425665ec17bbc419a76b..884488b0a16b509932e7b38b2915646f1b2f2e3a Suspect:https://chromium.googlesource.com/chromium/src/+/884488b0a16b509932e7b38b2915646f1b2f2e3a peter@:Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Note: 1. After clicking on Setting button on Notification Alert, click on 'Secure' chip in omnibox and observe.(Notification gets blocked) 2. Issue is not reproducible on Windows(7,8,8.1,10) and Linux(14.04 LTS).
,
Jan 8 2018
Still we are facing the same issue on Mac 10.12.6 using chrome latest Canary-65.0.3314.0 as per C#0. peter@, Could you please take a look and update the thread accordingly. Thanks..!
,
Jan 16 2018
Friendly ping to get an update on this issue as it is marked as stable blocker & still we are able to reproduce the issue on latest Canary-65.0.3322.3. Thanks..!
,
Jan 22 2018
peter@, Could you please take a look and update the thread accordingly as it is marked as M65 stable blocker. Thanks..!
,
Jan 30 2018
Adding stability sheriff for further help. Thanks..!
,
Jan 30 2018
Mac triage: Stability sheriffs are for crashes, not this kind of bug. Untagging Stability-Sheriff-Desktop, tagging estade@ (who reviewed the suspect CL).
,
Jan 30 2018
I'll take a look. Does this only happen on Mac?
,
Jan 30 2018
,
Jan 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/97658ebcea02e8e508a2957012c72fc9ee7bd04c commit 97658ebcea02e8e508a2957012c72fc9ee7bd04c Author: Peter Beverloo <peter@chromium.org> Date: Wed Jan 31 21:15:53 2018 Fix the settings button on native Mac notifications An enumeration got updated that has to be duplicated due to layering reasons, but its peer didn't get updated. This CL updates the second enumeration and adds static asserts to make sure that they keep in sync going forward. Bug: 798262 Change-Id: Ib01d3d0635bed3f7e232e2405e0a6a59f65bf3db Reviewed-on: https://chromium-review.googlesource.com/893379 Commit-Queue: Peter Beverloo <peter@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Cr-Commit-Position: refs/heads/master@{#533414} [modify] https://crrev.com/97658ebcea02e8e508a2957012c72fc9ee7bd04c/chrome/browser/notifications/notification_common.h [modify] https://crrev.com/97658ebcea02e8e508a2957012c72fc9ee7bd04c/chrome/browser/ui/cocoa/notifications/notification_response_builder_mac.h [modify] https://crrev.com/97658ebcea02e8e508a2957012c72fc9ee7bd04c/chrome/browser/ui/cocoa/notifications/notification_response_builder_mac.mm [modify] https://crrev.com/97658ebcea02e8e508a2957012c72fc9ee7bd04c/chrome/browser/ui/cocoa/notifications/notification_response_builder_mac_unittest.mm
,
Feb 2 2018
,
Feb 6 2018
Update : Retested above issue Mac(10.12.6, 10.13.1, 10.13.4) OS using latest Canary #66.0.3341.0 and issue is fixed. Now, able to navigate to chrome://settings page upon clicking Settings button on Notifications Alerts. Kindly review the attached screen-cast.
,
Feb 13 2018
Just to update: Still seeing the same issue on latest beta-65.0.3325.51 but no issue observed on stable-64.0.3282.140 and Canary-66.0.3346.0.So could you please merge this to M65 as it is marked as stable blocker for M65? Thanks..!
,
Feb 13 2018
I never requested merge, sorry! Hereby done.
,
Feb 13 2018
This bug requires manual review: M65 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 13 2018
Approving merge to M65 branch 3325 based on comment #11. Please merge ASAP so we can pick it up for tomorrow's beta release. Thank you.
,
Feb 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/39574ff0ada36bf28292fcf62aa81e8f7b070fff commit 39574ff0ada36bf28292fcf62aa81e8f7b070fff Author: Peter Beverloo <peter@chromium.org> Date: Tue Feb 13 17:34:58 2018 Fix the settings button on native Mac notifications An enumeration got updated that has to be duplicated due to layering reasons, but its peer didn't get updated. This CL updates the second enumeration and adds static asserts to make sure that they keep in sync going forward. Bug: 798262 Change-Id: Ib01d3d0635bed3f7e232e2405e0a6a59f65bf3db Reviewed-on: https://chromium-review.googlesource.com/893379 Commit-Queue: Peter Beverloo <peter@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#533414}(cherry picked from commit 97658ebcea02e8e508a2957012c72fc9ee7bd04c) Reviewed-on: https://chromium-review.googlesource.com/916641 Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#448} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/39574ff0ada36bf28292fcf62aa81e8f7b070fff/chrome/browser/notifications/notification_common.h [modify] https://crrev.com/39574ff0ada36bf28292fcf62aa81e8f7b070fff/chrome/browser/ui/cocoa/notifications/notification_response_builder_mac.h [modify] https://crrev.com/39574ff0ada36bf28292fcf62aa81e8f7b070fff/chrome/browser/ui/cocoa/notifications/notification_response_builder_mac.mm [modify] https://crrev.com/39574ff0ada36bf28292fcf62aa81e8f7b070fff/chrome/browser/ui/cocoa/notifications/notification_response_builder_mac_unittest.mm
,
Feb 13 2018
M65 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.
,
Feb 13 2018
Fixed following the merge in #16. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ranjitkan@chromium.org
, Jan 2 2018