diff options
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bubble_controller.mm | 31 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm | 40 |
2 files changed, 66 insertions, 5 deletions
diff --git a/chrome/browser/cocoa/bookmark_bubble_controller.mm b/chrome/browser/cocoa/bookmark_bubble_controller.mm index 06d95fd..9e44741 100644 --- a/chrome/browser/cocoa/bookmark_bubble_controller.mm +++ b/chrome/browser/cocoa/bookmark_bubble_controller.mm @@ -8,6 +8,7 @@ #include "chrome/browser/bookmarks/bookmark_model.h" #import "chrome/browser/cocoa/bookmark_bubble_controller.h" #import "chrome/browser/cocoa/bookmark_bubble_window.h" +#include "chrome/browser/metrics/user_metrics.h" #include "grit/generated_resources.h" @@ -28,7 +29,7 @@ alreadyBookmarked:(BOOL)alreadyBookmarked { if ((self = [super initWithNibName:@"BookmarkBubble" bundle:mac_util::MainAppBundle()])) { - // all these are weak... + // All these are weak... delegate_ = delegate; parentWindow_ = parentWindow; topLeftForBubble_ = topLeftForBubble; @@ -74,13 +75,19 @@ [self fillInFolderList]; } -- (IBAction)edit:(id)sender { +// Shows the bookmark editor sheet for more advanced editing. +- (void)showEditor { [self updateBookmarkNode]; [self closeWindow]; [delegate_ editBookmarkNode:node_]; [delegate_ doneWithBubbleController:self]; } +- (IBAction)edit:(id)sender { + UserMetrics::RecordAction(L"BookmarkBubble_Edit", model_->profile()); + [self showEditor]; +} + - (IBAction)close:(id)sender { if (node_) { // no node_ if the bookmark was just removed @@ -90,13 +97,21 @@ [delegate_ doneWithBubbleController:self]; } -// By implementing this, ESC causes the window to go away. +// By implementing this, ESC causes the window to go away. If clicking the +// star was what prompted this bubble to appear (i.e., not already bookmarked), +// remove the bookmark. - (IBAction)cancel:(id)sender { - [self close:sender]; + if (!alreadyBookmarked_) { + // |-remove:| calls |-close| so we don't have to bother. + [self remove:sender]; + } else { + [self close:sender]; + } } - (IBAction)remove:(id)sender { model_->SetURLStarred(node_->GetURL(), node_->GetTitle(), false); + UserMetrics::RecordAction(L"BookmarkBubble_Unstar", model_->profile()); node_ = NULL; // no longer valid [self close:self]; } @@ -106,7 +121,9 @@ - (void)comboBoxSelectionDidChange:(NSNotification*)notification { NSString* selected = [folderComboBox_ objectValueOfSelectedItem]; if ([selected isEqual:chooseAnotherFolder_.get()]) { - [self edit:self]; + UserMetrics::RecordAction(L"BookmarkBubble_EditFromCombobox", + model_->profile()); + [self showEditor]; } } @@ -195,6 +212,8 @@ NSString* newTitle = [nameTextField_ stringValue]; if (![oldTitle isEqual:newTitle]) { model_->SetTitle(node_, base::SysNSStringToWide(newTitle)); + UserMetrics::RecordAction(L"BookmarkBubble_ChangeTitleInBubble", + model_->profile()); } // Then the parent folder. NSString* oldParentTitle = base::SysWideToNSString( @@ -207,6 +226,8 @@ // newParent should only ever possibly be NULL in a unit test. int index = newParent->GetChildCount(); model_->Move(node_, newParent, index); + UserMetrics::RecordAction(L"BookmarkBubble_ChangeParent", + model_->profile()); } } } diff --git a/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm index 2e741c0..91dfa72 100644 --- a/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm @@ -209,5 +209,45 @@ TEST_F(BookmarkBubbleControllerTest, ComboSelectionChanged) { EXPECT_EQ([delegate_ edits], 1); } +// Create a controller that simulates the bookmark just now being created by +// the user clicking the star, then sending the "cancel" command to represent +// them pressing escape. The bookmark should not be there. +TEST_F(BookmarkBubbleControllerTest, EscapeRemovesNewBookmark) { + BookmarkModel* model = GetBookmarkModel(); + GURL gurl("http://www.google.com"); + const BookmarkNode* node = model->AddURL(model->GetBookmarkBarNode(), + 0, + L"Bookie markie title", + gurl); + scoped_nsobject<BookmarkBubbleController> controller( + [[BookmarkBubbleController alloc] + initWithDelegate:delegate_.get() + parentWindow:cocoa_helper_.window() + topLeftForBubble:[delegate_ topLeftForBubble] + model:helper_.profile()->GetBookmarkModel() + node:node + alreadyBookmarked:NO]); // The last param is the key difference. + EXPECT_TRUE(controller); + + [controller cancel:nil]; + EXPECT_FALSE(model->IsBookmarked(gurl)); +} + +// Create a controller where the bookmark already existed prior to clicking +// the star and test that sending a cancel command doesn't change the state +// of the bookmark. +TEST_F(BookmarkBubbleControllerTest, EscapeDoesntTouchExistingBookmark) { + BookmarkModel* model = GetBookmarkModel(); + GURL gurl("http://www.google.com"); + const BookmarkNode* node = model->AddURL(model->GetBookmarkBarNode(), + 0, + L"Bookie markie title", + gurl); + BookmarkBubbleController* controller = ControllerForNode(node); + EXPECT_TRUE(controller); + + [(id)controller cancel:nil]; + EXPECT_TRUE(model->IsBookmarked(gurl)); +} } // namespace |