From 998cd412ba2635c1d4b14c4eb56d83168a709e1c Mon Sep 17 00:00:00 2001 From: "shess@chromium.org" Date: Fri, 26 Mar 2010 20:28:10 +0000 Subject: [Mac] Refactor page-action context menus. Push the code to handle finding menus into the cell, which knows most about page actions. Also more thoroughly work around issues with Control-click context menus. BUG=none TEST=Right-click and Control-click work the same in the Omnibox proper. TEST=Right-click and control-click bring up appropriate menu on page actions. Review URL: http://codereview.chromium.org/1431001 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42814 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/cocoa/autocomplete_text_field.h | 5 ++ chrome/browser/cocoa/autocomplete_text_field.mm | 33 ++++++++----- .../browser/cocoa/autocomplete_text_field_cell.h | 6 +++ .../browser/cocoa/autocomplete_text_field_cell.mm | 14 ++++++ .../cocoa/autocomplete_text_field_editor.mm | 35 ++++--------- .../autocomplete_text_field_editor_unittest.mm | 56 +++++++++++++++++++++ .../cocoa/autocomplete_text_field_unittest.mm | 57 +++++++++++++++++++++- 7 files changed, 167 insertions(+), 39 deletions(-) (limited to 'chrome') diff --git a/chrome/browser/cocoa/autocomplete_text_field.h b/chrome/browser/cocoa/autocomplete_text_field.h index fa3668d..1b1856d 100644 --- a/chrome/browser/cocoa/autocomplete_text_field.h +++ b/chrome/browser/cocoa/autocomplete_text_field.h @@ -114,6 +114,11 @@ class AutocompleteTextFieldObserver { // of an I-beam. - (void)updateCursorAndToolTipRects; +// Return the appropriate menu for any page actions under event. +// Returns nil if no menu is present for the action, or if the event +// is not over an action. +- (NSMenu*)actionMenuForEvent:(NSEvent*)event; + @end #endif // CHROME_BROWSER_COCOA_AUTOCOMPLETE_TEXT_FIELD_H_ diff --git a/chrome/browser/cocoa/autocomplete_text_field.mm b/chrome/browser/cocoa/autocomplete_text_field.mm index de04957..60d3507 100644 --- a/chrome/browser/cocoa/autocomplete_text_field.mm +++ b/chrome/browser/cocoa/autocomplete_text_field.mm @@ -70,6 +70,16 @@ // a decoration area and get the expected selection behaviour, // likewise for multiple clicks in those areas. - (void)mouseDown:(NSEvent*)theEvent { + // If the click was a Control-click, bring up the context menu. + // |NSTextField| handles these cases inconsistently if the field is + // not already first responder. + if (([theEvent modifierFlags] & NSControlKeyMask) != 0) { + NSText* editor = [self currentEditor]; + NSMenu* menu = [editor menuForEvent:theEvent]; + [NSMenu popUpContextMenu:menu withEvent:theEvent forView:editor]; + return; + } + const NSPoint location = [self convertPoint:[theEvent locationInWindow] fromView:nil]; const NSRect bounds([self bounds]); @@ -114,20 +124,12 @@ return; } - // If the user clicked on one of the icons (security icon, Page Actions, etc), - // let the icon handle the click. - const BOOL ctrlKey = ([theEvent modifierFlags] & NSControlKeyMask) != 0; + // If the user clicked on one of the icons (security icon, Page + // Actions, etc), let the icon handle the click. for (AutocompleteTextFieldIcon* icon in [cell layedOutIcons:bounds]) { - if (NSMouseInRect(location, [icon rect], flipped)) { - if (ctrlKey) { - // If the click was a Ctrl+Click, then imitate a right click and open - // the contextual menu. - NSText* editor = [self currentEditor]; - NSMenu* menu = [editor menuForEvent:theEvent]; - [NSMenu popUpContextMenu:menu withEvent:theEvent forView:editor]; - } else { - [icon view]->OnMousePressed([icon rect]); - } + const NSRect iconRect = [icon rect]; + if (NSMouseInRect(location, iconRect, flipped)) { + [icon view]->OnMousePressed(iconRect); return; } } @@ -364,4 +366,9 @@ return [dropHandler_ performDragOperation:sender]; } +- (NSMenu*)actionMenuForEvent:(NSEvent*)event { + return [[self autocompleteTextFieldCell] + actionMenuForEvent:event inRect:[self bounds] ofView:self]; +} + @end diff --git a/chrome/browser/cocoa/autocomplete_text_field_cell.h b/chrome/browser/cocoa/autocomplete_text_field_cell.h index 1da806f..32f0590 100644 --- a/chrome/browser/cocoa/autocomplete_text_field_cell.h +++ b/chrome/browser/cocoa/autocomplete_text_field_cell.h @@ -94,6 +94,12 @@ class ExtensionAction; // case. - (NSRect)pageActionFrameForIndex:(size_t)index inFrame:(NSRect)cellFrame; +// Return the appropriate menu for any page actions under event. +// Returns nil if no menu is present for the action, or if the event +// is not over an action. +- (NSMenu*)actionMenuForEvent:(NSEvent*)event + inRect:(NSRect)cellFrame + ofView:(NSView*)aView; @end diff --git a/chrome/browser/cocoa/autocomplete_text_field_cell.mm b/chrome/browser/cocoa/autocomplete_text_field_cell.mm index 751d62c..5bae9867 100644 --- a/chrome/browser/cocoa/autocomplete_text_field_cell.mm +++ b/chrome/browser/cocoa/autocomplete_text_field_cell.mm @@ -455,4 +455,18 @@ CGFloat WidthForKeyword(NSAttributedString* keywordString) { return result; } +- (NSMenu*)actionMenuForEvent:(NSEvent*)event + inRect:(NSRect)cellFrame + ofView:(NSView*)aView { + NSPoint location = [aView convertPoint:[event locationInWindow] fromView:nil]; + + const BOOL flipped = [aView isFlipped]; + for (AutocompleteTextFieldIcon* icon in [self layedOutIcons:cellFrame]) { + if (NSMouseInRect(location, [icon rect], flipped)) { + return [icon view]->GetMenu(); + } + } + return nil; +} + @end diff --git a/chrome/browser/cocoa/autocomplete_text_field_editor.mm b/chrome/browser/cocoa/autocomplete_text_field_editor.mm index a4d5aec..5608e3f 100644 --- a/chrome/browser/cocoa/autocomplete_text_field_editor.mm +++ b/chrome/browser/cocoa/autocomplete_text_field_editor.mm @@ -20,11 +20,6 @@ class Extension; -@interface AutocompleteTextFieldEditor(Private) -// Returns the default context menu to be displayed on a right mouse click. -- (NSMenu*)defaultMenuForEvent:(NSEvent*)event; -@end - @implementation AutocompleteTextFieldEditor @synthesize profile = profile_; @@ -92,27 +87,17 @@ class Extension; - (void)updateRuler {} - (NSMenu*)menuForEvent:(NSEvent*)event { - NSPoint location = [self convertPoint:[event locationInWindow] fromView:nil]; - - // Was the right click within a Page Action? Show a different menu if so. - AutocompleteTextField* field = (AutocompleteTextField*)[self delegate]; - NSRect bounds([field bounds]); - AutocompleteTextFieldCell* cell = [field autocompleteTextFieldCell]; - BOOL flipped = [self isFlipped]; - - for (AutocompleteTextFieldIcon* icon in [cell layedOutIcons:bounds]) { - if (NSMouseInRect(location, [icon rect], flipped)) { - NSMenu* menu = [icon view]->GetMenu(); - if (menu) - return menu; - } - } - - // Otherwise, simply return the default menu for this instance. - return [self defaultMenuForEvent:event]; -} + // Give the control a chance to provide page-action menus. + // NOTE: Note that page actions aren't even in the editor's + // boundaries! The Cocoa control implementation seems to do a + // blanket forward to here if nothing more specific is returned from + // the control and cell calls. + // TODO(shess): Determine if the page-action part of this can be + // moved to the cell. + NSMenu* actionMenu = [[self delegate] actionMenuForEvent:event]; + if (actionMenu) + return actionMenu; -- (NSMenu*)defaultMenuForEvent:(NSEvent*)event { NSMenu* menu = [[[NSMenu alloc] initWithTitle:@"TITLE"] autorelease]; [menu addItemWithTitle:l10n_util::GetNSStringWithFixup(IDS_CUT) action:@selector(cut:) diff --git a/chrome/browser/cocoa/autocomplete_text_field_editor_unittest.mm b/chrome/browser/cocoa/autocomplete_text_field_editor_unittest.mm index 39b73a4..4d4b7a9 100644 --- a/chrome/browser/cocoa/autocomplete_text_field_editor_unittest.mm +++ b/chrome/browser/cocoa/autocomplete_text_field_editor_unittest.mm @@ -10,9 +10,11 @@ #include "chrome/app/chrome_dll_resource.h" // IDC_* #import "chrome/browser/cocoa/autocomplete_text_field_unittest_helper.h" #import "chrome/browser/cocoa/cocoa_test_helper.h" +#import "chrome/browser/cocoa/test_event_utils.h" #include "grit/generated_resources.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" +#import "third_party/ocmock/OCMock/OCMock.h" using ::testing::Return; using ::testing::StrictMock; @@ -93,6 +95,60 @@ TEST_F(AutocompleteTextFieldEditorTest, InsertStripsControlChars) { EXPECT_TRUE([[field_ stringValue] isEqualToString:@""]); } +// Test that |delegate| can provide page action menus. +TEST_F(AutocompleteTextFieldEditorTest, PageActionMenus) { + // The event just needs to be something the mock can recognize. + NSEvent* event = + test_event_utils::MouseEventAtPoint(NSZeroPoint, NSRightMouseDown, 0); + + // Trivial menu which we can recognize and which doesn't look like + // the default editor context menu. + scoped_nsobject menu([[NSMenu alloc] initWithTitle:@"Menu"]); + [menu addItemWithTitle:@"Go Fish" + action:@selector(goFish:) + keyEquivalent:@""]; + + // For OCMOCK_VALUE(). + BOOL yes = YES; + + // So that we don't have to mock the observer. + [editor_ setEditable:NO]; + + // The delegate's intercept point gets called, and results are + // propagated back. + { + id delegate = [OCMockObject mockForClass:[AutocompleteTextField class]]; + [[[delegate stub] andReturnValue:OCMOCK_VALUE(yes)] + isKindOfClass:[AutocompleteTextField class]]; + [[[delegate expect] andReturn:menu.get()] actionMenuForEvent:event]; + [editor_ setDelegate:delegate]; + NSMenu* contextMenu = [editor_ menuForEvent:event]; + [delegate verify]; + [editor_ setDelegate:nil]; + + EXPECT_EQ(contextMenu, menu.get()); + } + + // If the delegate does not return any menu, the default menu is + // returned. + { + id delegate = [OCMockObject mockForClass:[AutocompleteTextField class]]; + [[[delegate stub] andReturnValue:OCMOCK_VALUE(yes)] + isKindOfClass:[AutocompleteTextField class]]; + [[[delegate expect] andReturn:nil] actionMenuForEvent:event]; + [editor_ setDelegate:delegate]; + NSMenu* contextMenu = [editor_ menuForEvent:event]; + [delegate verify]; + [editor_ setDelegate:nil]; + + EXPECT_NE(contextMenu, menu.get()); + NSArray* items = [contextMenu itemArray]; + ASSERT_GT([items count], 0U); + EXPECT_EQ(@selector(cut:), [[items objectAtIndex:0] action]) + << "action is: " << sel_getName([[items objectAtIndex:0] action]); + } +} + // Base class for testing AutocompleteTextFieldObserver messages. class AutocompleteTextFieldEditorObserverTest : public AutocompleteTextFieldEditorTest { diff --git a/chrome/browser/cocoa/autocomplete_text_field_unittest.mm b/chrome/browser/cocoa/autocomplete_text_field_unittest.mm index fccae5c..c5ad20f 100644 --- a/chrome/browser/cocoa/autocomplete_text_field_unittest.mm +++ b/chrome/browser/cocoa/autocomplete_text_field_unittest.mm @@ -49,7 +49,13 @@ class MockPageActionImageView : public LocationBarViewMac::PageActionImageView { bool MouseWasPressed() { return mouse_was_pressed_; } - private: + void SetMenu(NSMenu* aMenu) { + menu_.reset([aMenu retain]); + } + virtual NSMenu* GetMenu() { return menu_; } + +private: + scoped_nsobject menu_; bool mouse_was_pressed_; }; @@ -664,6 +670,55 @@ TEST_F(AutocompleteTextFieldObserverTest, PageActionMouseDown) { EXPECT_TRUE(security_image_view.mouse_was_pressed_); } +// Test that page action menus are properly returned. +// TODO(shess): Really, this should test that things are forwarded to +// the cell, and the cell tests should test that the right things are +// selected. It's easier to mock the event here, though. This code's +// event-mockers might be worth promoting to |test_event_utils.h| or +// |cocoa_test_helper.h|. +TEST_F(AutocompleteTextFieldTest, PageActionMenu) { + AutocompleteTextFieldCell* cell = [field_ autocompleteTextFieldCell]; + const NSRect bounds([field_ bounds]); + + const CGFloat edge = NSHeight(bounds) - 4.0; + const NSSize size = NSMakeSize(edge, edge); + scoped_nsobject image([[NSImage alloc] initWithSize:size]); + + scoped_nsobject menu([[NSMenu alloc] initWithTitle:@"Menu"]); + + MockPageActionImageView page_action_views[2]; + page_action_views[0].SetImage(image); + page_action_views[0].SetMenu(menu); + page_action_views[0].SetVisible(true); + + page_action_views[1].SetImage(image); + page_action_views[1].SetVisible(true); + + TestPageActionViewList list; + list.Add(&page_action_views[0]); + list.Add(&page_action_views[1]); + [cell setPageActionViewList:&list]; + + // The item with a menu returns it. + NSRect actionFrame = [cell pageActionFrameForIndex:0 inFrame:bounds]; + NSPoint location = NSMakePoint(NSMidX(actionFrame), NSMidY(actionFrame)); + NSEvent* event = Event(field_, location, NSRightMouseDown, 1); + + NSMenu *actionMenu = [field_ actionMenuForEvent:event]; + EXPECT_EQ(actionMenu, menu); + + // The item without a menu returns nil. + actionFrame = [cell pageActionFrameForIndex:1 inFrame:bounds]; + location = NSMakePoint(NSMidX(actionFrame), NSMidY(actionFrame)); + event = Event(field_, location, NSRightMouseDown, 1); + EXPECT_FALSE([field_ actionMenuForEvent:event]); + + // Something not in an action returns nil. + location = NSMakePoint(NSMidX(bounds), NSMidY(bounds)); + event = Event(field_, location, NSRightMouseDown, 1); + EXPECT_FALSE([field_ actionMenuForEvent:event]); +} + // Verify that -setAttributedStringValue: works as expected when // focussed or when not focussed. Our code mostly depends on about // whether -stringValue works right. -- cgit v1.1