Regression: Can't click desktop notifications |
||||||||
Issue descriptionChrome Version: 67.0.3377.0 (and a few versions before as well) OS: Win10 What steps will reproduce the problem? (1) Have a desktop notification pop (tried Hangouts and Calendar) (2) Click it What is the expected result? Focuses the tab that popped it What happens instead? Nothing (the notification isn't even dismissed) The rest of the browser is responsive (I can manually navigate to Hangouts/Calendar and can also manually close the notification with the X).
,
Mar 22 2018
Calendar is clearly a website. Hangouts is an app (which I believe is a WebContents in its own window?)
,
Mar 23 2018
,
Mar 23 2018
The report doesn't say, but I'm guessing this is with the new notifications (black background), as opposed to the old (white background)? If it is the former, then this is because Xi's fix is not in Canary yet. Commit hash: If19f1c802b7da3b1cd0476c0a585138a2d2edcaa
,
Mar 23 2018
,
Mar 23 2018
No, this is with white background
,
Mar 23 2018
Then it's still the old notification. Did you turn on the flag: chrome://flags/#enable-native-notifications, Gab?
,
Mar 24 2018
I just saw this too with Tampermonkey desktop notification for updates - could click X to close but clicking actual notification now does nothing. This happened once before so does appear to be regression issue. 67.0.3377.0
,
Mar 24 2018
@#8: nope, no special flags, just Canary on corp.
,
Mar 25 2018
@#10: This feature is still behind a flag -- chrome://flags/#enable-native-notifications, so you need to turn it on. We are still working on all kinds of surveys which are required for launch.
,
Mar 25 2018
@#11: what do you mean by "This happened once ..."? Can you provide more information please?
,
Mar 25 2018
@#10: you mean it's expected that the feature not behind flag is broken..?
,
Mar 25 2018
@#12: For the old notification feature on Windows (not behind flag), peter@ or finnur@ should have better answers than I do.
,
Mar 26 2018
Thanks for confirming, Gabriel. It seems like there's a regression in the old-style notifications and we need to get to the bottom of it. I'm more versed in the new-style notifications, but there's a chance our changes there broke the old-style notifications. I'll take a look. Essentially, to repro, I should be able to create a meeting in Google Calendar on my test account, wait for the reminder to pop up and click it, right? Does it repro consistently for you, Gabriel?
,
Mar 26 2018
Yes, repros 100% with desktop notifications from all sources (hangouts might be easier for a repro?).
,
Mar 26 2018
Removing Xi, since this is not part of native notifications. Regression range is: https://chromium.googlesource.com/chromium/src/+log/d5e692b8160c82e8a7152b8592afac99d2f63808..1ccdac4afa1d98f721ed3e16f89ee48a05d1603c This CL looks like a very promising candidate... https://chromium-review.googlesource.com/c/chromium/src/+/962556 Adding committer + reviewer to the thread.
,
Mar 27 2018
Seems we have to return true from OnMousePressed/Dragged also from MessageView. I'll fix it. https://chromium-review.googlesource.com/c/chromium/src/+/962556/3/ui/message_center/views/notification_view_md.cc#752
,
Mar 27 2018
,
Mar 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bc09d4d561afd7ab8257f4ba875e2990cfc1d40c commit bc09d4d561afd7ab8257f4ba875e2990cfc1d40c Author: Tetsui Ohkubo <tetsui@chromium.org> Date: Tue Mar 27 08:41:28 2018 Fix notification click. In https://crrev.com/c/962556, MessageView started to use OnMouseReleased for detecting notification click. OnMouseReleased requires OnMousePressed and OnMouseDragged to return true in order to receive events. However, they are only overridden in NotificationViewMD, thus NotificationView's click event is broken. TEST=manual BUG= 824789 Change-Id: Ie4e0c803ddf8425eebaad30c927c356bab699b69 Reviewed-on: https://chromium-review.googlesource.com/981873 Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> Cr-Commit-Position: refs/heads/master@{#546064} [modify] https://crrev.com/bc09d4d561afd7ab8257f4ba875e2990cfc1d40c/ui/message_center/views/message_view.cc [modify] https://crrev.com/bc09d4d561afd7ab8257f4ba875e2990cfc1d40c/ui/message_center/views/message_view.h
,
Mar 27 2018
The Regression CL initially landed in 67.0.3376.0 so no need to merge.
,
Mar 27 2018
Thanks for the quick turnaround!
,
Mar 28 2018
Able to reproduce this issue on Windows 10 on the reported version 67.0.3377.0 and the issue is fixed on the latest Canary 67.0.3382.0. On clicking on the Hangouts desktop notification, getting redirected to the Hangouts Chat window. Attached is the screen cast for reference. Hence adding TE verified labels as the fix is working as intended. Thanks.. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by peter@chromium.org
, Mar 22 2018