New issue
Advanced search Search tips

Issue 825866 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: A second click on the Extension icon on the Toolbar doesn't hide the Extension Menu

Project Member Reported by meh...@chromium.org, Mar 26 2018

Issue description

Chrome Version: Canary Version 67.0.3379.0
OS: macOS 10.12.6

What steps will reproduce the problem?
(1) Install an extension: e.g https://chrome.google.com/webstore/detail/adblock/gighmmpiobklfepjocnamgkkbiglidom
(2) Click on the extension's Toolbar icon, so that the menu appears
(3) Click again on the extension's Toolbar icon

What is the expected result?
The menu should disappear.

What happens instead?
The menu reappears.

Please use labels and text to provide additional information.
This is a regression. Works fine in latest Chrome Stable 65. 

If you need more information, please let me know.


 
Labels: -Needs-Bisect hasbisect-per-revision ReleaseBlock-Stable Triaged-ET RegressedIn-67 M-67 Target-67 FoundIn-67 Needs-Triage-M67 OS-Linux OS-Windows
Owner: robliao@chromium.org
Status: Assigned (was: Untriaged)
Able to reproduce the issue on reported chrome version 67.0.3379.0 using Windows-10, Mac 10.12.6/10.13.3 & Ubuntu 14.04  hence providing Bisect Info
Bisect Info:
================
Good build: 67.0.3362.0
Bad build: 67.0.3364.0

You are probably looking for a change made after 540879 (known good), but no later than 540880 (first known bad).

https://chromium.googlesource.com/chromium/src/+log/7bea94e12121bc43c45d68779803b5628f7ebbf1..a0259b3ae165b0673416f02848bed75082d639f4

Reviewed-on: https://chromium-review.googlesource.com/941883

@Robert Liao: Please confirm the issue and help in re-assigning if it is not related to your change.
Adding ReleaseBlock-Stable as it is seems a receent break, feel free to remove it if not applicable.

Thanks!
Cc: tapted@chromium.org
This is actually one of the side-effects of moving to views. The menu gets dismissed before the click occurs and this behavior occasionally occurs on Windows.

Comment 3 by tapted@chromium.org, Mar 27 2018

It's possible that nerfing the ui::BubbleCloser in BubbleDialogDelegate will help. But I think this applies to buttons in general. They just need to keep track of their `down` state and not activate until their popup/menu lets go of the `down` state. I think LocationBarDecoration has some generic code for this - maybe we can pull that out.
Adding related note from https://bugs.chromium.org/p/chromium/issues/detail?id=816421#c2

For clicking the button twice: Mac seems to handle focusing on the window a bit differently compared to Windows. On Windows, the click on the button is suppressed and as a result, another show doesn't occur. The normal parent window received active code dismissed the dialog.

On Mac, we do the parent window receiving active dismissal, except the button also receives a click, triggering the flow again.
I'll also note this isn't a problem in MacViews, only Cocoa + Views ExtensionPopup.
The reason this sometimes works and sometimes doesn't work is it depends on when the click arrives in the message queue.

Sometimes the click is enqueued before the window finally closes, which triggers the expected behavior (however racy) and is incidentally what Cocoa has.

Sometimes the click is enqueued after the window closes, and this triggers the the extension popup to reshow as it thinks it's no longer showing.

Comment 7 Deleted

...and the main reason this works in Cocoa is because of the close animation. If the click arrives during the close animation, Cocoa will correctly not show the extension popup. If the click arrives after close animation completes (and thus after the popup is destroyed), then the popup will reappear.
Status: Started (was: Assigned)
Project Member

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

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

commit a9e005a04cab85ce8a9f040cbb8c19c1957effb8
Author: Robert Liao <robliao@chromium.org>
Date: Tue Apr 03 17:41:02 2018

Port Views MenuButton Pressed Lock Logic to Cocoa BrowserActionButton

Before this change, when the user clicks on the button to dismiss the
Extension Popup, the popup would dismiss and destroy. Next, the
incoming mouse-up would note that the popup is already gone and show
the popup.

We want the popup to stay closed if it was dismissed by clicking the
action button.

The Views Browser doesn't have this problem because MenuButton tracks
the last time the locked state was released and suppresses any
activation if it occurs too soon.

This change simply carries similar logic from Views MenuButton to
Cocoa BrowserActionButton.

BUG= 825866 

Change-Id: I089f0ab8658277ae8cbf71e2813e62aeb3c5505f
Reviewed-on: https://chromium-review.googlesource.com/991596
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547755}
[modify] https://crrev.com/a9e005a04cab85ce8a9f040cbb8c19c1957effb8/chrome/browser/ui/cocoa/extensions/browser_action_button.mm

Status: Fixed (was: Started)
Labels: Needs-Feedback
Tested the issue on latest chrome version 67.0.3388.0 using Ubuntu 14.04 with steps mentioned below:
1) Launched chrome reported version and installed the extension from URL: https://chrome.google.com/webstore/detail/adblock/gighmmpiobklfepjocnamgkkbiglidom
2) Clicked on extension icon, extension menu appears
3) Again clicked on extension icon, seen extension menu again which is not excepted
Observations: Tested the issue on Mac 10.12.6 and Windows-10 seen same behaviour as mentioned above(behaviour as in build without the fix)

@Robert Liao: Please find the attached screen cast for your reference and help in verifying the fix.

Thanks!
825866.ogv
3.2 MB View Download
@12: This change only applies to Mac.

Ubuntu 14.04 is not affected by the changes and is likely to have exhibited the issue in the past.
I'll also note that because of the time based nature of dismissal, remote desktop platforms are more vulnerable to the issue (hence the inconsistent repro in the past).
I'm currently not able to repro this on Chrome Canary Windows or Linux (	67.0.3388.0 (Official Build) unknown (64-bit))

Sign in to add a comment