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

Issue 774983 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Nothing happens after clicking on the bubble for extension.

Reported by aiman.an...@etouch.net, Oct 16 2017

Issue description

Chrome Version: 63.0.3239.7 (Official Build)cf45ad0d825782ed20db86e21a1b92d0c725c41b-refs/branch-heads/3239@{#9} (64-bit)

OS:Mac(10.12.6).

Test URL: https://chrome.google.com/webstore/detail/pushbullet/chlffgpmiacpedhhbkiomidkjlcfhogd?utm_source=chrome-ntp-icon

Steps to reproduce:
1.Launch Chrome, go to the above link and add the above extension.
2.Click on the sign-in bubble that appears on installing the extension and observe. 

Actual Result: Nothing happens after clicking on the bubble.
Expected Result: Clicking on the bubble should navigate to sign-in page. 

This is regression issue broken in ‘M-63’ and below per-revision bisect result

Using the per-revision bisect providing the bisect results,
Good Build: 63.0.3210.0 (Revision:500530)
Bad Build: 63.0.3211.0 (Revision:500753)

You are probably looking for a change made after 500657 (known good), but no later than 500658 (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/858e034f3d6a4e41464d450866913da1286f4f65..ae64ba9ecc216db5b618a70e4489bee4eb7943de

Suspect: https://chromium.googlesource.com/chromium/src/+/ae64ba9ecc216db5b618a70e4489bee4eb7943de

@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.

Thank You!

 
Actual Result.mov
3.2 MB Download
Expected Result.mov
3.8 MB Download
Labels: ReleaseBlock-Stable
Tagging with blocker label, please undo if not the case.
Note: Unable to reproduce the issue on Win(7,8,10) and Linux(14.04 LTS).

Comment 3 by peter@chromium.org, Oct 19 2017

 Issue 776227  has been merged into this issue.
@peter: Request you to please take a look into it. issue is tagged with a stable blocker and M63 is set to be pushed to Beta soon.

Thanks.!

Comment 5 by ni...@google.com, Oct 26 2017

in  Issue 770332 #c10, I created a test extension to repro this bug
test-notification (1).zip
4.1 KB Download
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 26 2017

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

commit 178f4e39143777e5db15a6b7abc992ceb33df4ce
Author: Peter Beverloo <peter@chromium.org>
Date: Thu Oct 26 18:15:25 2017

Don't use -1 in Mac OS native notification action index responses

The negative value used to represent that no action button was used, but
that has since been changed to use base::Optional instead. This is
causing code on Mac to unintentionally believe that a button was used
anyway, which can lead to unexpected behaviour.

BUG= 774983 

Change-Id: I8c13aff77b77772077c9145b705d84904eb67a55
Reviewed-on: https://chromium-review.googlesource.com/730753
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511882}
[modify] https://crrev.com/178f4e39143777e5db15a6b7abc992ceb33df4ce/chrome/browser/notifications/notification_platform_bridge_mac.mm
[modify] https://crrev.com/178f4e39143777e5db15a6b7abc992ceb33df4ce/chrome/browser/ui/cocoa/notifications/notification_constants_mac.h
[modify] https://crrev.com/178f4e39143777e5db15a6b7abc992ceb33df4ce/chrome/browser/ui/cocoa/notifications/notification_response_builder_mac.mm
[modify] https://crrev.com/178f4e39143777e5db15a6b7abc992ceb33df4ce/chrome/browser/ui/cocoa/notifications/notification_response_builder_mac_unittest.mm

Comment 7 by peter@chromium.org, Oct 27 2017

Components: UI>Notifications
Labels: Merge-Request-63
#c5 - thank you, I confirmed it's working in the latest Canary.

Requesting merge based on that.
Project Member

Comment 8 by sheriffbot@chromium.org, Oct 27 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 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), gkihumba@(ChromeOS), govind@(Desktop)

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

Comment 9 by gov...@chromium.org, Oct 27 2017

Before we approve merge to M63, could you pls confirm change is well baked/verified in Canary, having enough automation tests coverage and safe to merge?
Peter has verified the fix and updated at comment#7.
Labels: -Merge-Review-63 Merge-Approved-63
Approving merge to M63 branch 3239 based on comment #7. Please merge by 4:00 PM PT Monday (10/30), so we can pick it up for next week Beta release. Thank you.
[Bulk Edit]
URGENT - PTAL.
M63 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP. Thank you.

Project Member

Comment 13 by sheriffbot@chromium.org, Oct 31 2017

Cc: gov...@chromium.org
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 14 by bugdroid1@chromium.org, Oct 31 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/103dbb8624a1bcc16dc146881e38aa72c8831324

commit 103dbb8624a1bcc16dc146881e38aa72c8831324
Author: Peter Beverloo <peter@chromium.org>
Date: Tue Oct 31 15:12:50 2017

Don't use -1 in Mac OS native notification action index responses

The negative value used to represent that no action button was used, but
that has since been changed to use base::Optional instead. This is
causing code on Mac to unintentionally believe that a button was used
anyway, which can lead to unexpected behaviour.

BUG= 774983 

Change-Id: I8c13aff77b77772077c9145b705d84904eb67a55
Reviewed-on: https://chromium-review.googlesource.com/730753
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#511882}(cherry picked from commit 178f4e39143777e5db15a6b7abc992ceb33df4ce)
Reviewed-on: https://chromium-review.googlesource.com/746647
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#314}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/103dbb8624a1bcc16dc146881e38aa72c8831324/chrome/browser/notifications/notification_platform_bridge_mac.mm
[modify] https://crrev.com/103dbb8624a1bcc16dc146881e38aa72c8831324/chrome/browser/ui/cocoa/notifications/notification_constants_mac.h
[modify] https://crrev.com/103dbb8624a1bcc16dc146881e38aa72c8831324/chrome/browser/ui/cocoa/notifications/notification_response_builder_mac.mm
[modify] https://crrev.com/103dbb8624a1bcc16dc146881e38aa72c8831324/chrome/browser/ui/cocoa/notifications/notification_response_builder_mac_unittest.mm

M63 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge to M63 ASAP. Thank you.
Status: Fixed (was: Assigned)

Sign in to add a comment