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

Issue 825867 link

Starred by 5 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Regression: When the Menu of an extension's Toolbar icon is visible, it no longer possible to hide it when you click outside of Chrome's window

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) Now click outside of Chrome's window (e.g. on the Desktop), so that Chrome is in the background

What is the expected result?
The extension's menu should disappear.

What happens instead?
The extension's menu still keeps visible.

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.

 

Comment 1 by meh...@chromium.org, Mar 26 2018

Summary: Regression: When the Menu of an extension's Toolbar icon is visible, it no longer possible to hide it when you click outside of Chrome's window (was: Regression: When the Menu of an extension's Toolbar icon is visible, it no longer possible to to hide it when you click outside of Chrome's window)
Cc: sindhu.chelamcherla@chromium.org
Components: Platform>Extensions
Labels: -Needs-Bisect hasbisect-per-revision ReleaseBlock-Stable Triaged-ET RegressedIn-67 M-67 Target-67 FoundIn-67 Needs-Triage-M67 OS-Windows
Owner: robliao@chromium.org
Status: Assigned (was: Untriaged)
Able to reproduce this issue on reported version 67.0.3379.0 using Mac 10.13.3 and Windows 10. Extension toolbar remains after clicking on desktop.  But issue is not reproducible on Ubuntu 14.04(Checked with 67.0.3378.0, 67.0.3365.0 and 67.0.3379.0)

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).
CHANGELOG URL:
 https://chromium.googlesource.com/chromium/src/+log/7bea94e12121bc43c45d68779803b5628f7ebbf1..a0259b3ae165b0673416f02848bed75082d639f4

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

Suspecting same from changelog.

@ robliao: Please confirm the bug and help in re-assigning if it is not related to your chnage. Adding RB-Stable as this is a recent regression. Please remove if not the case.

Thanks!
Cc: ellyjo...@chromium.org
Status: WontFix (was: Assigned)
This is the current views behavior for extension popups (Windows also demonstrates this behavior).

If this breaks Mac UI guidelines, reopen and we'll work on an exception.

Comment 4 by meh...@chromium.org, Mar 27 2018

Cc: rsesek@chromium.org sdy@chromium.org
Hello sdy@ / rsesek@: Do you know, if the current views behavior breaks Mac UI guidelines? I tend to say yes, because native menus are disappearing when you click outside the window.

Thanks in advance :-)

Comment 5 by rsesek@chromium.org, Mar 27 2018

I do think bubbles should dismiss when the window loses active status. That's what the Cocoa bubbles do.

Comment 6 by meh...@chromium.org, Mar 27 2018

Status: Assigned (was: WontFix)
Thanks rsesek@. So then changing the status of this report back to "Assigned".
Status: Started (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 29 2018

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

commit bbfef63d5b1f38305c238f13373a1b96a1313bde
Author: Robert Liao <robliao@chromium.org>
Date: Thu Mar 29 18:36:36 2018

Rename ExtensionPopup::OnAnchorWindowActivation() to ExtensionPopup::CloseUnlessUnderInspection()

This name removes a policy implication from what the function does.

BUG=825867

Change-Id: Ida740958cf416bc917ec27e338f3eaaad180a5e1
Reviewed-on: https://chromium-review.googlesource.com/985637
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546873}
[modify] https://crrev.com/bbfef63d5b1f38305c238f13373a1b96a1313bde/chrome/browser/ui/cocoa/extensions/extension_popup_views_mac.mm
[modify] https://crrev.com/bbfef63d5b1f38305c238f13373a1b96a1313bde/chrome/browser/ui/views/extensions/extension_popup.cc
[modify] https://crrev.com/bbfef63d5b1f38305c238f13373a1b96a1313bde/chrome/browser/ui/views/extensions/extension_popup.h
[modify] https://crrev.com/bbfef63d5b1f38305c238f13373a1b96a1313bde/chrome/browser/ui/views/extensions/extension_popup_aura.cc

Labels: Needs-Feedback
Able to reproduce this issue on reported version hence verifying the fix on latest canary 67.0.3384.0.

Still observing extension dialog after clicking outside on desktop. Attaching screencast for reference.

@robalio: Please check the video and let us know if we missed something. Please help in verifying the fix.

Thanks!
@9: The bug hasn't yet been marked as fixed.
Labels: -Needs-Feedback
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 31 2018

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

commit 902d73ba5f92c7f10ea8c07e5839a7143b28d107
Author: Robert Liao <robliao@chromium.org>
Date: Sat Mar 31 22:53:19 2018

Adjust ExtensionPopup Dismissal Behavior

Before, dismissing the ExtensionPopup when the anchor window received
focus was an arbitrary decision ( http://crbug.com/179786 ) that allowed
the ExtensionPopup to dismiss at most of the right times. However, if
the some other top-level window received activation, the ExtensionPopup
would not dismiss, unlike a typical menu.

This change adjusts the ExtensionPopup to always dismiss when it loses
activation as long as devtools is not attached.

When devtools is detached, activation is placed back on the
ExtensionPopup so that the normal dismissal behavior can continue to
work. Failure to receive activation means the ExtensionPopup will not
dismiss until it receives activation at least once.

BUG=825867

Change-Id: I802af281616c66013c370e892953ad2805533728
Reviewed-on: https://chromium-review.googlesource.com/984404
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547391}
[modify] https://crrev.com/902d73ba5f92c7f10ea8c07e5839a7143b28d107/chrome/browser/ui/cocoa/extensions/extension_popup_views_mac.mm
[modify] https://crrev.com/902d73ba5f92c7f10ea8c07e5839a7143b28d107/chrome/browser/ui/views/extensions/extension_popup.cc

Status: Fixed (was: Started)
Labels: TE-Verified-M67 TE-Verified-67.0.3386.0
Able to reproduce this issue on reported version hence verifying the fix on latest canary 67.0.3386.0.

Observing dismissal of extension popup on clicking on desktop. Attaching screencast for reference.
As fix is working as expected adding Verified labels.

Thanks!
825867_67.0.3386.0.mp4
831 KB View Download
Status: Verified (was: Fixed)
Labels: -ReleaseBlock-Stable
Status: Assigned (was: Verified)
We've decided we will live with this for M67 as detecting child activation of arbitrary native widgets doesn't seem to be possible at the moment and will be too big of a change for M67.
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 17 2018

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

commit f58eccdd24c115ab1f70e310c3823880cdad04a8
Author: Robert Liao <robliao@chromium.org>
Date: Tue Apr 17 20:03:48 2018

Revert "Adjust ExtensionPopup Dismissal Behavior"

This reverts commit 902d73ba5f92c7f10ea8c07e5839a7143b28d107.

Reason for revert: This causes the ExtensionPopup to dismiss if it brings up a child dialog. See  http://crbug.com/832623 
Fixing this will require adding support to check for descendant NativeWidget activation, which is too big to merge.

Original change's description:
> Adjust ExtensionPopup Dismissal Behavior
>
> Before, dismissing the ExtensionPopup when the anchor window received
> focus was an arbitrary decision ( http://crbug.com/179786 ) that allowed
> the ExtensionPopup to dismiss at most of the right times. However, if
> the some other top-level window received activation, the ExtensionPopup
> would not dismiss, unlike a typical menu.
>
> This change adjusts the ExtensionPopup to always dismiss when it loses
> activation as long as devtools is not attached.
>
> When devtools is detached, activation is placed back on the
> ExtensionPopup so that the normal dismissal behavior can continue to
> work. Failure to receive activation means the ExtensionPopup will not
> dismiss until it receives activation at least once.
>
> BUG=825867
>
> Change-Id: I802af281616c66013c370e892953ad2805533728
> Reviewed-on: https://chromium-review.googlesource.com/984404
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Commit-Queue: Robert Liao <robliao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#547391}

TBR=ellyjones@chromium.org,sky@chromium.org,robliao@chromium.org,rdevlin.cronin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 825867,  832623 
Change-Id: I69678b789fdbc56501e67b62dd41467e5f2f8cc9
Reviewed-on: https://chromium-review.googlesource.com/1015181
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551444}
[modify] https://crrev.com/f58eccdd24c115ab1f70e310c3823880cdad04a8/chrome/browser/ui/cocoa/extensions/extension_popup_views_mac.mm
[modify] https://crrev.com/f58eccdd24c115ab1f70e310c3823880cdad04a8/chrome/browser/ui/views/extensions/extension_popup.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/51623832b41997874faf9ee68e9f0430f58e69e9

commit 51623832b41997874faf9ee68e9f0430f58e69e9
Author: Robert Liao <robliao@chromium.org>
Date: Tue Apr 17 20:07:48 2018

Revert "Adjust ExtensionPopup Dismissal Behavior"

This reverts commit 902d73ba5f92c7f10ea8c07e5839a7143b28d107.

Reason for revert: This causes the ExtensionPopup to dismiss if it brings up a child dialog. See  http://crbug.com/832623 
Fixing this will require adding support to check for descendant NativeWidget activation, which is too big to merge.

Original change's description:
> Adjust ExtensionPopup Dismissal Behavior
>
> Before, dismissing the ExtensionPopup when the anchor window received
> focus was an arbitrary decision ( http://crbug.com/179786 ) that allowed
> the ExtensionPopup to dismiss at most of the right times. However, if
> the some other top-level window received activation, the ExtensionPopup
> would not dismiss, unlike a typical menu.
>
> This change adjusts the ExtensionPopup to always dismiss when it loses
> activation as long as devtools is not attached.
>
> When devtools is detached, activation is placed back on the
> ExtensionPopup so that the normal dismissal behavior can continue to
> work. Failure to receive activation means the ExtensionPopup will not
> dismiss until it receives activation at least once.
>
> BUG=825867
>
> Change-Id: I802af281616c66013c370e892953ad2805533728
> Reviewed-on: https://chromium-review.googlesource.com/984404
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Commit-Queue: Robert Liao <robliao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#547391}

TBR=ellyjones@chromium.org,sky@chromium.org,robliao@chromium.org,rdevlin.cronin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  832623 
Change-Id: I69678b789fdbc56501e67b62dd41467e5f2f8cc9
Reviewed-on: https://chromium-review.googlesource.com/1015181
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#551444}(cherry picked from commit f58eccdd24c115ab1f70e310c3823880cdad04a8)
Reviewed-on: https://chromium-review.googlesource.com/1015723
Cr-Commit-Position: refs/branch-heads/3396@{#58}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/51623832b41997874faf9ee68e9f0430f58e69e9/chrome/browser/ui/cocoa/extensions/extension_popup_views_mac.mm
[modify] https://crrev.com/51623832b41997874faf9ee68e9f0430f58e69e9/chrome/browser/ui/views/extensions/extension_popup.cc

Labels: Needs-Feedback
Able to reproduce this issue on reported version hence verifying the fix on latest dev 67.0.3396.10 and canary 68.0.3399.0 using Mac 10.13.3 and Windows 10.

Still observing extension dialog after clicking outside on desktop. Attaching screencast for reference.

@robalio: Please help in verifying the fix on latest dev and canary builds.

Thanks!
825867.mp4
1.1 MB View Download

Comment 21 by ajha@chromium.org, Apr 18 2018

Labels: -Needs-Feedback
Looks like the fixing CL is reverted that's why this is still reproducible.
Cc: robliao@chromium.org nyerramilli@chromium.org rbasuvula@chromium.org
 Issue 837588  has been merged into this issue.
 Issue 844330  has been merged into this issue.
Labels: -Pri-1 Pri-2
Adjusting priority as Win+Linux have had this behavior for many years now.
Labels: Proj-MacViews
Labels: -Target-67 Target-69
Labels: MacViews-Release
Labels: -MacViews-Release
Labels: -M-67 Group-Views_Regressions_from_Cocoa
Labels: M-67
Labels: -M-67 M-69
Labels: -M-69 -Target-69 M-70 Target-70

Sign in to add a comment