Skip to content

Commit

Permalink
[Extensions Toolbar Mac] Make context menus work for overflowed actions
Browse files Browse the repository at this point in the history
On Mac, it doesn't look like nested menus (meaning a completely separate menu
running in an original menu, not to be mistaken with submenus) are supported.
This is very unfortunate. For overflowed toolbar actions, close the wrench menu,
then pop up the context menu over the wrench button (this is similar to what we
do for overflowed extension popups).

BUG=429810

Review URL: https://codereview.chromium.org/821953004

Cr-Commit-Position: refs/heads/master@{#312013}
  • Loading branch information
rdcronin authored and Commit bot committed Jan 17, 2015
1 parent 8a5f525 commit a77c966
Show file tree
Hide file tree
Showing 9 changed files with 344 additions and 15 deletions.
10 changes: 10 additions & 0 deletions chrome/browser/ui/cocoa/extensions/browser_action_button.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,13 @@ extern NSString* const kBrowserActionButtonDragEndNotification;
// The bridge between the view controller and this object.
scoped_ptr<ToolbarActionViewDelegateBridge> viewControllerDelegate_;

// The context menu controller.
base::scoped_nsobject<MenuController> contextMenuController_;

// A substitute context menu to use in testing. We need this because normally
// menu code is blocking, making it difficult to test.
NSMenu* testContextMenu_;

// The controller for the browser actions bar that owns this button. Weak.
BrowserActionsController* browserActionsController_;

Expand Down Expand Up @@ -82,6 +87,11 @@ extern NSString* const kBrowserActionButtonDragEndNotification;

@end

@interface BrowserActionButton(TestingAPI)
// Sets a context menu to use for testing purposes.
- (void)setTestContextMenu:(NSMenu*)testContextMenu;
@end

@interface BrowserActionCell : ImageButtonCell {
@private
// The controller for the browser actions bar that owns the button. Weak.
Expand Down
85 changes: 83 additions & 2 deletions chrome/browser/ui/cocoa/extensions/browser_action_button.mm
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,18 @@
#include <cmath>

#include "base/logging.h"
#include "base/memory/weak_ptr.h"
#include "base/strings/sys_string_conversions.h"
#include "chrome/browser/extensions/extension_context_menu_model.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_window.h"
#import "chrome/browser/ui/cocoa/browser_window_controller.h"
#import "chrome/browser/ui/cocoa/extensions/browser_actions_controller.h"
#import "chrome/browser/ui/cocoa/themed_window.h"
#import "chrome/browser/ui/cocoa/toolbar/toolbar_action_view_delegate_cocoa.h"
#import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h"
#import "chrome/browser/ui/cocoa/wrench_menu/wrench_menu_controller.h"
#include "chrome/browser/ui/extensions/extension_action_view_controller.h"
#include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
#include "grit/theme_resources.h"
Expand Down Expand Up @@ -45,13 +49,19 @@
ToolbarActionViewController* viewController);
~ToolbarActionViewDelegateBridge() override;

// Shows the context menu for the owning action.
void ShowContextMenu();

private:
// ToolbarActionViewDelegateCocoa:
ToolbarActionViewController* GetPreferredPopupViewController() override;
content::WebContents* GetCurrentWebContents() const override;
void UpdateState() override;
NSPoint GetPopupPoint() override;

// A helper method to implement showing the context menu.
void DoShowContextMenu();

// The owning button. Weak.
BrowserActionButton* owner_;

Expand All @@ -61,6 +71,8 @@
// The ToolbarActionViewController for which this is the delegate. Weak.
ToolbarActionViewController* viewController_;

base::WeakPtrFactory<ToolbarActionViewDelegateBridge> weakFactory_;

DISALLOW_COPY_AND_ASSIGN(ToolbarActionViewDelegateBridge);
};

Expand All @@ -70,17 +82,41 @@
ToolbarActionViewController* viewController)
: owner_(owner),
controller_(controller),
viewController_(viewController) {
viewController_(viewController),
weakFactory_(this) {
viewController_->SetDelegate(this);
}

ToolbarActionViewDelegateBridge::~ToolbarActionViewDelegateBridge() {
viewController_->SetDelegate(nullptr);
}

void ToolbarActionViewDelegateBridge::ShowContextMenu() {
// We should only be showing the context menu in this way if we're doing so
// for an overflowed action.
DCHECK(![owner_ superview]);

WrenchMenuController* wrenchMenuController =
[[[BrowserWindowController browserWindowControllerForWindow:
[controller_ browser]->window()->GetNativeWindow()]
toolbarController] wrenchMenuController];
// If the wrench menu is open, we have to first close it. Part of this happens
// asynchronously, so we have to use a posted task to open the next menu.
if ([wrenchMenuController isMenuOpen]) {
[wrenchMenuController cancel];
base::MessageLoop::current()->PostTask(
FROM_HERE,
base::Bind(&ToolbarActionViewDelegateBridge::DoShowContextMenu,
weakFactory_.GetWeakPtr()));
} else {
DoShowContextMenu();
}
}

ToolbarActionViewController*
ToolbarActionViewDelegateBridge::GetPreferredPopupViewController() {
return viewController_;
return [[controller_ mainButtonForId:viewController_->GetId()]
viewController];
}

content::WebContents* ToolbarActionViewDelegateBridge::GetCurrentWebContents()
Expand All @@ -96,6 +132,27 @@
return [controller_ popupPointForId:[owner_ viewController]->GetId()];
}

void ToolbarActionViewDelegateBridge::DoShowContextMenu() {
NSButton* wrenchButton =
[[[BrowserWindowController browserWindowControllerForWindow:
[controller_ browser]->window()->GetNativeWindow()]
toolbarController] wrenchButton];
// The point the menu shows matches that of the normal wrench menu - that is,
// the right-left most corner of the menu is left-aligned with the wrench
// button, and the menu is displayed "a little bit" lower. It would be nice to
// be able to avoid the magic '5' here, but since it's built into Cocoa, it's
// not too hopeful.
NSPoint menuPoint = NSMakePoint(0, NSHeight([wrenchButton bounds]) + 5);

// We set the wrench to be highlighted so it remains "pressed" when the menu
// is running.
[[wrenchButton cell] setHighlighted:YES];
[[owner_ menu] popUpMenuPositioningItem:nil
atLocation:menuPoint
inView:wrenchButton];
[[wrenchButton cell] setHighlighted:NO];
}

@interface BrowserActionCell (Internals)
- (void)drawBadgeWithinFrame:(NSRect)frame
forWebContents:(content::WebContents*)webContents;
Expand Down Expand Up @@ -164,6 +221,20 @@ - (BOOL)acceptsFirstResponder {
return YES;
}

- (void)rightMouseDown:(NSEvent*)theEvent {
// Cocoa doesn't allow menus-running-in-menus, so in order to show the
// context menu for an overflowed action, we close the wrench menu and show
// the context menu over the wrench (similar to what we do for popups).
// Let the main bar's button handle showing the context menu, since the wrench
// menu will close..
if ([browserActionsController_ isOverflow]) {
[browserActionsController_ mainButtonForId:viewController_->GetId()]->
viewControllerDelegate_->ShowContextMenu();
} else {
[super rightMouseDown:theEvent];
}
}

- (void)mouseDown:(NSEvent*)theEvent {
NSPoint location = [self convertPoint:[theEvent locationInWindow]
fromView:nil];
Expand Down Expand Up @@ -353,6 +424,9 @@ - (NSImage*)compositedImage {
}

- (NSMenu*)menu {
if (testContextMenu_)
return testContextMenu_;

ui::MenuModel* contextMenu = viewController_->GetContextMenu();
if (!contextMenu)
return nil;
Expand All @@ -362,6 +436,13 @@ - (NSMenu*)menu {
return [contextMenuController_ menu];
}

#pragma mark -
#pragma mark Testing Methods

- (void)setTestContextMenu:(NSMenu*)testContextMenu {
testContextMenu_ = testContextMenu;
}

@end

@implementation BrowserActionCell
Expand Down
Loading

0 comments on commit a77c966

Please sign in to comment.