diff options
Diffstat (limited to 'chrome/browser')
18 files changed, 424 insertions, 111 deletions
diff --git a/chrome/browser/cocoa/bookmark_bar_bridge.h b/chrome/browser/cocoa/bookmark_bar_bridge.h index 79805953..a6661cc 100644 --- a/chrome/browser/cocoa/bookmark_bar_bridge.h +++ b/chrome/browser/cocoa/bookmark_bar_bridge.h @@ -33,6 +33,9 @@ class BookmarkBarBridge : public BookmarkModelObserver { virtual void BookmarkNodeAdded(BookmarkModel* model, const BookmarkNode* parent, int index); + virtual void BookmarkNodeRemoved(BookmarkModel* model, + const BookmarkNode* parent, + int index); virtual void BookmarkNodeChanged(BookmarkModel* model, const BookmarkNode* node); virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model, diff --git a/chrome/browser/cocoa/bookmark_bar_bridge.mm b/chrome/browser/cocoa/bookmark_bar_bridge.mm index 2263a3c..4e13218 100644 --- a/chrome/browser/cocoa/bookmark_bar_bridge.mm +++ b/chrome/browser/cocoa/bookmark_bar_bridge.mm @@ -44,6 +44,12 @@ void BookmarkBarBridge::BookmarkNodeAdded(BookmarkModel* model, [controller_ nodeAdded:model parent:parent index:index]; } +void BookmarkBarBridge::BookmarkNodeRemoved(BookmarkModel* model, + const BookmarkNode* parent, + int index) { + [controller_ nodeRemoved:model parent:parent index:index]; +} + void BookmarkBarBridge::BookmarkNodeChanged(BookmarkModel* model, const BookmarkNode* node) { [controller_ nodeChanged:model node:node]; diff --git a/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm b/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm index 687c64a..0a21e04 100644 --- a/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm @@ -32,10 +32,12 @@ typedef std::pair<GURL,WindowOpenDisposition> OpenInfo; @implementation FakeBookmarkBarController -- (id)initWithProfile:(Profile*)profile view:(NSView*)view { +- (id)initWithProfile:(Profile*)profile + parentView:(NSView*)parentView + webContentView:(NSView*)webContentView { if ((self = [super initWithProfile:profile - view:(BookmarkBarView*)view - webContentView:nil + parentView:parentView + webContentView:webContentView delegate:self])) { callbacks_.reset([[NSMutableArray alloc] init]); } @@ -76,6 +78,11 @@ typedef std::pair<GURL,WindowOpenDisposition> OpenInfo; [callbacks_ addObject:[NSNumber numberWithInt:6]]; } +- (void)nodeRemoved:(BookmarkModel*)model + parent:(const BookmarkNode*)oldParent index:(int)index { + [callbacks_ addObject:[NSNumber numberWithInt:7]]; +} + // Save the request. - (void)openBookmarkURL:(const GURL&)url disposition:(WindowOpenDisposition)disposition { @@ -102,12 +109,16 @@ TEST_F(BookmarkBarBridgeTest, TestRedirect) { Profile *profile = browser_test_helper_.profile(); BookmarkModel *model = profile->GetBookmarkModel(); - scoped_nsobject<NSView> view([[NSView alloc] - initWithFrame:NSMakeRect(0,0,10,10)]); - [view.get() setHidden:YES]; + scoped_nsobject<NSView> parentView([[NSView alloc] + initWithFrame:NSMakeRect(0,0,100,100)]); + scoped_nsobject<NSView> webView([[NSView alloc] + initWithFrame:NSMakeRect(0,0,100,100)]); + scoped_nsobject<FakeBookmarkBarController> - controller([[FakeBookmarkBarController alloc] initWithProfile:profile - view:view.get()]); + controller([[FakeBookmarkBarController alloc] + initWithProfile:profile + parentView:parentView.get() + webContentView:webView.get()]); EXPECT_TRUE(controller.get()); scoped_ptr<BookmarkBarBridge> bridge(new BookmarkBarBridge(controller.get(), model)); @@ -120,11 +131,12 @@ TEST_F(BookmarkBarBridgeTest, TestRedirect) { bridge->BookmarkNodeChanged(NULL, NULL); bridge->BookmarkNodeFavIconLoaded(NULL, NULL); bridge->BookmarkNodeChildrenReordered(NULL, NULL); + bridge->BookmarkNodeRemoved(NULL, NULL, 0); - // 7 calls above plus an initial Loaded() in init routine makes 8 - EXPECT_TRUE([controller.get()->callbacks_ count] == 8); + // 8 calls above plus an initial Loaded() in init routine makes 9 + EXPECT_TRUE([controller.get()->callbacks_ count] == 9); - for (int x = 1; x < 8; x++) { + for (int x = 1; x < 9; x++) { NSNumber *num = [NSNumber numberWithInt:x-1]; EXPECT_TRUE([[controller.get()->callbacks_ objectAtIndex:x] isEqual:num]); } diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h index 1993411..5d401de 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller.h +++ b/chrome/browser/cocoa/bookmark_bar_controller.h @@ -27,7 +27,7 @@ class PrefService; // A controller for the bookmark bar in the browser window. Handles showing // and hiding based on the preference in the given profile. -@interface BookmarkBarController : NSObject { +@interface BookmarkBarController : NSViewController { @private BookmarkModel* bookmarkModel_; // weak; part of the profile owned by the // top-level Browser object. @@ -44,10 +44,7 @@ class PrefService; // Set when using fullscreen mode. BOOL barIsEnabled_; - // The view of the bookmark bar itself. - // Owned by the toolbar view, its parent view. - BookmarkBarView* bookmarkBarView_; // weak - + NSView* parentView_; // weak; our parent view NSView* webContentView_; // weak; where the web goes // Bridge from Chrome-style C++ notifications (e.g. derived from @@ -56,14 +53,15 @@ class PrefService; // Delegate which can open URLs for us. id<BookmarkURLOpener> delegate_; // weak + + IBOutlet NSMenu* buttonContextMenu_; } -// Initializes the controller with the given browser profile and -// content view. We use |webContentView| as the view for the bookmark -// bar view and for geometry management. |delegate| is used for -// opening URLs. |view| is expected to be hidden. +// Initializes the bookmark bar controller with the given browser +// profile, parent view (the toolbar), web content view, and delegate. +// |delegate| is used for opening URLs. - (id)initWithProfile:(Profile*)profile - view:(BookmarkBarView*)view + parentView:(NSView*)parentView webContentView:(NSView*)webContentView delegate:(id<BookmarkURLOpener>)delegate; @@ -78,6 +76,14 @@ class PrefService; // if needed. For fullscreen mode. - (void)setBookmarkBarEnabled:(BOOL)enabled; +// Actions for opening bookmarks. From a button, ... +- (IBAction)openBookmark:(id)sender; +// ... or from a context menu over the button. +- (IBAction)openBookmarkInNewForegroundTab:(id)sender; +- (IBAction)openBookmarkInNewWindow:(id)sender; +- (IBAction)openBookmarkInIncognitoWindow:(id)sender; +- (IBAction)deleteBookmark:(id)sender; + @end // Redirects from BookmarkBarBridge, the C++ object which glues us to @@ -90,6 +96,8 @@ class PrefService; newParent:(const BookmarkNode*)newParent newIndex:(int)newIndex; - (void)nodeAdded:(BookmarkModel*)model parent:(const BookmarkNode*)oldParent index:(int)index; +- (void)nodeRemoved:(BookmarkModel*)model + parent:(const BookmarkNode*)oldParent index:(int)index; - (void)nodeChanged:(BookmarkModel*)model node:(const BookmarkNode*)node; - (void)nodeFavIconLoaded:(BookmarkModel*)model @@ -101,12 +109,8 @@ class PrefService; // These APIs should only be used by unit tests (or used internally). @interface BookmarkBarController(TestingAPI) -// Access to the bookmark bar's view represented by this controller. -- (NSView*)view; // Set the delegate for a unit test. - (void)setDelegate:(id<BookmarkURLOpener>)delegate; -// Action for our bookmark buttons. -- (void)openBookmark:(id)sender; @end #endif // CHROME_BROWSER_COCOA_BOOKMARK_BAR_CONTROLLER_H_ diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm index 7550c99..4cf5d07 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller.mm +++ b/chrome/browser/cocoa/bookmark_bar_controller.mm @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/mac_util.h" #include "base/sys_string_conversions.h" #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/browser.h" @@ -22,6 +23,9 @@ namespace { +// TODO(jrg): this is the right proportional height but overlaps the +// "blue outline" of the omnibox. Fix. + // Our height, when opened. const int kBookmarkBarHeight = 30; // How much to adjust our parent view. @@ -38,35 +42,41 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; @implementation BookmarkBarController - (id)initWithProfile:(Profile*)profile - view:(BookmarkBarView*)view + parentView:(NSView*)parentView webContentView:(NSView*)webContentView delegate:(id<BookmarkURLOpener>)delegate { - if ((self = [super init])) { + if ((self = [super initWithNibName:@"BookmarkBar" + bundle:mac_util::MainAppBundle()])) { bookmarkModel_ = profile->GetBookmarkModel(); - bookmarkBarView_ = view; - // We default to NOT open, which means height=0. - DCHECK([view isHidden]); // OK to change - NSRect frame = [view frame]; - frame.size.height = 0; - [view setFrame:frame]; - - // Make sure the nodes stay bottom-aligned. - [bookmarkBarView_ setAutoresizingMask:(NSViewWidthSizable | - NSViewMinYMargin)]; + preferences_ = profile->GetPrefs(); + parentView_ = parentView; webContentView_ = webContentView; delegate_ = delegate; - // Be sure to enable the bar before trying to show it... - barIsEnabled_ = YES; - preferences_ = profile->GetPrefs(); - if (preferences_->GetBoolean(prefs::kShowBookmarkBar)) - [self showBookmarkBar:YES immediately:YES]; } - // Don't pass ourself along until our init is done. - // Thus, this call is (almost) last. - bridge_.reset(new BookmarkBarBridge(self, bookmarkModel_)); return self; } +- (void)awakeFromNib { + // We default to NOT open, which means height=0. + DCHECK([[self view] isHidden]); // Hidden so it's OK to change. + NSRect frame = [[self view] frame]; + frame.size.height = 0; + frame.size.width = [parentView_ frame].size.width; + [[self view] setFrame:frame]; + + // Make sure the nodes stay bottom-aligned. + [[self view] setAutoresizingMask:(NSViewWidthSizable | + NSViewMinYMargin)]; + // Be sure to enable the bar before trying to show it... + barIsEnabled_ = YES; + if (preferences_->GetBoolean(prefs::kShowBookmarkBar)) + [self showBookmarkBar:YES immediately:YES]; + + // Don't pass ourself along (as 'self') until our init is completely + // done. Thus, this call is (almost) last. + bridge_.reset(new BookmarkBarBridge(self, bookmarkModel_)); +} + // Show or hide the bar based on the value of |show|. Handles // animating the resize of the content view. if |immediately| is YES, // make changes immediately instead of using an animator. If the bar @@ -76,7 +86,7 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; - (void)showBookmarkBar:(BOOL)show immediately:(BOOL)immediately { if (barIsEnabled_ && (barShouldBeShown_ != show)) { contentViewHasOffset_ = show; - [bookmarkBarView_ setHidden:show ? NO : YES]; + [[self view] setHidden:show ? NO : YES]; barShouldBeShown_ = show; if (show) { [self loaded:bookmarkModel_]; @@ -97,7 +107,7 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; // running. Thus, if you resize the window while the bookmark bar is // animating, you'll mess things up. Fix. - (void)applyContentAreaOffset:(BOOL)apply immediately:(BOOL)immediately { - if (bookmarkBarView_ == nil) { + if ([self view] == nil) { // We're too early, but awakeFromNib will call me again. return; } @@ -107,9 +117,8 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; } // None of these locals are members of the Hall of Justice. - NSView* superview = [bookmarkBarView_ superview]; - NSRect superframe = [superview frame]; - NSRect frame = [bookmarkBarView_ frame]; + NSRect superframe = [parentView_ frame]; + NSRect frame = [[self view] frame]; NSRect webframe = [webContentView_ frame]; if (apply) { superframe.size.height += kBookmarkBarSuperviewHeightAdjustment; @@ -128,17 +137,17 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; // expected. Fix, or clean out the animators as an option. // Odd racing is FAR worse than a lack of an animator. if (1 /* immediately */) { - [superview setFrame:superframe]; + [parentView_ setFrame:superframe]; [webContentView_ setFrame:webframe]; - [bookmarkBarView_ setFrame:frame]; + [[self view] setFrame:frame]; } else { - [[superview animator] setFrame:superframe]; + [[parentView_ animator] setFrame:superframe]; [[webContentView_ animator] setFrame:webframe]; - [[bookmarkBarView_ animator] setFrame:frame]; + [[[self view] animator] setFrame:frame]; } - [bookmarkBarView_ setNeedsDisplay:YES]; - [superview setNeedsDisplay:YES]; + [[self view] setNeedsDisplay:YES]; + [parentView_ setNeedsDisplay:YES]; [webContentView_ setNeedsDisplay:YES]; } @@ -168,20 +177,59 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; } } -// Delete all items from the bookmark bar. -- (void)clearBookmarkBar { - [bookmarkBarView_ setSubviews:[NSArray array]]; +- (BookmarkNode*)nodeFromMenuItem:(id)menuItem { + NSCell* cell = [[menuItem menu] delegate]; + BookmarkNode* node = static_cast<BookmarkNode*>( + [[cell representedObject] pointerValue]); + DCHECK(node); + return node; } -// TODO(jrg): add an openBookmarkInBackground() for ctrl-click which -// has a different disposition. -- (void)openBookmark:(id)sender { - const BookmarkNode* node = static_cast<const BookmarkNode*>( - [[[sender cell] representedObject] pointerValue]); +- (BookmarkNode*)nodeFromButton:(id)button { + NSCell* cell = [button cell]; + BookmarkNode* node = static_cast<BookmarkNode*>( + [[cell representedObject] pointerValue]); DCHECK(node); + return node; +} + +- (IBAction)openBookmark:(id)sender { + BookmarkNode* node = [self nodeFromButton:sender]; [delegate_ openBookmarkURL:node->GetURL() disposition:CURRENT_TAB]; } +// As a convention we set the menu's delegate to be the button's cell +// so we can easily obtain bookmark info. Convention applied in +// -[BookmarkButtonCell menu]. + +- (IBAction)openBookmarkInNewForegroundTab:(id)sender { + BookmarkNode* node = [self nodeFromMenuItem:sender]; + [delegate_ openBookmarkURL:node->GetURL() disposition:NEW_FOREGROUND_TAB]; +} + +- (IBAction)openBookmarkInNewWindow:(id)sender { + BookmarkNode* node = [self nodeFromMenuItem:sender]; + [delegate_ openBookmarkURL:node->GetURL() disposition:NEW_WINDOW]; +} + +- (IBAction)openBookmarkInIncognitoWindow:(id)sender { + BookmarkNode* node = [self nodeFromMenuItem:sender]; + [delegate_ openBookmarkURL:node->GetURL() disposition:OFF_THE_RECORD]; +} + +- (IBAction)deleteBookmark:(id)sender { + BookmarkNode* node = [self nodeFromMenuItem:sender]; + bookmarkModel_->Remove(node->GetParent(), + node->GetParent()->IndexOfChild(node)); +} + +// Delete all items from the bookmark bar. TODO(jrg): once the +// bookmark bar has other subviews (e.g. "off the side" button/menu, +// "Other Bookmarks"), etc, this routine will need revisiting. +- (void)clearBookmarkBar { + [[self view] setSubviews:[NSArray array]]; +} + // Return an autoreleased NSCell suitable for a bookmark button. // TODO(jrg): move much of the cell config into the BookmarkButtonCell class. - (NSCell *)cellForBookmarkNode:(const BookmarkNode*)node frame:(NSRect)frame { @@ -190,9 +238,6 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; autorelease]; DCHECK(cell); [cell setRepresentedObject:[NSValue valueWithPointer:node]]; - [cell setButtonType:NSMomentaryPushInButton]; - [cell setBezelStyle:NSShadowlessSquareBezelStyle]; - [cell setShowsBorderOnlyWhileMouseInside:YES]; // The favicon may be NULL if we haven't loaded it yet. Bookmarks // (and their icons) are loaded on the IO thread to speed launch. @@ -204,13 +249,8 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; [cell setImagePosition:NSImageLeft]; } } - [cell setTitle:title]; - [cell setControlSize:NSSmallControlSize]; - [cell setAlignment:NSLeftTextAlignment]; - [cell setFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]]; - [cell setWraps:NO]; - [cell setLineBreakMode:NSLineBreakByTruncatingMiddle]; + [cell setMenu:buttonContextMenu_]; return cell; } @@ -257,7 +297,6 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; // object. To honor the assumed semantics, we do nothing with // NSButton between alloc/init and setCell:. [button setCell:[self cellForBookmarkNode:child frame:frame]]; - // [button sizeToFit]; if (child->is_folder()) { // For now just disable the button if it's a folder. @@ -275,7 +314,7 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; [button setToolTip:tooltip]; } // Finally, add it to the bookmark bar. - [bookmarkBarView_ addSubview:button]; + [[self view] addSubview:button]; } } @@ -309,6 +348,12 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; } // TODO(jrg): for now this is brute force. +- (void)nodeRemoved:(BookmarkModel*)model + parent:(const BookmarkNode*)oldParent index:(int)index { + [self loaded:model]; +} + +// TODO(jrg): for now this is brute force. - (void)nodeChanged:(BookmarkModel*)model node:(const BookmarkNode*)node { [self loaded:model]; @@ -318,7 +363,7 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; // Need a BookmarkNode-->NSCell mapping. - (void)nodeFavIconLoaded:(BookmarkModel*)model node:(const BookmarkNode*)node { - NSArray* views = [bookmarkBarView_ subviews]; + NSArray* views = [[self view] subviews]; for (NSButton* button in views) { NSButtonCell* cell = [button cell]; void* pointer = [[cell representedObject] pointerValue]; @@ -340,10 +385,6 @@ const CGFloat kBookmarkHorizontalPadding = 8.0; [self loaded:model]; } -- (NSView*)view { - return bookmarkBarView_; -} - - (void)setDelegate:(id<BookmarkURLOpener>)delegate { delegate_ = delegate; } diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm index 671e9f1..e852d90 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm @@ -14,6 +14,7 @@ @interface BookmarkURLOpenerPong : NSObject<BookmarkURLOpener> { @public GURL url_; + WindowOpenDisposition disposition_; } @end @@ -21,6 +22,7 @@ - (void)openBookmarkURL:(const GURL&)url disposition:(WindowOpenDisposition)disposition { url_ = url; + disposition_ = disposition; } @end @@ -33,24 +35,21 @@ class BookmarkBarControllerTest : public testing::Test { public: BookmarkBarControllerTest() { NSRect content_frame = NSMakeRect(0, 0, 800, kContentAreaHeight); - NSRect bar_frame = NSMakeRect(0, 0, 800, 0); + NSRect parent_frame = NSMakeRect(0, 0, 800, 50); content_area_.reset([[NSView alloc] initWithFrame:content_frame]); - bar_view_.reset([[NSView alloc] initWithFrame:bar_frame]); - [bar_view_ setHidden:YES]; - BookmarkBarView *bbv = (BookmarkBarView*)bar_view_.get(); + parent_view_.reset([[NSView alloc] initWithFrame:parent_frame]); + [parent_view_ setHidden:YES]; bar_.reset( [[BookmarkBarController alloc] initWithProfile:helper_.profile() - view:bbv + parentView:parent_view_.get() webContentView:content_area_.get() delegate:nil]); - NSView* parent = cocoa_helper_.contentView(); - [parent addSubview:content_area_.get()]; - [parent addSubview:[bar_ view]]; + [bar_ view]; // force loading of the nib } CocoaTestHelper cocoa_helper_; // Inits Cocoa, creates window, etc... scoped_nsobject<NSView> content_area_; - scoped_nsobject<NSView> bar_view_; + scoped_nsobject<NSView> parent_view_; BrowserTestHelper helper_; scoped_nsobject<BookmarkBarController> bar_; }; @@ -59,7 +58,6 @@ TEST_F(BookmarkBarControllerTest, ShowHide) { // Assume hidden by default in a new profile. EXPECT_FALSE([bar_ isBookmarkBarVisible]); EXPECT_TRUE([[bar_ view] isHidden]); - EXPECT_EQ([bar_view_ frame].size.height, 0); // Show and hide it by toggling. [bar_ toggleBookmarkBar]; @@ -67,13 +65,14 @@ TEST_F(BookmarkBarControllerTest, ShowHide) { EXPECT_FALSE([[bar_ view] isHidden]); NSRect content_frame = [content_area_ frame]; EXPECT_NE(content_frame.size.height, kContentAreaHeight); - EXPECT_GT([bar_view_ frame].size.height, 0); + EXPECT_GT([[bar_ view] frame].size.height, 0); + [bar_ toggleBookmarkBar]; EXPECT_FALSE([bar_ isBookmarkBarVisible]); EXPECT_TRUE([[bar_ view] isHidden]); content_frame = [content_area_ frame]; EXPECT_EQ(content_frame.size.height, kContentAreaHeight); - EXPECT_EQ([bar_view_ frame].size.height, 0); + EXPECT_EQ([[bar_ view] frame].size.height, 0); } // Confirm openBookmark: forwards the request to the controller's delegate @@ -88,18 +87,61 @@ TEST_F(BookmarkBarControllerTest, OpenBookmark) { scoped_nsobject<NSButton> button([[NSButton alloc] init]); [button setCell:cell.get()]; [cell setRepresentedObject:[NSValue valueWithPointer:node.get()]]; - [bar_ openBookmark:button]; + [bar_ openBookmark:button]; EXPECT_EQ(pong.get()->url_, node->GetURL()); + EXPECT_EQ(pong.get()->disposition_, CURRENT_TAB); + + [bar_ setDelegate:nil]; } +// Confirm opening of bookmarks works from the menus (different +// dispositions than clicking on the button). +TEST_F(BookmarkBarControllerTest, OpenBookmarkFromMenus) { + scoped_nsobject<BookmarkURLOpenerPong> pong([[BookmarkURLOpenerPong alloc] + init]); + [bar_ setDelegate:pong.get()]; + + scoped_nsobject<NSMenu> menu([[NSMenu alloc] initWithTitle:@"I_dont_care"]); + scoped_nsobject<NSMenuItem> item([[NSMenuItem alloc] + initWithTitle:@"still_dont_care" + action:NULL + keyEquivalent:@""]); + scoped_nsobject<NSButtonCell> cell([[NSButtonCell alloc] init]); + [item setMenu:menu.get()]; + [menu setDelegate:cell]; + + const char* urls[] = { "http://walla.walla.ding.dong.com", + "http://i_dont_know.com", + "http://cee.enn.enn.dot.com" }; + SEL selectors[] = { @selector(openBookmarkInNewForegroundTab:), + @selector(openBookmarkInNewWindow:), + @selector(openBookmarkInIncognitoWindow:) }; + WindowOpenDisposition dispositions[] = { NEW_FOREGROUND_TAB, + NEW_WINDOW, + OFF_THE_RECORD }; + for (unsigned int i = 0; + i < sizeof(dispositions)/sizeof(dispositions[0]); + i++) { + scoped_ptr<BookmarkNode> node(new BookmarkNode(GURL(urls[i]))); + [cell setRepresentedObject:[NSValue valueWithPointer:node.get()]]; + [bar_ performSelector:selectors[i] withObject:item.get()]; + EXPECT_EQ(pong.get()->url_, node->GetURL()); + EXPECT_EQ(pong.get()->disposition_, dispositions[i]); + [cell setRepresentedObject:nil]; + } +} + + // TODO(jrg): Make sure showing the bookmark bar calls loaded: (to // process bookmarks) TEST_F(BookmarkBarControllerTest, ShowAndLoad) { } -// TODO(jrg): Make sure a cleared bar has no subviews -TEST_F(BookmarkBarControllerTest, Clear) { +// TODO(jrg): Make sure adding 1 bookmark adds 1 subview, and removing +// 1 removes 1 subview. (We can't test for a simple Clear since there +// will soon be views in here which aren't bookmarks.) +TEST_F(BookmarkBarControllerTest, ViewChanges) { } // TODO(jrg): Make sure loaded: does something useful diff --git a/chrome/browser/cocoa/bookmark_bar_view.h b/chrome/browser/cocoa/bookmark_bar_view.h index a8b343e..c92cc7f 100644 --- a/chrome/browser/cocoa/bookmark_bar_view.h +++ b/chrome/browser/cocoa/bookmark_bar_view.h @@ -13,7 +13,13 @@ // I expect changes for new features (themes, extensions, etc). @interface BookmarkBarView : BackgroundGradientView { + @private + IBOutlet NSMenu* barContextualMenu_; } @end +@interface BookmarkBarView(TestingAPI) +- (void)setContextMenu:(NSMenu*)menu; +@end + #endif // CHROME_BROWSER_COCOA_BOOKMARK_BAR_VIEW_H_ diff --git a/chrome/browser/cocoa/bookmark_bar_view.mm b/chrome/browser/cocoa/bookmark_bar_view.mm index 63171e4..9bdab26 100644 --- a/chrome/browser/cocoa/bookmark_bar_view.mm +++ b/chrome/browser/cocoa/bookmark_bar_view.mm @@ -5,4 +5,18 @@ #include "chrome/browser/cocoa/bookmark_bar_view.h" @implementation BookmarkBarView + +// Only hit in a unit test. +- (void)setContextMenu:(NSMenu*)menu { + barContextualMenu_ = menu; +} + +// Unlike controls, generic views don't have a well-defined context +// menu (e.g. responds to the "menu" selector). So we add our own. +- (NSMenu *)menuForEvent:(NSEvent *)event { + if ([event type] == NSRightMouseDown) + return barContextualMenu_; + return nil; +} + @end diff --git a/chrome/browser/cocoa/bookmark_bar_view_unittest.mm b/chrome/browser/cocoa/bookmark_bar_view_unittest.mm index 51b9f12..a1cb121 100644 --- a/chrome/browser/cocoa/bookmark_bar_view_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_view_unittest.mm @@ -2,5 +2,57 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// This file is intentionally empty; there is no code in -// BookmarkBarView to test yet. +#import <Cocoa/Cocoa.h> + +#import "chrome/browser/cocoa/bookmark_bar_view.h" +#import "chrome/browser/cocoa/cocoa_test_helper.h" +#include "testing/gtest/include/gtest/gtest.h" + +class BookmarkBarViewTest : public testing::Test { + CocoaTestHelper cocoa_helper_; // Inits Cocoa, creates window, etc... +}; + +// Make sure we only get a menu from a right-click. Not left-click or keyDown. +TEST_F(BookmarkBarViewTest, TestMenu) { + scoped_nsobject<BookmarkBarView> view([[BookmarkBarView alloc] + initWithFrame:NSMakeRect(0,0,10,10)]); + EXPECT_TRUE(view.get()); + + // Not loaded from a nib so we must set it explicitly + scoped_nsobject<NSMenu> menu([[NSMenu alloc] initWithTitle:@"spork"]); + [view setContextMenu:menu.get()]; + + EXPECT_TRUE([view menuForEvent:[NSEvent mouseEventWithType:NSRightMouseDown + location:NSMakePoint(0,0) + modifierFlags:0 + timestamp:0 + windowNumber:0 + context:nil + eventNumber:0 + clickCount:0 + pressure:0.0]]); + EXPECT_FALSE([view menuForEvent:[NSEvent mouseEventWithType:NSLeftMouseDown + location:NSMakePoint(0,0) + modifierFlags:0 + timestamp:0 + windowNumber:0 + context:nil + eventNumber:0 + clickCount:0 + pressure:0.0]]); + EXPECT_FALSE([view menuForEvent:[NSEvent keyEventWithType:NSKeyDown + location:NSMakePoint(0,0) + modifierFlags:0 + timestamp:0 + windowNumber:0 + context:nil + characters:@"x" + charactersIgnoringModifiers:@"x" + isARepeat:NO + keyCode:7]]); + + [view setContextMenu:nil]; +} + + + diff --git a/chrome/browser/cocoa/bookmark_button_cell.mm b/chrome/browser/cocoa/bookmark_button_cell.mm index 8f78010..2a5a3eb 100644 --- a/chrome/browser/cocoa/bookmark_button_cell.mm +++ b/chrome/browser/cocoa/bookmark_button_cell.mm @@ -3,12 +3,26 @@ // found in the LICENSE file. #import "chrome/browser/cocoa/bookmark_button_cell.h" +#import "third_party/GTM/AppKit/GTMTheme.h" @implementation BookmarkButtonCell - (id)initTextCell:(NSString *)string { if ((self = [super initTextCell:string])) { - [self setBordered:NO]; + [self setButtonType:NSMomentaryPushInButton]; + [self setBezelStyle:NSShadowlessSquareBezelStyle]; + [self setShowsBorderOnlyWhileMouseInside:YES]; + [self setControlSize:NSSmallControlSize]; + [self setAlignment:NSLeftTextAlignment]; + [self setFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]]; + [self setWraps:NO]; + // NSLineBreakByTruncatingMiddle seems more common on OSX but let's + // try to match Windows for a bit to see what happens. + [self setLineBreakMode:NSLineBreakByTruncatingTail]; + + // Theming doesn't work for bookmark buttons yet (text chucked). + [super setShouldTheme:NO]; + } return self; } @@ -20,4 +34,15 @@ return size; } +// We share the context menu among all bookmark buttons. To allow us +// to disambiguate when needed (e.g. "open bookmark"), we set the +// menu's delegate to be us. We (the cell) have the bookmark encoded +// in our represented object. +// Convention needed in -[BookmarkBarController openBookmarkIn***] calls. +- (NSMenu*)menu { + NSMenu* menu = [super menu]; + [menu setDelegate:self]; + return menu; +} + @end diff --git a/chrome/browser/cocoa/bookmark_button_cell_unittest.mm b/chrome/browser/cocoa/bookmark_button_cell_unittest.mm index 7b61ebd..7074084 100644 --- a/chrome/browser/cocoa/bookmark_button_cell_unittest.mm +++ b/chrome/browser/cocoa/bookmark_button_cell_unittest.mm @@ -30,5 +30,26 @@ TEST_F(BookmarkButtonCellTest, SizeForBounds) { EXPECT_TRUE(size.width < 200 && size.height < 200); } +// Make sure a cell's menu has the cell itself as the delegate. This +// is our convention for reusing the context menu across all bookmarks +// while being unambiguous when used. +TEST_F(BookmarkButtonCellTest, MenuDelegate) { + scoped_nsobject<BookmarkButtonCell> cell([[BookmarkButtonCell alloc] + initTextCell:@"Testing"]); + EXPECT_FALSE([cell.get() menu]); + + scoped_nsobject<NSMenu> menu([[NSMenu alloc] initWithTitle:@"foo"]); + [cell setMenu:menu.get()]; + EXPECT_TRUE([cell.get() menu]); + EXPECT_EQ([[cell.get() menu] delegate], cell.get()); + [cell setMenu:nil]; +} + +// Make sure the default from the base class is overridden +TEST_F(BookmarkButtonCellTest, MouseEnterStuff) { + scoped_nsobject<BookmarkButtonCell> cell([[BookmarkButtonCell alloc] + initTextCell:@"Testing"]); + EXPECT_TRUE([cell.get() showsBorderOnlyWhileMouseInside]); +} } // namespace diff --git a/chrome/browser/cocoa/gradient_button_cell.h b/chrome/browser/cocoa/gradient_button_cell.h index ae85e6e..5aeb249 100644 --- a/chrome/browser/cocoa/gradient_button_cell.h +++ b/chrome/browser/cocoa/gradient_button_cell.h @@ -7,6 +7,8 @@ #import <Cocoa/Cocoa.h> +#include "base/scoped_nsobject.h" + // Base class for button cells for toolbar and bookmark bar. // // This is a button cell that handles drawing/highlighting of buttons. @@ -22,7 +24,19 @@ enum { typedef NSInteger ButtonType; @interface GradientButtonCell : NSButtonCell { + @private + // Custom drawing means we need to perform our own mouse tracking if + // the cell is setShowsBorderOnlyWhileMouseInside:YES. + BOOL isMouseInside_; + scoped_nsobject<NSTrackingArea> trackingArea_; + BOOL shouldTheme_; } +// Turn off theming. Temporary work-around. +- (void)setShouldTheme:(BOOL)shouldTheme; +@end + +@interface GradientButtonCell(TestingAPI) +- (BOOL)isMouseInside; @end #endif // CHROME_BROWSER_COCOA_CHROMIUM_BUTTON_CELL_H_ diff --git a/chrome/browser/cocoa/gradient_button_cell.mm b/chrome/browser/cocoa/gradient_button_cell.mm index 1ad72d4..87c87ba 100644 --- a/chrome/browser/cocoa/gradient_button_cell.mm +++ b/chrome/browser/cocoa/gradient_button_cell.mm @@ -8,11 +8,62 @@ @implementation GradientButtonCell +- (id)initTextCell:(NSString*)string { + if ((self = [super initTextCell:string])) { + shouldTheme_ = YES; + } + return self; +} + +- (void)setShouldTheme:(BOOL)shouldTheme { + shouldTheme_ = shouldTheme; +} + - (NSBackgroundStyle)interiorBackgroundStyle { return [self isHighlighted] ? NSBackgroundStyleLowered : NSBackgroundStyleRaised; } +- (void)mouseEntered:(NSEvent *)theEvent { + isMouseInside_ = YES; + [[self controlView] setNeedsDisplay:YES]; +} + +- (void)mouseExited:(NSEvent *)theEvent { + isMouseInside_ = NO; + [[self controlView] setNeedsDisplay:YES]; +} + +- (BOOL)isMouseInside { + return trackingArea_ && isMouseInside_; +} + +// Since we have our own drawWithFrame:, we need to also have our own +// logic for determining when the mouse is inside for honoring this +// request. +- (void)setShowsBorderOnlyWhileMouseInside:(BOOL)showOnly { + [super setShowsBorderOnlyWhileMouseInside:showOnly]; + if (showOnly) { + if (trackingArea_.get()) { + [self setShowsBorderOnlyWhileMouseInside:NO]; + } + trackingArea_.reset([[NSTrackingArea alloc] + initWithRect:[[self controlView] + bounds] + options:(NSTrackingMouseEnteredAndExited | + NSTrackingActiveInActiveApp) + owner:self + userInfo:nil]); + [[self controlView] addTrackingArea:trackingArea_]; + } else { + if (trackingArea_) { + [[self controlView] removeTrackingArea:trackingArea_]; + trackingArea_.reset(nil); + isMouseInside_ = NO; + } + } +} + - (void)drawWithFrame:(NSRect)cellFrame inView:(NSView*)controlView { // Constants from Cole. Will kConstant them once the feedback loop // is complete. @@ -50,7 +101,9 @@ // Stroke the borders and appropriate fill gradient. If we're borderless, // the only time we want to draw the inner gradient is if we're highlighted. - if ([self isBordered] || pressed) { + if (([self isBordered] && ![self showsBorderOnlyWhileMouseInside]) || + pressed || + [self isMouseInside]) { [[NSColor colorWithCalibratedWhite:1.0 alpha:0.25] set]; [outerPath stroke]; @@ -129,8 +182,7 @@ - (void)drawInteriorWithFrame:(NSRect)cellFrame inView:(NSView*)controlView { GTMTheme* theme = [controlView gtm_theme]; - BOOL shouldTheme = YES; - if (shouldTheme) { + if (shouldTheme_) { BOOL isTemplate = [[self image] isTemplate]; [NSGraphicsContext saveGraphicsState]; diff --git a/chrome/browser/cocoa/gradient_button_cell_unittest.mm b/chrome/browser/cocoa/gradient_button_cell_unittest.mm index 42abf28..3e0f162 100644 --- a/chrome/browser/cocoa/gradient_button_cell_unittest.mm +++ b/chrome/browser/cocoa/gradient_button_cell_unittest.mm @@ -39,4 +39,25 @@ TEST_F(GradientButtonCellTest, Display) { [view_ display]; } +// Tracking rects +TEST_F(GradientButtonCellTest, TrackingRects) { + GradientButtonCell* cell = [view_ cell]; + EXPECT_FALSE([cell showsBorderOnlyWhileMouseInside]); + EXPECT_FALSE([cell isMouseInside]); + + [cell setShowsBorderOnlyWhileMouseInside:YES]; + [cell mouseEntered:nil]; + EXPECT_TRUE([cell isMouseInside]); + [cell mouseExited:nil]; + EXPECT_FALSE([cell isMouseInside]); + + [cell setShowsBorderOnlyWhileMouseInside:NO]; + EXPECT_FALSE([cell isMouseInside]); + + [cell setShowsBorderOnlyWhileMouseInside:YES]; + [cell setShowsBorderOnlyWhileMouseInside:YES]; + [cell setShowsBorderOnlyWhileMouseInside:NO]; + [cell setShowsBorderOnlyWhileMouseInside:NO]; +} + } // namespace diff --git a/chrome/browser/cocoa/tab_window_controller.mm b/chrome/browser/cocoa/tab_window_controller.mm index 14b6b7b..35d9f8b 100644 --- a/chrome/browser/cocoa/tab_window_controller.mm +++ b/chrome/browser/cocoa/tab_window_controller.mm @@ -16,10 +16,6 @@ @synthesize tabContentArea = tabContentArea_; - (void)windowDidLoad { - // TODO(jrg): a non-normal window (e.g. for pop-ups) needs more work - // than just removal of the tab strip offset. But this is enough to - // avoid confusion (e.g. "new tab" on popup gets created in a - // different window). if ([self isNormalWindow]) { // Place the tab bar above the content box and add it to the view hierarchy // as a sibling of the content view so it can overlap with the window frame. @@ -27,8 +23,8 @@ tabFrame.origin = NSMakePoint(0, NSMaxY(tabFrame)); tabFrame.size.height = NSHeight([tabStripView_ frame]); [tabStripView_ setFrame:tabFrame]; + [[[[self window] contentView] superview] addSubview:tabStripView_]; } - [[[[self window] contentView] superview] addSubview:tabStripView_]; } - (void)removeOverlay { diff --git a/chrome/browser/cocoa/toolbar_controller.h b/chrome/browser/cocoa/toolbar_controller.h index b6ec8d4..31c896e 100644 --- a/chrome/browser/cocoa/toolbar_controller.h +++ b/chrome/browser/cocoa/toolbar_controller.h @@ -13,7 +13,6 @@ #import "chrome/browser/cocoa/bookmark_bar_controller.h" #include "chrome/common/pref_member.h" -@class BookmarkBarView; class CommandUpdater; class LocationBar; class LocationBarViewMac; @@ -71,7 +70,6 @@ class ToolbarView; IBOutlet NSButton* pageButton_; IBOutlet NSButton* wrenchButton_; IBOutlet NSTextField* locationBar_; - IBOutlet BookmarkBarView* bookmarkBarView_; } // Initialize the toolbar and register for command updates. The profile is diff --git a/chrome/browser/cocoa/toolbar_controller.mm b/chrome/browser/cocoa/toolbar_controller.mm index b9b219c..d1a44c6 100644 --- a/chrome/browser/cocoa/toolbar_controller.mm +++ b/chrome/browser/cocoa/toolbar_controller.mm @@ -120,9 +120,15 @@ class PrefObserverBridge : public NotificationObserver { // Create a sub-controller for the bookmark bar. bookmarkBarController_.reset([[BookmarkBarController alloc] initWithProfile:profile_ - view:bookmarkBarView_ + parentView:[self view] webContentView:webContentView_ delegate:bookmarkBarDelegate_]); + + // Add bookmark bar to the view hierarchy. This also triggers the + // nib load. The bookmark bar is defined (in the nib) to be + // bottom-aligned to it's parent view (among other things), so + // position and resize properties don't need to be set. + [[self view] addSubview:[bookmarkBarController_ view]]; } - (LocationBar*)locationBar { @@ -223,7 +229,7 @@ class PrefObserverBridge : public NotificationObserver { - (NSArray*)toolbarViews { return [NSArray arrayWithObjects:backButton_, forwardButton_, reloadButton_, homeButton_, starButton_, goButton_, pageButton_, wrenchButton_, - locationBar_, bookmarkBarView_, nil]; + locationBar_, nil]; } // Moves |rect| to the right by |delta|, keeping the right side fixed by diff --git a/chrome/browser/cocoa/toolbar_controller_unittest.mm b/chrome/browser/cocoa/toolbar_controller_unittest.mm index 93e1cfe..ed3cf89 100644 --- a/chrome/browser/cocoa/toolbar_controller_unittest.mm +++ b/chrome/browser/cocoa/toolbar_controller_unittest.mm @@ -22,7 +22,7 @@ class ToolbarControllerTest : public testing::Test { // |-toolbarViews| method. enum { kBackIndex, kForwardIndex, kReloadIndex, kHomeIndex, kStarIndex, kGoIndex, - kPageIndex, kWrenchIndex, kLocationIndex, kBookmarkIndex, + kPageIndex, kWrenchIndex, kLocationIndex, }; ToolbarControllerTest() { |