Skip to content

Commit f58eccd

Browse files
Robert LiaoCommit Bot
authored andcommitted
Revert "Adjust ExtensionPopup Dismissal Behavior"
This reverts commit 902d73b. 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 <[email protected]> > Reviewed-by: Elly Fong-Jones <[email protected]> > Commit-Queue: Robert Liao <[email protected]> > Cr-Commit-Position: refs/heads/master@{#547391} [email protected],[email protected],[email protected],[email protected] # 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 <[email protected]> Reviewed-by: Robert Liao <[email protected]> Cr-Commit-Position: refs/heads/master@{#551444}
1 parent 2e1d92f commit f58eccd

File tree

2 files changed

+3
-11
lines changed

2 files changed

+3
-11
lines changed

chrome/browser/ui/cocoa/extensions/extension_popup_views_mac.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@
3434

3535
NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
3636
// ObjC id collides with views::View::id().
37-
::id token = [center addObserverForName:NSWindowDidResignKeyNotification
38-
object:popup->GetWidget()->GetNativeWindow()
37+
::id token = [center addObserverForName:NSWindowDidBecomeKeyNotification
38+
object:parent_window
3939
queue:nil
4040
usingBlock:^(NSNotification* notification) {
4141
popup->CloseUnlessUnderInspection();

chrome/browser/ui/views/extensions/extension_popup.cc

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -123,14 +123,6 @@ void ExtensionPopup::DevToolsAgentHostDetached(
123123
if (host()->host_contents() != agent_host->GetWebContents())
124124
return;
125125
inspect_with_devtools_ = false;
126-
// Reset the dismissing logic back to a known state.
127-
// The Chrome top-level window often receives activation when devtools is
128-
// closed. However, because devtools was shown, ExtensionPopup doesn't have
129-
// activation. This means there is no chance for it to dismiss until it
130-
// receives activation again. Checking the anchor window for activation and
131-
// then dismissing would result in the ExtensionPopup dismissing with
132-
// devtools, which is also unexpected.
133-
GetWidget()->Activate();
134126
}
135127

136128
ExtensionViewViews* ExtensionPopup::GetExtensionView() {
@@ -160,7 +152,7 @@ void ExtensionPopup::AddedToWidget() {
160152

161153
void ExtensionPopup::OnWidgetActivationChanged(views::Widget* widget,
162154
bool active) {
163-
if (!active && widget == GetWidget())
155+
if (active && widget == anchor_widget())
164156
CloseUnlessUnderInspection();
165157
}
166158

0 commit comments

Comments
 (0)