diff options
author | mrossetti@chromium.org <mrossetti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-18 17:11:02 +0000 |
---|---|---|
committer | mrossetti@chromium.org <mrossetti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-18 17:11:02 +0000 |
commit | 4ac884b18f10f3ae3a5daafbfa25f4545dd401ce (patch) | |
tree | daca6e7903d29ccf169723a88265574f1fa90d29 /chrome | |
parent | b808eb6f0f47a7c4ad15dd8ec115404ce14b47b6 (diff) | |
download | chromium_src-4ac884b18f10f3ae3a5daafbfa25f4545dd401ce.zip chromium_src-4ac884b18f10f3ae3a5daafbfa25f4545dd401ce.tar.gz chromium_src-4ac884b18f10f3ae3a5daafbfa25f4545dd401ce.tar.bz2 |
Implement drag from bookmark manager to bookmark bar or a folder thereof. Implement drag of other bookmark and URL sources such as from the Desktop or a text selection.
BUG=39884,44228
TEST=Note: For all drag operations verify that the appropriate drop indicator is presented while dragging. When dragging onto a bookmark bar folder the folder button should be highlighted and, after a short delay, open into a menu. When dragging between folders or on top of a non-folder bookmark an insertion bar (vertical when in the bookmark bar, horizontal when in a folder menu) should be presented. For 'copy' moves the drag image should be adorned with a plus sign in a green circle. A non-copy move should show no such tag.
1) Drag a non-folder bookmark from the bookmark manager to the bookmark bar such that it falls on a non-folder bookmark. Verify that the bookmark is moved to be on the bar just before the bookmark upon which is was dropped.
2) Drag a folder bookmark from the manager to the bookmark bar such that it falls on a non-folder bookmark. Verify that the folder (as a folder) is moved to be on the bar just before the bookmark upon which is was dropped.
3) Drag a bookmark from the manager to a folder on the bookmark bar. Verify that the bookmark is added to the folder at the end and removed from the old location in the manager.
4) Drag a bookmark from the manager to a folder on the bookmark bar and wait for the folder to open and then drag to within the folder. Verify that dropping places the bookmark and removes it from the old location in the manager.
5) Drag a bookmark from the bar to the manager and verify that the manager now shows the dragged bookmark. (Note that this should be a 'move' but is currently a 'copy'. See http://crbug.com/44039.)
6) Drag a bookmark clipping from the Finder Desktop to the bookmark bar. Verify that it is added to the bar as a new bookmark.
7) Drag a bookmark clipping from the Finder Desktop to a folder on the bookmark bar and drop it within the folder menu. Verify that it is added to the folder as a new bookmark.
8) Create a URL in TextEdit or Stickies, select it and drag to the bookmark bar. Verify that it is added to the bar.
9) Create a URL in TextEdit or Stickies, select it and drag to a folder on the bookmark bar and drop it within the folder menu. Verify that it is added to the folder.
Review URL: http://codereview.chromium.org/2066001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@47523 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
17 files changed, 1146 insertions, 338 deletions
diff --git a/chrome/browser/bookmarks/bookmark_pasteboard_helper_mac.h b/chrome/browser/bookmarks/bookmark_pasteboard_helper_mac.h index 7cf5e46..00cf6d7 100644 --- a/chrome/browser/bookmarks/bookmark_pasteboard_helper_mac.h +++ b/chrome/browser/bookmarks/bookmark_pasteboard_helper_mac.h @@ -48,4 +48,11 @@ void StartDrag(Profile* profile, const std::vector<const BookmarkNode*>& nodes, } +#ifdef __OBJC__ +@class NSString; +// Pasteboard type for dictionary containing bookmark structure consisting +// of individual bookmark nodes and/or bookmark folders. +extern "C" NSString* const kBookmarkDictionaryListPboardType; +#endif // __OBJC__ + #endif // CHROME_BROWSER_BOOKMARKS_BOOKMARK_PASTEBOARD_HELPER_MAC_H_ diff --git a/chrome/browser/bookmarks/bookmark_pasteboard_helper_mac.mm b/chrome/browser/bookmarks/bookmark_pasteboard_helper_mac.mm index dd8f938..6d692f6 100644 --- a/chrome/browser/bookmarks/bookmark_pasteboard_helper_mac.mm +++ b/chrome/browser/bookmarks/bookmark_pasteboard_helper_mac.mm @@ -11,6 +11,9 @@ #include "chrome/browser/cocoa/bookmark_drag_source.h" #include "chrome/browser/tab_contents/tab_contents_view_mac.h" +NSString* const kBookmarkDictionaryListPboardType = + @"BookmarkDictionaryListPboardType"; + namespace { // An unofficial standard pasteboard title type to be provided alongside the @@ -33,9 +36,6 @@ NSString* const kChromiumBookmarkId = NSString* const kWebURLsWithTitlesPboardType = @"WebURLsWithTitlesPboardType"; -NSString* const kBookmarkDictionaryListPboardType = - @"BookmarkDictionaryListPboardType"; - // Keys for the type of node in BookmarkDictionaryListPboardType. NSString* const kWebBookmarkType = @"WebBookmarkType"; diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h index 9804be3..bde2cc2 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller.h +++ b/chrome/browser/cocoa/bookmark_bar_controller.h @@ -261,9 +261,6 @@ willAnimateFromState:(bookmarks::VisualState)oldState // shouldn't be shown. - (CGFloat)toolbarDividerOpacity; -// Returns true if at least one bookmark was added. -- (BOOL)addURLs:(NSArray*)urls withTitles:(NSArray*)titles at:(NSPoint)point; - // Updates the sizes and positions of the subviews. // TODO(viettrungluu): I'm not convinced this should be public, but I currently // need it for animations. Try not to propagate its use. @@ -278,6 +275,16 @@ willAnimateFromState:(bookmarks::VisualState)oldState // Provide a favIcon for a bookmark node. May return nil. - (NSImage*)favIconForNode:(const BookmarkNode*)node; +// Used for situations where the bookmark bar folder menus should no longer +// be actively popping up. Called when the window loses focus, a click has +// occured outside the menus or a bookmark has been activated. (Note that this +// differs from the behavior of the -[BookmarkButtonControllerProtocol +// closeAllBookmarkFolders] method in that the latter does not terminate menu +// tracking since it may be being called in response to actions (such as +// dragging) where a 'stale' menu presentation should first be collapsed before +// presenting a new menu.) +- (void)closeFolderAndStopTrackingMenus; + // Actions for manipulating bookmarks. // Open a normal bookmark or folder from a button, ... - (IBAction)openBookmark:(id)sender; @@ -351,9 +358,4 @@ willAnimateFromState:(bookmarks::VisualState)oldState @end -// The (internal) |NSPasteboard| type string for bookmark button drags, used for -// dragging buttons around the bookmark bar. The data for this type is just a -// pointer to the |BookmarkButton| being dragged. -extern NSString* kBookmarkButtonDragType; - #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 4d70f0a..312e3b8 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller.mm +++ b/chrome/browser/cocoa/bookmark_bar_controller.mm @@ -159,12 +159,22 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; // the animation, NO if not (and hence it should be done instantly). - (BOOL)doBookmarkBarAnimation; -// Returns the index in the model for a drag of the given button to the given -// location; currently, only the x-coordinate of |point| is considered. I -// reserve the right to check for errors, in which case this would return -// negative value; callers should check for this. -- (int)indexForDragOfButton:(BookmarkButton*)sourceButton - toPoint:(NSPoint)point; +// |point| is in the base coordinate system of the destination window; +// it comes from an id<NSDraggingInfo>. |copy| is YES if a copy is to be +// made and inserted into the new location while leaving the bookmark in +// the old location, otherwise move the bookmark by removing from its old +// location and inserting into the new location. +- (BOOL)dragBookmark:(const BookmarkNode*)sourceNode + to:(NSPoint)point + copy:(BOOL)copy; + +// Returns the index in the model for a drag to the location given by +// |point|. This is determined by finding the first button before the center +// of which |point| falls, scanning left to right. Note that, currently, only +// the x-coordinate of |point| is considered. Though not currently implemented, +// we may check for errors, in which case this would return negative value; +// callers should check for this. +- (int)indexForDragToPoint:(NSPoint)point; // Add or remove buttons to/from the bar until it is filled but not overflowed. - (void)redistributeButtonsOnBarAsNeeded; @@ -174,16 +184,6 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; // then show the no items label. - (void)reconfigureBookmarkBar; -// Used for situations where the bookmark bar folder menus should no longer -// be actively popping up. Called when the window loses focus, a click has -// occured outside the menus or a bookmark has been activated. (Note that this -// differs from the behavior of the -[BookmarkButtonControllerProtocol -// closeAllBookmarkFolders] method in that the latter does not terminate menu -// tracking since it may be being called in response to actions (such as -// dragging) where a 'stale' menu presentation should first be collapsed before -// presenting a new menu.) -- (void)closeFolderAndStopTrackingMenus; - - (void)addNode:(const BookmarkNode*)child toMenu:(NSMenu*)menu; - (void)addFolderNode:(const BookmarkNode*)node toMenu:(NSMenu*)menu; - (void)tagEmptyMenu:(NSMenu*)menu; @@ -393,7 +393,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; - (void)configureOffTheSideButtonContentsAndVisibility { [[offTheSideButton_ cell] setStartingChildIndex:displayedButtonCount_]; [[offTheSideButton_ cell] - setBookmarkNode:bookmarkModel_->GetBookmarkBarNode()]; + setBookmarkNode:bookmarkModel_->GetBookmarkBarNode()]; int bookmarkChildren = bookmarkModel_->GetBookmarkBarNode()->GetChildCount(); if (bookmarkChildren > displayedButtonCount_) { [offTheSideButton_ setHidden:NO]; @@ -497,7 +497,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; // focus. - (void)watchForExitEvent:(BOOL)watch { CrApplication* app = static_cast<CrApplication*>([NSApplication - sharedApplication]); + sharedApplication]); DCHECK([app isKindOfClass:[CrApplication class]]); if (watch) { if (!watchingForExitEvent_) @@ -542,7 +542,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; // If a click in a bookmark bar folder window and that isn't // one of my bookmark bar folders, YES is click outside. if (![eventWindow isKindOfClass:[BookmarkBarFolderWindow - class]]) { + class]]) { return YES; } break; @@ -573,7 +573,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; // The textfield sits in the itemcontainer, so to center it we maintain // equal vertical padding on the top and bottom. int yoffset = (NSHeight([[buttonView_ noItemTextfield] frame]) - - NSHeight([[buttonView_ noItemContainer] frame])) / 2; + NSHeight([[buttonView_ noItemContainer] frame])) / 2; [[buttonView_ noItemContainer] setFrameOrigin:NSMakePoint(0, yoffset)]; } @@ -721,44 +721,54 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; return [self isInState:bookmarks::kShowingState] ? 0 : 1; } +// TODO(mrossetti): Duplicate code with BookmarkBarFolderController. +// http://crbug.com/35966 - (BOOL)addURLs:(NSArray*)urls withTitles:(NSArray*)titles at:(NSPoint)point { - // TODO(jrg): Support drops on folders etc - // TODO(jrg): Use |point|. DCHECK([urls count] == [titles count]); - const BookmarkNode* node = bookmarkModel_->GetBookmarkBarNode(); - int added = 0; - - for (size_t i = 0; i < [urls count]; ++i) { - // URLs come in several forms (see NSPasteboard+Utils.mm). - GURL gurl; - const char* string = [[urls objectAtIndex:i] UTF8String]; - if (string) - gurl = GURL(string); - if (!gurl.is_valid() && string) { - gurl = GURL([[NSString stringWithFormat:@"file://%s", string] - UTF8String]); - } + BOOL nodesWereAdded = NO; + // Figure out where these new bookmarks nodes are to be added. + BookmarkButton* button = [self buttonForDroppingOnAtPoint:point]; + const BookmarkNode* destParent = NULL; + int destIndex = 0; + if ([button isFolder]) { + destParent = [button bookmarkNode]; + // Drop it at the end. + destIndex = [button bookmarkNode]->GetChildCount(); + } else { + // Else we're dropping somewhere on the bar, so find the right spot. + destParent = bookmarkModel_->GetBookmarkBarNode(); + destIndex = [self indexForDragToPoint:point]; + } - DCHECK(gurl.is_valid()); - if (gurl.is_valid()) { - bookmarkModel_->AddURL( - node, - node->GetChildCount(), - base::SysNSStringToWide([titles objectAtIndex:i]), - gurl); - added++; + // Don't add the bookmarks if the destination index shows an error. + if (destIndex >= 0) { + // Create and add the new bookmark nodes. + size_t urlCount = [urls count]; + for (size_t i = 0; i < urlCount; ++i) { + // URLs come in several forms (see NSPasteboard+Utils.mm). + GURL gurl; + const char* string = [[urls objectAtIndex:i] UTF8String]; + if (string) + gurl = GURL(string); + if (!gurl.is_valid() && string) { + gurl = GURL([[NSString stringWithFormat:@"file://%s", string] + UTF8String]); + } + DCHECK(gurl.is_valid()); + if (gurl.is_valid()) { + bookmarkModel_->AddURL(destParent, + destIndex++, + base::SysNSStringToWide([titles + objectAtIndex:i]), + gurl); + nodesWereAdded = YES; + } } } - return (added ? YES : NO); + return nodesWereAdded; } -- (int)indexForDragOfButton:(BookmarkButton*)sourceButton - toPoint:(NSPoint)point { - DCHECK([sourceButton isKindOfClass:[BookmarkButton class]]); - - // Identify which buttons we are between. For now, assume a button - // location is at the center point of its view, and that an exact - // match means "place before". +- (int)indexForDragToPoint:(NSPoint)point { // TODO(jrg): revisit position info based on UI team feedback. // dropLocation is in bar local coordinates. NSPoint dropLocation = @@ -785,14 +795,22 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; return displayedButtonCount_; } +// TODO(mrossetti,jrg): Yet more code dup with BookmarkBarFolderController. +// http://crbug.com/35966 - (BOOL)dragButton:(BookmarkButton*)sourceButton to:(NSPoint)point copy:(BOOL)copy { DCHECK([sourceButton isKindOfClass:[BookmarkButton class]]); - const BookmarkNode* sourceNode = [sourceButton bookmarkNode]; - DCHECK(sourceNode); + return [self dragBookmark:sourceNode to:point copy:copy]; +} +// TODO(mrossetti,jrg): Yet more duplicated code. +// http://crbug.com/35966 +- (BOOL)dragBookmark:(const BookmarkNode*)sourceNode + to:(NSPoint)point + copy:(BOOL)copy { + DCHECK(sourceNode); // Drop destination. const BookmarkNode* destParent = NULL; int destIndex = 0; @@ -807,7 +825,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; } else { // Else we're dropping somewhere on the bar, so find the right spot. destParent = bookmarkModel_->GetBookmarkBarNode(); - destIndex = [self indexForDragOfButton:sourceButton toPoint:point]; + destIndex = [self indexForDragToPoint:point]; } // Be sure we don't try and drop a folder into itself. @@ -841,8 +859,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { fromArray:(NSArray*)array { for (BookmarkButton* button in array) { // Break early if we've gone too far. - if ((NSMinX([button frame]) > point.x) || - (![button superview])) + if ((NSMinX([button frame]) > point.x) || (![button superview])) return nil; // Careful -- this only applies to the bar with horiz buttons. // Intentionally NOT using NSPointInRect() so that scrolling into @@ -876,6 +893,8 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { // none. Works with both the bookmark buttons and the "Other // Bookmarks" button. Point is in [self view] coordinates. - (BookmarkButton*)buttonForDroppingOnAtPoint:(NSPoint)point { + point = [[self view] convertPoint:point + fromView:[[[self view] window] contentView]]; BookmarkButton* button = [self buttonForDroppingOnAtPoint:point fromArray:buttons_.get()]; // One more chance -- try "Other Bookmarks" and "off the side" (if visible). @@ -919,7 +938,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { if (hoverButton_) { // Oops, another one triggered or open. [NSObject cancelPreviousPerformRequestsWithTarget:[hoverButton_ - target]]; + target]]; // Unlike BookmarkBarFolderController, we do not delay the close // of the previous one. Given the lack of diagonal movement, // there is no need, and it feels awkward to do so. See @@ -930,7 +949,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { } hoverButton_.reset([button retain]); DCHECK([[hoverButton_ target] - respondsToSelector:@selector(openBookmarkFolderFromButton:)]); + respondsToSelector:@selector(openBookmarkFolderFromButton:)]); [[hoverButton_ target] performSelector:@selector(openBookmarkFolderFromButton:) withObject:hoverButton_ @@ -956,16 +975,19 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { } } +- (void)draggingEnded:(id<NSDraggingInfo>)info { + [self closeFolderAndStopTrackingMenus]; +} + // Return YES if we should show the drop indicator, else NO. - (BOOL)shouldShowIndicatorShownForPoint:(NSPoint)point { return ![self buttonForDroppingOnAtPoint:point]; } // Return the x position for a drop indicator. -- (CGFloat)indicatorPosForDragOfButton:(BookmarkButton*)sourceButton - toPoint:(NSPoint)point { +- (CGFloat)indicatorPosForDragToPoint:(NSPoint)point { CGFloat x = 0; - int destIndex = [self indexForDragOfButton:sourceButton toPoint:point]; + int destIndex = [self indexForDragToPoint:point]; int numButtons = displayedButtonCount_; // If it's a drop strictly between existing buttons ... @@ -986,8 +1008,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { [buttons_ objectAtIndex:static_cast<NSUInteger>(destIndex - 1)]; DCHECK(button); NSRect buttonFrame = [button frame]; - x = buttonFrame.origin.x + buttonFrame.size.width + - 0.5 * bookmarks::kBookmarkHorizontalPadding; + x = NSMaxX(buttonFrame) + 0.5 * bookmarks::kBookmarkHorizontalPadding; // Otherwise, put it right at the beginning. } else { @@ -1038,6 +1059,33 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { ![self isAnimatingToState:bookmarks::kDetachedState]; } +- (std::vector<const BookmarkNode*>)retrieveBookmarkDragDataNodes { + std::vector<const BookmarkNode*> dragDataNodes; + BookmarkDragData dragData; + if(dragData.ReadFromDragClipboard()) { + BookmarkModel* bookmarkModel = [self bookmarkModel]; + Profile* profile = bookmarkModel->profile(); + std::vector<const BookmarkNode*> nodes(dragData.GetNodes(profile)); + dragDataNodes.assign(nodes.begin(), nodes.end()); + } + return dragDataNodes; +} + +- (BOOL)dragBookmarkData:(id<NSDraggingInfo>)info { + BOOL dragged = NO; + std::vector<const BookmarkNode*> nodes([self retrieveBookmarkDragDataNodes]); + if (nodes.size()) { + BOOL copy = !([info draggingSourceOperationMask] & NSDragOperationMove); + NSPoint dropPoint = [info draggingLocation]; + for (std::vector<const BookmarkNode*>::const_iterator it = nodes.begin(); + it != nodes.end(); ++it) { + const BookmarkNode* sourceNode = *it; + dragged = [self dragBookmark:sourceNode to:dropPoint copy:copy]; + } + } + return dragged; +} + - (NSWindow*)browserWindow { return [[self view] window]; } @@ -1201,8 +1249,8 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { // Add a tooltip std::string url_string = child->GetURL().possibly_invalid_spec(); NSString* tooltip = [NSString stringWithFormat:@"%@\n%s", - base::SysWideToNSString(child->GetTitle()), - url_string.c_str()]; + base::SysWideToNSString(child->GetTitle()), + url_string.c_str()]; [item setToolTip:tooltip]; } } @@ -1438,11 +1486,11 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { newIndex = parent->IndexOfChild(senderNode) + 1; } BookmarkNameFolderController* controller = - [[BookmarkNameFolderController alloc] - initWithParentWindow:[[self view] window] - profile:browser_->profile() - parent:parent - newIndex:newIndex]; + [[BookmarkNameFolderController alloc] + initWithParentWindow:[[self view] window] + profile:browser_->profile() + parent:parent + newIndex:newIndex]; [controller runAsModalSheet]; } @@ -1461,7 +1509,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { // Make sure there are no stale pointers in the pasteboard. This // can be important if a bookmark is deleted (via bookmark sync) // while in the middle of a drag. The "drag completed" code - // (e.g. [BookmarkBarView performDragOperationForBookmark:]) is + // (e.g. [BookmarkBarView performDragOperationForBookmarkButton:]) is // careful enough to bail if there is no data found at "drop" time. // // Unfortunately the clearContents selector is 10.6 only. The best @@ -1633,7 +1681,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { NSMutableArray* buttons = [self buttons]; for (NSButton* button in buttons) { if (NSMaxX([button frame]) > (NSMinX([offTheSideButton_ frame]) - - bookmarks::kBookmarkHorizontalPadding)) + bookmarks::kBookmarkHorizontalPadding)) break; [buttonView_ addSubview:button]; ++displayedButtonCount_; @@ -1641,8 +1689,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { NSUInteger removalCount = [buttons count] - (NSUInteger)displayedButtonCount_; if (removalCount > 0) { - NSRange removalRange = - NSMakeRange(displayedButtonCount_, removalCount); + NSRange removalRange = NSMakeRange(displayedButtonCount_, removalCount); [buttons removeObjectsInRange:removalRange]; } } diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm index e1c2c9a4..d02d927 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm @@ -128,6 +128,43 @@ @end +// Just like a BookmarkBarController but intercedes when providing +// pasteboard drag data. +@interface BookmarkBarControllerDragData : BookmarkBarController { + const BookmarkNode* dragDataNode_; // Weak +} +- (void)setDragDataNode:(const BookmarkNode*)node; +@end + +@implementation BookmarkBarControllerDragData + +- (id)initWithBrowser:(Browser*)browser + initialWidth:(CGFloat)initialWidth + delegate:(id<BookmarkBarControllerDelegate>)delegate + resizeDelegate:(id<ViewResizer>)resizeDelegate { + if ((self = [super initWithBrowser:browser + initialWidth:initialWidth + delegate:delegate + resizeDelegate:resizeDelegate])) { + dragDataNode_ = NULL; + } + return self; +} + +- (void)setDragDataNode:(const BookmarkNode*)node { + dragDataNode_ = node; +} + +- (std::vector<const BookmarkNode*>)retrieveBookmarkDragDataNodes { + std::vector<const BookmarkNode*> dragDataNodes; + if(dragDataNode_) { + dragDataNodes.push_back(dragDataNode_); + } + return dragDataNodes; +} + +@end + class FakeTheme : public ThemeProvider { public: @@ -159,6 +196,54 @@ class FakeTheme : public ThemeProvider { }; +@interface FakeDragInfo : NSObject { + @public + NSPoint dropLocation_; + NSDragOperation sourceMask_; +} +@property (assign) NSPoint dropLocation; +- (void)setDraggingSourceOperationMask:(NSDragOperation)mask; +@end + +@implementation FakeDragInfo + +@synthesize dropLocation = dropLocation_; + +- (id)init { + if ((self = [super init])) { + dropLocation_ = NSZeroPoint; + sourceMask_ = NSDragOperationMove; + } + return self; +} + +// NSDraggingInfo protocol functions. + +- (id)draggingPasteboard { + return self; +} + +- (id)draggingSource { + return self; +} + +- (NSDragOperation)draggingSourceOperationMask { + return sourceMask_; +} + +- (NSPoint)draggingLocation { + return dropLocation_; +} + +// Other functions. + +- (void)setDraggingSourceOperationMask:(NSDragOperation)mask { + sourceMask_ = mask; +} + +@end + + namespace { class BookmarkBarControllerTestBase : public CocoaTest { @@ -1011,7 +1096,8 @@ TEST_F(BookmarkBarControllerTest, TestDragButton) { model->AddURL(folder, 0, L"already", GURL("http://www.google.com")); EXPECT_EQ(arraysize(titles) + 1, [[bar_ buttons] count]); EXPECT_EQ(1, folder->GetChildCount()); - x = [[[bar_ buttons] objectAtIndex:0] frame].size.width / 2; + x = NSMidX([[[bar_ buttons] objectAtIndex:0] frame]); + x += [[bar_ view] frame].origin.x; std::wstring title = [[[bar_ buttons] objectAtIndex:2] bookmarkNode]->GetTitle(); [bar_ dragButton:[[bar_ buttons] objectAtIndex:2] @@ -1307,8 +1393,9 @@ TEST_F(BookmarkBarControllerTest, DropDestination) { // Confirm "right in the center" (give or take a pixel) is a match, // and confirm "just barely in the button" is not. Anything more // specific seems likely to be tweaked. + CGFloat viewFrameXOffset = [[bar_ view] frame].origin.x; for (BookmarkButton* button in [bar_ buttons]) { - CGFloat x = NSMidX([button frame]); + CGFloat x = NSMidX([button frame]) + viewFrameXOffset; // Somewhere near the center: a match EXPECT_EQ(button, [bar_ buttonForDroppingOnAtPoint:NSMakePoint(x-1, 10)]); @@ -1317,10 +1404,10 @@ TEST_F(BookmarkBarControllerTest, DropDestination) { EXPECT_FALSE([bar_ shouldShowIndicatorShownForPoint:NSMakePoint(x, 10)]);; // On the very edges: NOT a match - x = NSMinX([button frame]); + x = NSMinX([button frame]) + viewFrameXOffset; EXPECT_NE(button, [bar_ buttonForDroppingOnAtPoint:NSMakePoint(x, 9)]); - x = NSMaxX([button frame]); + x = NSMaxX([button frame]) + viewFrameXOffset; EXPECT_NE(button, [bar_ buttonForDroppingOnAtPoint:NSMakePoint(x, 11)]); } @@ -1494,8 +1581,6 @@ TEST_F(BookmarkBarControllerOpenAllTest, OpenAllBookmarks) { (BookmarkBarControllerOpenAllPong*)bar_.get(); EXPECT_EQ([specialBar dispositionDetected], NEW_FOREGROUND_TAB); - std::cout << "OPEN_ALL_BOOKMARKS C" << std::endl; - // Now try an OpenAll... from a folder node. [specialBar setDispositionDetected:IGNORE_ACTION]; // Reset [bar_ openAllBookmarks:ItemForBookmarkBarMenu(folder_)]; @@ -1509,8 +1594,6 @@ TEST_F(BookmarkBarControllerOpenAllTest, OpenAllNewWindow) { (BookmarkBarControllerOpenAllPong*)bar_.get(); EXPECT_EQ([specialBar dispositionDetected], NEW_WINDOW); - std::cout << "OPEN_ALL_BOOKMARKS C" << std::endl; - // Now try an OpenAll... from a folder node. [specialBar setDispositionDetected:IGNORE_ACTION]; // Reset [bar_ openAllBookmarksNewWindow:ItemForBookmarkBarMenu(folder_)]; @@ -1524,8 +1607,6 @@ TEST_F(BookmarkBarControllerOpenAllTest, OpenAllIncognito) { (BookmarkBarControllerOpenAllPong*)bar_.get(); EXPECT_EQ([specialBar dispositionDetected], OFF_THE_RECORD); - std::cout << "OPEN_ALL_BOOKMARKS C" << std::endl; - // Now try an OpenAll... from a folder node. [specialBar setDispositionDetected:IGNORE_ACTION]; // Reset [bar_ openAllBookmarksIncognitoWindow:ItemForBookmarkBarMenu(folder_)]; @@ -1624,6 +1705,12 @@ TEST_F(BookmarkBarControllerNotificationTest, DeregistersForNotifications) { class BookmarkBarControllerDragDropTest : public BookmarkBarControllerTestBase { public: BookmarkBarControllerDragDropTest() { + bar_.reset( + [[BookmarkBarControllerDragData alloc] + initWithBrowser:helper_.browser() + initialWidth:NSWidth([parent_view_ frame]) + delegate:nil + resizeDelegate:resizeDelegate_.get()]); InstallAndToggleBar(bar_.get()); } }; @@ -1643,7 +1730,7 @@ TEST_F(BookmarkBarControllerDragDropTest, DragMoveBarBookmarkToOffTheSide) { std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ(model_string, actualModelString); - // Insure that the off-the-side is showing. + // Insure that the off-the-side is not showing. ASSERT_FALSE([bar_ offTheSideButtonIsHidden]); // Remember how many buttons are showing and are available. @@ -1680,6 +1767,98 @@ TEST_F(BookmarkBarControllerDragDropTest, DragMoveBarBookmarkToOffTheSide) { EXPECT_EQ(newOTSCount, newChildCount - newDisplayedButtons); } +TEST_F(BookmarkBarControllerDragDropTest, DragBookmarkData) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] " + "2f3b ] 3b 4b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + const BookmarkNode* other = model.other_node(); + const std::wstring other_string(L"O1b O2b O3f:[ O3f1b O3f2f ] " + "O4f:[ O4f1b O4f2f ] 05b "); + model_test_utils::AddNodesFromModelString(model, other, other_string); + + // Validate initial model. + std::wstring actual = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actual); + actual = model_test_utils::ModelStringFromNode(other); + EXPECT_EQ(other_string, actual); + + // Remember the little ones. + int oldChildCount = root->GetChildCount(); + + BookmarkButton* targetButton = [bar_ buttonWithTitleEqualTo:@"3b"]; + ASSERT_TRUE(targetButton); + + // Gen up some dragging data. + const BookmarkNode* newNode = other->GetChild(2); + [bar_ setDragDataNode:newNode]; + scoped_nsobject<FakeDragInfo> dragInfo([[FakeDragInfo alloc] init]); + [dragInfo setDropLocation:[targetButton center]]; + [bar_ dragBookmarkData:(id<NSDraggingInfo>)dragInfo.get()]; + + // There should one more button in the bar. + int newChildCount = root->GetChildCount(); + EXPECT_EQ(oldChildCount + 1, newChildCount); + // Verify the model. + const std::wstring expected(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] " + "2f3b ] O3f:[ O3f1b O3f2f ] 3b 4b "); + actual = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(expected, actual); + oldChildCount = newChildCount; + + // Now do it over a folder button. + targetButton = [bar_ buttonWithTitleEqualTo:@"2f"]; + ASSERT_TRUE(targetButton); + NSPoint targetPoint = [targetButton center]; + newNode = other->GetChild(2); // Should be O4f. + EXPECT_EQ(newNode->GetTitle(), L"O4f"); + [bar_ setDragDataNode:newNode]; + [dragInfo setDropLocation:targetPoint]; + [bar_ dragBookmarkData:(id<NSDraggingInfo>)dragInfo.get()]; + + newChildCount = root->GetChildCount(); + EXPECT_EQ(oldChildCount, newChildCount); + // Verify the model. + const std::wstring expected1(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] " + "2f3b O4f:[ O4f1b O4f2f ] ] O3f:[ O3f1b O3f2f ] " + "3b 4b "); + actual = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(expected1, actual); +} + +TEST_F(BookmarkBarControllerDragDropTest, AddURLs) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] " + "2f3b ] 3b 4b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actual = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actual); + + // Remember the children. + int oldChildCount = root->GetChildCount(); + + BookmarkButton* targetButton = [bar_ buttonWithTitleEqualTo:@"3b"]; + ASSERT_TRUE(targetButton); + + NSArray* urls = [NSArray arrayWithObjects: @"http://www.a.com/", + @"http://www.b.com/", nil]; + NSArray* titles = [NSArray arrayWithObjects: @"SiteA", @"SiteB", nil]; + [bar_ addURLs:urls withTitles:titles at:[targetButton center]]; + + // There should two more nodes in the bar. + int newChildCount = root->GetChildCount(); + EXPECT_EQ(oldChildCount + 2, newChildCount); + // Verify the model. + const std::wstring expected(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] " + "2f3b ] SiteA SiteB 3b 4b "); + actual = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(expected, actual); +} + TEST_F(BookmarkBarControllerDragDropTest, ControllerForNode) { BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); const BookmarkNode* root = model.GetBookmarkBarNode(); @@ -1696,4 +1875,36 @@ TEST_F(BookmarkBarControllerDragDropTest, ControllerForNode) { EXPECT_EQ(expectedController, actualController); } +TEST_F(BookmarkBarControllerDragDropTest, DropPositionIndicator) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2b 2f3b ] 3b 4b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModel = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModel); + + // Test a series of points starting at the right edge of the bar. + BookmarkButton* targetButton = [bar_ buttonWithTitleEqualTo:@"1b"]; + ASSERT_TRUE(targetButton); + NSPoint targetPoint = [targetButton left]; + const CGFloat xDelta = 0.5 * bookmarks::kBookmarkHorizontalPadding; + const CGFloat baseOffset = targetPoint.x; + CGFloat expected = xDelta; + CGFloat actual = [bar_ indicatorPosForDragToPoint:targetPoint]; + EXPECT_CGFLOAT_EQ(expected, actual); + targetButton = [bar_ buttonWithTitleEqualTo:@"2f"]; + actual = [bar_ indicatorPosForDragToPoint:[targetButton right]]; + targetButton = [bar_ buttonWithTitleEqualTo:@"3b"]; + expected = [targetButton left].x - baseOffset + xDelta; + EXPECT_CGFLOAT_EQ(expected, actual); + targetButton = [bar_ buttonWithTitleEqualTo:@"4b"]; + targetPoint = [targetButton right]; + targetPoint.x += 100; // Somewhere off to the right. + expected = NSMaxX([targetButton frame]) + xDelta; + actual = [bar_ indicatorPosForDragToPoint:targetPoint]; + EXPECT_CGFLOAT_EQ(expected, actual); +} + } // namespace diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm index bc9a75a..ddb79c6 100644 --- a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm +++ b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm @@ -63,6 +63,15 @@ const CGFloat kScrollWindowVerticalMargin = 0.0; // Show or hide the scroll arrows at the top/bottom of the window. - (void)showOrHideScrollArrows; +// |point| is in the base coordinate system of the destination window; +// it comes from an id<NSDraggingInfo>. |copy| is YES if a copy is to be +// made and inserted into the new location while leaving the bookmark in +// the old location, otherwise move the bookmark by removing from its old +// location and inserting into the new location. +- (BOOL)dragBookmark:(const BookmarkNode*)sourceNode + to:(NSPoint)point + copy:(BOOL)copy; + @end @interface BookmarkButton (BookmarkBarFolderMenuHighlighting) @@ -942,10 +951,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { // difference is accomodation for the "empty" button (which may not // exist in the future). // http://crbug.com/35966 -- (int)indexForDragOfButton:(BookmarkButton*)sourceButton - toPoint:(NSPoint)point { - DCHECK([sourceButton isKindOfClass:[BookmarkButton class]]); - +- (int)indexForDragToPoint:(NSPoint)point { // Identify which buttons we are between. For now, assume a button // location is at the center point of its view, and that an exact // match means "place before". @@ -986,6 +992,52 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { [[parentButton_ cell] startingChildIndex]); } +// More code which essentially duplicates that of BookmarkBarController. +// TODO(mrossetti,jrg): http://crbug.com/35966 +- (BOOL)addURLs:(NSArray*)urls withTitles:(NSArray*)titles at:(NSPoint)point { + DCHECK([urls count] == [titles count]); + BOOL nodesWereAdded = NO; + // Figure out where these new bookmarks nodes are to be added. + BookmarkButton* button = [self buttonForDroppingOnAtPoint:point]; + BookmarkModel* bookmarkModel = [self bookmarkModel]; + const BookmarkNode* destParent = NULL; + int destIndex = 0; + if ([button isFolder]) { + destParent = [button bookmarkNode]; + // Drop it at the end. + destIndex = [button bookmarkNode]->GetChildCount(); + } else { + // Else we're dropping somewhere in the folder, so find the right spot. + destParent = [parentButton_ bookmarkNode]; + destIndex = [self indexForDragToPoint:point]; + // Be careful if the number of buttons != number of nodes. + destIndex += [[parentButton_ cell] startingChildIndex]; + } + + // Create and add the new bookmark nodes. + size_t urlCount = [urls count]; + for (size_t i = 0; i < urlCount; ++i) { + // URLs come in several forms (see NSPasteboard+Utils.mm). + GURL gurl; + const char* string = [[urls objectAtIndex:i] UTF8String]; + if (string) + gurl = GURL(string); + if (!gurl.is_valid() && string) { + gurl = GURL([[NSString stringWithFormat:@"file://%s", string] + UTF8String]); + } + DCHECK(gurl.is_valid()); + if (gurl.is_valid()) { + bookmarkModel->AddURL(destParent, + destIndex++, + base::SysNSStringToWide([titles objectAtIndex:i]), + gurl); + nodesWereAdded = YES; + } + } + return nodesWereAdded; +} + - (BookmarkModel*)bookmarkModel { return [barController_ bookmarkModel]; } @@ -996,8 +1048,15 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { to:(NSPoint)point copy:(BOOL)copy { DCHECK([sourceButton isKindOfClass:[BookmarkButton class]]); - const BookmarkNode* sourceNode = [sourceButton bookmarkNode]; + return [self dragBookmark:sourceNode to:point copy:copy]; +} + +// TODO(jrg): Yet more code dup. +// http://crbug.com/35966 +- (BOOL)dragBookmark:(const BookmarkNode*)sourceNode + to:(NSPoint)point + copy:(BOOL)copy { DCHECK(sourceNode); // Drop destination. @@ -1014,7 +1073,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { } else { // Else we're dropping somewhere in the folder, so find the right spot. destParent = [parentButton_ bookmarkNode]; - destIndex = [self indexForDragOfButton:sourceButton toPoint:point]; + destIndex = [self indexForDragToPoint:point]; // Be careful if the number of buttons != number of nodes. destIndex += [[parentButton_ cell] startingChildIndex]; } @@ -1034,6 +1093,37 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { return wasCopiedOrMoved; } +// TODO(mrossetti,jrg): Identical to the same function in BookmarkBarController. +// http://crbug.com/35966 +- (std::vector<const BookmarkNode*>)retrieveBookmarkDragDataNodes { + std::vector<const BookmarkNode*> dragDataNodes; + BookmarkDragData dragData; + if(dragData.ReadFromDragClipboard()) { + BookmarkModel* bookmarkModel = [self bookmarkModel]; + Profile* profile = bookmarkModel->profile(); + std::vector<const BookmarkNode*> nodes(dragData.GetNodes(profile)); + dragDataNodes.assign(nodes.begin(), nodes.end()); + } + return dragDataNodes; +} + +// TODO(mrossetti,jrg): Identical to the same function in BookmarkBarController. +// http://crbug.com/35966 +- (BOOL)dragBookmarkData:(id<NSDraggingInfo>)info { + BOOL dragged = NO; + std::vector<const BookmarkNode*> nodes([self retrieveBookmarkDragDataNodes]); + if (nodes.size()) { + BOOL copy = !([info draggingSourceOperationMask] & NSDragOperationMove); + NSPoint dropPoint = [info draggingLocation]; + for (std::vector<const BookmarkNode*>::const_iterator it = nodes.begin(); + it != nodes.end(); ++it) { + const BookmarkNode* sourceNode = *it; + dragged = [self dragBookmark:sourceNode to:dropPoint copy:copy]; + } + } + return dragged; +} + // Return YES if we should show the drop indicator, else NO. // TODO(jrg): ARGH code dup! // http://crbug.com/35966 @@ -1047,10 +1137,9 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { // TODO(jrg): again we have code dup, sort of, with // bookmark_bar_controller.mm, but the axis is changed. // http://crbug.com/35966 -- (CGFloat)indicatorPosForDragOfButton:(BookmarkButton*)sourceButton - toPoint:(NSPoint)point { +- (CGFloat)indicatorPosForDragToPoint:(NSPoint)point { CGFloat y = 0; - int destIndex = [self indexForDragOfButton:sourceButton toPoint:point]; + int destIndex = [self indexForDragToPoint:point]; int numButtons = static_cast<int>([buttons_ count]); // If it's a drop strictly between existing buttons or at the very beginning @@ -1060,9 +1149,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { [buttons_ objectAtIndex:static_cast<NSUInteger>(destIndex)]; DCHECK(button); NSRect buttonFrame = [button frame]; - y = buttonFrame.origin.y + - buttonFrame.size.height + - 0.5 * bookmarks::kBookmarkVerticalPadding; + y = NSMaxY(buttonFrame) + 0.5 * bookmarks::kBookmarkVerticalPadding; // If it's a drop at the end (past the last button, if there are any) ... } else if (destIndex == numButtons) { diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm index ed03b6f..cb47d82 100644 --- a/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm @@ -18,29 +18,6 @@ #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" -#if CGFLOAT_IS_DOUBLE -#define CGFLOAT_EPSILON DBL_EPSILON -#else -#define CGFLOAT_EPSILON FLT_EPSILON -#endif - -#define CGFLOAT_EQ(expected, actual) \ - (actual >= (expected - CGFLOAT_EPSILON) && \ - actual <= (expected + CGFLOAT_EPSILON)) - -#define EXPECT_CGFLOAT_EQ(expected, actual) \ - EXPECT_TRUE(CGFLOAT_EQ(actual, expected)) - -#define EXPECT_NSRECT_EQ(expected, actual) \ - EXPECT_TRUE(CGFLOAT_EQ(expected.origin.x, actual.origin.x) && \ - CGFLOAT_EQ(expected.origin.y, actual.origin.y) && \ - CGFLOAT_EQ(expected.size.width, actual.size.width) && \ - CGFLOAT_EQ(expected.size.height, actual.size.height)) << \ - "Rects do not match: " << \ - [NSStringFromRect(expected) UTF8String] << \ - " != " << [NSStringFromRect(actual) UTF8String] - - // Add a redirect to make testing easier. @interface BookmarkBarFolderController(MakeTestingEasier) - (IBAction)openBookmarkFolderFromButton:(id)sender; @@ -255,7 +232,8 @@ TEST_F(BookmarkBarFolderControllerTest, Position) { [bbfc2 setRealTopLeft:YES]; pt = [bbfc2 windowTopLeft]; // We're now overlapping the window a bit. - EXPECT_EQ(pt.x, NSMaxX([[bbfc.get() window] frame]) - 1); + EXPECT_EQ(pt.x, NSMaxX([[bbfc.get() window] frame]) - + bookmarks::kBookmarkMenuOverlap); } TEST_F(BookmarkBarFolderControllerTest, DropDestination) { @@ -414,6 +392,54 @@ TEST_F(BookmarkBarFolderControllerTest, SimpleScroll) { } } +@interface FakedDragInfo : NSObject { +@public + NSPoint dropLocation_; + NSDragOperation sourceMask_; +} +@property (assign) NSPoint dropLocation; +- (void)setDraggingSourceOperationMask:(NSDragOperation)mask; +@end + +@implementation FakedDragInfo + +@synthesize dropLocation = dropLocation_; + +- (id)init { + if ((self = [super init])) { + dropLocation_ = NSZeroPoint; + sourceMask_ = NSDragOperationMove; + } + return self; +} + +// NSDraggingInfo protocol functions. + +- (id)draggingPasteboard { + return self; +} + +- (id)draggingSource { + return self; +} + +- (NSDragOperation)draggingSourceOperationMask { + return sourceMask_; +} + +- (NSPoint)draggingLocation { + return dropLocation_; +} + +// Other functions. + +- (void)setDraggingSourceOperationMask:(NSDragOperation)mask { + sourceMask_ = mask; +} + +@end + + class BookmarkBarFolderControllerMenuTest : public CocoaTest { public: BrowserTestHelper helper_; @@ -997,6 +1023,175 @@ TEST_F(BookmarkBarFolderControllerMenuTest, MenuSizingAndScrollArrows) { EXPECT_GT(buttonWidth, NSWidth([button frame])); } +// Just like a BookmarkBarFolderController but intercedes when providing +// pasteboard drag data. +@interface BookmarkBarFolderControllerDragData : BookmarkBarFolderController { + const BookmarkNode* dragDataNode_; // Weak +} +- (void)setDragDataNode:(const BookmarkNode*)node; +@end + +@implementation BookmarkBarFolderControllerDragData + +- (id)initWithParentButton:(BookmarkButton*)button + parentController:(BookmarkBarFolderController*)parentController + barController:(BookmarkBarController*)barController { + if ((self = [super initWithParentButton:button + parentController:parentController + barController:barController])) { + dragDataNode_ = NULL; + } + return self; +} + +- (void)setDragDataNode:(const BookmarkNode*)node { + dragDataNode_ = node; +} + +- (std::vector<const BookmarkNode*>)retrieveBookmarkDragDataNodes { + std::vector<const BookmarkNode*> dragDataNodes; + if(dragDataNode_) { + dragDataNodes.push_back(dragDataNode_); + } + return dragDataNodes; +} + +@end + +TEST_F(BookmarkBarFolderControllerMenuTest, DragBookmarkData) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] " + "2f3b ] 3b 4b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + const BookmarkNode* other = model.other_node(); + const std::wstring other_string(L"O1b O2b O3f:[ O3f1b O3f2f ] " + "O4f:[ O4f1b O4f2f ] 05b "); + model_test_utils::AddNodesFromModelString(model, other, other_string); + + // Validate initial model. + std::wstring actual = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actual); + actual = model_test_utils::ModelStringFromNode(other); + EXPECT_EQ(other_string, actual); + + // Pop open a folder. + BookmarkButton* button = [bar_ buttonWithTitleEqualTo:@"2f"]; + scoped_nsobject<BookmarkBarFolderControllerDragData> folderController; + folderController.reset([[BookmarkBarFolderControllerDragData alloc] + initWithParentButton:button + parentController:nil + barController:bar_]); + [folderController window]; + BookmarkButton* targetButton = + [folderController buttonWithTitleEqualTo:@"2f1b"]; + ASSERT_TRUE(targetButton); + + // Gen up some dragging data. + const BookmarkNode* newNode = other->GetChild(2); + [folderController setDragDataNode:newNode]; + scoped_nsobject<FakedDragInfo> dragInfo([[FakedDragInfo alloc] init]); + [dragInfo setDropLocation:[targetButton top]]; + [folderController dragBookmarkData:(id<NSDraggingInfo>)dragInfo.get()]; + + // Verify the model. + const std::wstring expected(L"1b 2f:[ O3f:[ O3f1b O3f2f ] 2f1b 2f2f:[ 2f2f1b " + "2f2f2b 2f2f3b ] 2f3b ] 3b 4b "); + actual = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(expected, actual); + + // Now drag over a folder button. + targetButton = [folderController buttonWithTitleEqualTo:@"2f2f"]; + ASSERT_TRUE(targetButton); + newNode = other->GetChild(2); // Should be O4f. + EXPECT_EQ(newNode->GetTitle(), L"O4f"); + [folderController setDragDataNode:newNode]; + [dragInfo setDropLocation:[targetButton center]]; + [folderController dragBookmarkData:(id<NSDraggingInfo>)dragInfo.get()]; + + // Verify the model. + const std::wstring expectedA(L"1b 2f:[ O3f:[ O3f1b O3f2f ] 2f1b 2f2f:[ " + "2f2f1b 2f2f2b 2f2f3b O4f:[ O4f1b O4f2f ] ] " + "2f3b ] 3b 4b "); + actual = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(expectedA, actual); +} + +TEST_F(BookmarkBarFolderControllerMenuTest, AddURLs) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] " + "2f3b ] 3b 4b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actual = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actual); + + // Pop open a folder. + BookmarkButton* button = [bar_ buttonWithTitleEqualTo:@"2f"]; + [[button target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:button]; + BookmarkBarFolderController* folderController = [bar_ folderController]; + EXPECT_TRUE(folderController); + NSArray* buttons = [folderController buttons]; + EXPECT_TRUE(buttons); + + // Remember how many buttons are showing. + int oldDisplayedButtons = [buttons count]; + + BookmarkButton* targetButton = + [folderController buttonWithTitleEqualTo:@"2f1b"]; + ASSERT_TRUE(targetButton); + + NSArray* urls = [NSArray arrayWithObjects: @"http://www.a.com/", + @"http://www.b.com/", nil]; + NSArray* titles = [NSArray arrayWithObjects: @"SiteA", @"SiteB", nil]; + [folderController addURLs:urls withTitles:titles at:[targetButton top]]; + + // There should two more buttons in the folder. + int newDisplayedButtons = [buttons count]; + EXPECT_EQ(oldDisplayedButtons + 2, newDisplayedButtons); + // Verify the model. + const std::wstring expected(L"1b 2f:[ SiteA SiteB 2f1b 2f2f:[ 2f2f1b 2f2f2b " + "2f2f3b ] 2f3b ] 3b 4b "); + actual = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(expected, actual); +} + +TEST_F(BookmarkBarFolderControllerMenuTest, DropPositionIndicator) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] " + "2f3b ] 3b 4b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actual = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actual); + + // Pop open the folder. + BookmarkButton* button = [bar_ buttonWithTitleEqualTo:@"2f"]; + [[button target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:button]; + BookmarkBarFolderController* folder = [bar_ folderController]; + EXPECT_TRUE(folder); + + // Test a series of points starting at the top of the folder. + const CGFloat yOffset = 0.5 * bookmarks::kBookmarkVerticalPadding; + BookmarkButton* targetButton = [folder buttonWithTitleEqualTo:@"2f1b"]; + ASSERT_TRUE(targetButton); + NSPoint targetPoint = [targetButton top]; + CGFloat pos = [folder indicatorPosForDragToPoint:targetPoint]; + EXPECT_CGFLOAT_EQ(targetPoint.y + yOffset, pos); + pos = [folder indicatorPosForDragToPoint:[targetButton bottom]]; + targetButton = [folder buttonWithTitleEqualTo:@"2f2f"]; + EXPECT_CGFLOAT_EQ([targetButton top].y + yOffset, pos); + pos = [folder indicatorPosForDragToPoint:NSMakePoint(10,0)]; + targetButton = [folder buttonWithTitleEqualTo:@"2f3b"]; + EXPECT_CGFLOAT_EQ([targetButton bottom].y - yOffset, pos); +} + @interface BookmarkBarControllerNoDelete : BookmarkBarController - (IBAction)deleteBookmark:(id)sender; @end diff --git a/chrome/browser/cocoa/bookmark_bar_folder_view.h b/chrome/browser/cocoa/bookmark_bar_folder_view.h index e4142d6..5b48fcf 100644 --- a/chrome/browser/cocoa/bookmark_bar_folder_view.h +++ b/chrome/browser/cocoa/bookmark_bar_folder_view.h @@ -5,6 +5,7 @@ #import <Cocoa/Cocoa.h> @protocol BookmarkButtonControllerProtocol; +@class BookmarkBarFolderController; // Main content view for a bookmark bar folder "menu" window. This is // logically similar to a BookmarkBarView but is oriented vertically. @@ -13,12 +14,17 @@ BOOL inDrag_; // Are we in the middle of a drag? BOOL dropIndicatorShown_; CGFloat dropIndicatorPosition_; // y position + // The following |controller_| is weak; used for testing only. See the imple- + // mentation comment for - (id<BookmarkButtonControllerProtocol>)controller. + BookmarkBarFolderController* controller_; } // Return the controller that owns this view. - (id<BookmarkButtonControllerProtocol>)controller; @end -@interface BookmarkBarFolderView(TestingAPI) -- (void)setDropIndicatorShown:(BOOL)shown; +@interface BookmarkBarFolderView() // TestingOrInternalAPI +@property (assign) BOOL dropIndicatorShown; +@property (readonly) CGFloat dropIndicatorPosition; +- (void)setController:(id)controller; @end diff --git a/chrome/browser/cocoa/bookmark_bar_folder_view.mm b/chrome/browser/cocoa/bookmark_bar_folder_view.mm index 15a4db6..f3e1791 100644 --- a/chrome/browser/cocoa/bookmark_bar_folder_view.mm +++ b/chrome/browser/cocoa/bookmark_bar_folder_view.mm @@ -3,16 +3,24 @@ // found in the LICENSE file. #import "chrome/browser/cocoa/bookmark_bar_folder_view.h" +#include "chrome/browser/bookmarks/bookmark_pasteboard_helper_mac.h" #import "chrome/browser/cocoa/bookmark_bar_controller.h" +#import "chrome/browser/cocoa/bookmark_folder_target.h" +#import "third_party/mozilla/NSPasteboard+Utils.h" @implementation BookmarkBarFolderView -- (id<BookmarkButtonControllerProtocol>)controller { - return [[self window] windowController]; -} +@synthesize dropIndicatorShown = dropIndicatorShown_; +@synthesize dropIndicatorPosition = dropIndicatorPosition_; - (void)awakeFromNib { - NSArray* types = [NSArray arrayWithObject:kBookmarkButtonDragType]; + NSArray* types = [NSArray arrayWithObjects: + NSStringPboardType, + NSHTMLPboardType, + NSURLPboardType, + kBookmarkButtonDragType, + kBookmarkDictionaryListPboardType, + nil]; [self registerForDraggedTypes:types]; } @@ -21,6 +29,16 @@ [super dealloc]; } +- (id<BookmarkButtonControllerProtocol>)controller { + // When needed for testing, set the local data member |controller_| to + // the test controller. + return controller_ ? controller_ : [[self window] windowController]; +} + +- (void)setController:(id)controller { + controller_ = controller; +} + - (void)drawRect:(NSRect)rect { // TODO(jrg): copied from bookmark_bar_view but orientation changed. // Code dup sucks but I'm not sure I can take 16 lines and make it @@ -45,23 +63,25 @@ } } +// TODO(mrossetti,jrg): Identical to -[BookmarkBarView +// dragClipboardContainsBookmarks]. http://crbug.com/35966 +// Shim function to assist in unit testing. +- (BOOL)dragClipboardContainsBookmarks { + return bookmark_pasteboard_helper_mac::DragClipboardContainsBookmarks(); +} + // Virtually identical to [BookmarkBarView draggingEntered:]. // TODO(jrg): find a way to share code. Lack of multiple inheritance // makes things more of a pain but there should be no excuse for laziness. // http://crbug.com/35966 - (NSDragOperation)draggingEntered:(id<NSDraggingInfo>)info { inDrag_ = YES; - - NSData* data = [[info draggingPasteboard] - dataForType:kBookmarkButtonDragType]; - // [info draggingSource] is nil if not the same application. - if (data && [info draggingSource]) { + if ([[info draggingPasteboard] dataForType:kBookmarkButtonDragType] || + [self dragClipboardContainsBookmarks] || + [[info draggingPasteboard] containsURLData]) { // Find the position of the drop indicator. - BookmarkButton* button = nil; - [data getBytes:&button length:sizeof(button)]; - BOOL showIt = [[self controller] - shouldShowIndicatorShownForPoint:[info draggingLocation]]; + shouldShowIndicatorShownForPoint:[info draggingLocation]]; if (!showIt) { if (dropIndicatorShown_) { dropIndicatorShown_ = NO; @@ -69,9 +89,8 @@ } } else { CGFloat y = - [[self controller] - indicatorPosForDragOfButton:button - toPoint:[info draggingLocation]]; + [[self controller] + indicatorPosForDragToPoint:[info draggingLocation]]; // Need an update if the indicator wasn't previously shown or if it has // moved. @@ -83,9 +102,8 @@ } [[self controller] draggingEntered:info]; // allow hover-open to work - return NSDragOperationMove; + return [info draggingSource] ? NSDragOperationMove : NSDragOperationCopy; } - return NSDragOperationNone; } @@ -129,9 +147,30 @@ return YES; } +// This code is practically identical to the same function in BookmarkBarView +// with the only difference being how the controller is retrieved. +// TODO(mrossetti,jrg): http://crbug.com/35966 // Implement NSDraggingDestination protocol method -// performDragOperation: for bookmarks. -- (BOOL)performDragOperationForBookmark:(id<NSDraggingInfo>)info { +// performDragOperation: for URLs. +- (BOOL)performDragOperationForURL:(id<NSDraggingInfo>)info { + NSPasteboard* pboard = [info draggingPasteboard]; + DCHECK([pboard containsURLData]); + + NSArray* urls = nil; + NSArray* titles = nil; + [pboard getURLs:&urls andTitles:&titles]; + + return [[self controller] addURLs:urls + withTitles:titles + at:[info draggingLocation]]; +} + +// This code is practically identical to the same function in BookmarkBarView +// with the only difference being how the controller is retrieved. +// http://crbug.com/35966 +// Implement NSDraggingDestination protocol method +// performDragOperation: for bookmark buttons. +- (BOOL)performDragOperationForBookmarkButton:(id<NSDraggingInfo>)info { BOOL doDrag = NO; NSData* data = [[info draggingPasteboard] dataForType:kBookmarkButtonDragType]; @@ -148,22 +187,15 @@ } - (BOOL)performDragOperation:(id<NSDraggingInfo>)info { + if ([[self controller] dragBookmarkData:info]) + return YES; NSPasteboard* pboard = [info draggingPasteboard]; - if ([pboard dataForType:kBookmarkButtonDragType]) { - if ([self performDragOperationForBookmark:info]) - return YES; - // Fall through.... - } + if ([pboard dataForType:kBookmarkButtonDragType] && + [self performDragOperationForBookmarkButton:info]) + return YES; + if ([pboard containsURLData] && [self performDragOperationForURL:info]) + return YES; return NO; } -@end // BookmarkBarFolderView - - -@implementation BookmarkBarFolderView(TestingAPI) - -- (void)setDropIndicatorShown:(BOOL)shown { - dropIndicatorShown_ = shown; -} - @end diff --git a/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm b/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm index 83472af..a1e2b1e 100644 --- a/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm @@ -6,135 +6,144 @@ #import "chrome/browser/cocoa/bookmark_bar_controller.h" #import "chrome/browser/cocoa/bookmark_bar_folder_view.h" #import "chrome/browser/cocoa/bookmark_button.h" +#import "chrome/browser/cocoa/bookmark_folder_target.h" #import "chrome/browser/cocoa/cocoa_test_helper.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" +#import "third_party/mozilla/NSPasteboard+Utils.h" -// Fake NSDraggingInfo for testing -@interface FakeDraggingInfo : NSObject +namespace { + const CGFloat kFakeIndicatorPos = 7.0; +}; + +// Fake DraggingInfo, fake BookmarkBarController, fake NSPasteboard... +@interface FakeDraggingInfo : NSObject { + @public + BOOL dragButtonToPong_; + BOOL dragURLsPong_; + BOOL dragBookmarkDataPong_; + BOOL dropIndicatorShown_; + BOOL draggingEnteredCalled_; + // Only mock one type of drag data at a time. + NSString* dragDataType_; +} +@property (readwrite) BOOL dropIndicatorShown; +@property (readwrite) BOOL draggingEnteredCalled; +@property (copy) NSString* dragDataType; @end @implementation FakeDraggingInfo -- (id)draggingPasteboard { +@synthesize dropIndicatorShown = dropIndicatorShown_; +@synthesize draggingEnteredCalled = draggingEnteredCalled_; +@synthesize dragDataType = dragDataType_; + +- (id)init { + if ((self = [super init])) { + dropIndicatorShown_ = YES; + } return self; } -- (NSData*)dataForType:(NSString*)type { - if ([type isEqual:kBookmarkButtonDragType]) - return [NSData dataWithBytes:&self length:sizeof(self)]; - return nil; +- (void)dealloc { + [dragDataType_ release]; + [super dealloc]; } -- (BOOL)draggingSource { - return YES; // pretend we're local +- (void)reset { + [dragDataType_ release]; + dragDataType_ = nil; + dragButtonToPong_ = NO; + dragURLsPong_ = NO; + dragBookmarkDataPong_ = NO; + dropIndicatorShown_ = YES; + draggingEnteredCalled_ = NO; } -@end - +// NSDragInfo mocking functions. -// We are our own controller for test convenience. -@interface BookmarkBarFolderViewFakeController : - BookmarkBarFolderView<BookmarkButtonControllerProtocol> { - @public - BOOL closedAll_; - BOOL controllerEntered_; - BOOL controllerExited_; -} -@end - -@implementation BookmarkBarFolderViewFakeController - -- (id<BookmarkButtonControllerProtocol>)controller { +- (id)draggingPasteboard { return self; } -- (BOOL)shouldShowIndicatorShownForPoint:(NSPoint)pt { - return YES; +// So we can look local. +- (id)draggingSource { + return self; } -- (CGFloat)indicatorPosForDragOfButton:(BookmarkButton*)button - toPoint:(NSPoint)point { - return 101.0; // Arbitrary value. +- (NSDragOperation)draggingSourceOperationMask { + return NSDragOperationCopy | NSDragOperationMove; } -- (NSDragOperation)draggingEntered:(id<NSDraggingInfo>)info { - controllerEntered_ = YES; - return NSDragOperationMove; +- (NSPoint)draggingLocation { + return NSMakePoint(10, 10); } -- (void)draggingExited:(id<NSDraggingInfo>)info { - controllerExited_ = YES; -} +// NSPasteboard mocking functions. -- (BOOL)dragShouldLockBarVisibility { +- (BOOL)containsURLData { + NSArray* urlTypes = [NSArray arrayWithObjects: + kWebURLsWithTitlesPboardType, + NSURLPboardType, + NSFilenamesPboardType, + NSStringPboardType, + nil]; + if (dragDataType_) + return [urlTypes containsObject:dragDataType_]; return NO; } -- (void)closeAllBookmarkFolders { - closedAll_ = YES; -} - -- (BOOL)dragButton:(BookmarkButton*)sourceButton - to:(NSPoint)point - copy:(BOOL)copy { - return NO; -} - -- (BookmarkModel*)bookmarkModel { - NOTREACHED(); - return NULL; -} - -- (void)closeBookmarkFolder:(id)sender { -} - -- (NSWindow*)parentWindow { +- (NSData*)dataForType:(NSString*)type { + if (dragDataType_ && [dragDataType_ isEqualToString:type]) + return [NSData data]; // Return something, anything. return nil; } -- (ThemeProvider*)themeProvider { - return nil; -} +// Fake a controller for callback ponging -- (void)childFolderWillShow:(id<BookmarkButtonControllerProtocol>)child { +- (BOOL)dragButton:(BookmarkButton*)button to:(NSPoint)point copy:(BOOL)copy { + dragButtonToPong_ = YES; + return YES; } -- (void)childFolderWillClose:(id<BookmarkButtonControllerProtocol>)child { +- (BOOL)addURLs:(NSArray*)urls withTitles:(NSArray*)titles at:(NSPoint)point { + dragURLsPong_ = YES; + return YES; } -- (void)addNewFolderControllerWithParentButton:(BookmarkButton*)parentButton { +- (void)getURLs:(NSArray**)outUrls andTitles:(NSArray**)outTitles { } -- (BookmarkBarFolderController*)folderController { - return nil; +- (BOOL)dragBookmarkData:(id<NSDraggingInfo>)info { + dragBookmarkDataPong_ = YES; + return NO; } -- (NSImage*)favIconForNode:(const BookmarkNode*)node { - return nil; -} +// Confirm the pongs. -- (NSMenu*)contextMenuForNode:(const BookmarkNode*)node { - return nil; +- (BOOL)dragButtonToPong { + return dragButtonToPong_; } -- (void)openAll:(const BookmarkNode*)node - disposition:(WindowOpenDisposition)disposition { +- (BOOL)dragURLsPong { + return dragURLsPong_; } -- (void)removeButton:(NSInteger)buttonIndex animate:(BOOL)animate { +- (BOOL)dragBookmarkDataPong { + return dragBookmarkDataPong_; } -- (id<BookmarkButtonControllerProtocol>)controllerForNode: - (const BookmarkNode*)node { - return nil; +- (CGFloat)indicatorPosForDragToPoint:(NSPoint)point { + return kFakeIndicatorPos; } -- (void)moveButtonFromIndex:(NSInteger)fromIndex toIndex:(NSInteger)toIndex { +- (BOOL)shouldShowIndicatorShownForPoint:(NSPoint)point { + return dropIndicatorShown_; } -- (void)addButtonForNode:(const BookmarkNode*)node - atIndex:(NSInteger)buttonIndex { +- (NSDragOperation)draggingEntered:(id<NSDraggingInfo>)info { + draggingEnteredCalled_ = YES; + return NSDragOperationNone; } @end @@ -145,38 +154,80 @@ class BookmarkBarFolderViewTest : public CocoaTest { public: virtual void SetUp() { CocoaTest::SetUp(); - view_.reset([[BookmarkBarFolderViewFakeController alloc] init]); + view_.reset([[BookmarkBarFolderView alloc] init]); } - scoped_nsobject<BookmarkBarFolderViewFakeController> view_; + scoped_nsobject<BookmarkBarFolderView> view_; }; -TEST_F(BookmarkBarFolderViewTest, Basics) { +TEST_F(BookmarkBarFolderViewTest, BookmarkButtonDragAndDrop) { [view_ awakeFromNib]; - [[test_window() contentView] addSubview:view_]; - - // Make sure we're set up for DnD - NSArray* types = [view_ registeredDraggedTypes]; - EXPECT_TRUE([types containsObject:kBookmarkButtonDragType]); - - // This doesn't confirm results but it makes sure we don't crash or leak. - [view_ drawRect:NSMakeRect(0,0,10,10)]; - [view_ setDropIndicatorShown:YES]; - [view_ drawRect:NSMakeRect(0,0,10,10)]; + scoped_nsobject<FakeDraggingInfo> info([[FakeDraggingInfo alloc] init]); + [view_ setController:info.get()]; + [info reset]; - [view_ removeFromSuperview]; + [info setDragDataType:kBookmarkButtonDragType]; + EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); + EXPECT_TRUE([view_ performDragOperation:(id)info.get()]); + EXPECT_TRUE([info dragButtonToPong]); + EXPECT_FALSE([info dragURLsPong]); + EXPECT_TRUE([info dragBookmarkDataPong]); } -TEST_F(BookmarkBarFolderViewTest, SimpleDragEnterExit) { +TEST_F(BookmarkBarFolderViewTest, URLDragAndDrop) { [view_ awakeFromNib]; scoped_nsobject<FakeDraggingInfo> info([[FakeDraggingInfo alloc] init]); - - [view_ draggingEntered:(id<NSDraggingInfo>)info.get()]; - // Confirms we got a chance to hover-open. - EXPECT_TRUE(view_.get()->controllerEntered_); - - [view_ draggingEnded:(id<NSDraggingInfo>)info.get()]; - EXPECT_TRUE(view_.get()->controllerExited_); + [view_ setController:info.get()]; + [info reset]; + + [info setDragDataType:kWebURLsWithTitlesPboardType]; + EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); + EXPECT_TRUE([view_ performDragOperation:(id)info.get()]); + EXPECT_FALSE([info dragButtonToPong]); + EXPECT_TRUE([info dragURLsPong]); + EXPECT_TRUE([info dragBookmarkDataPong]); + [info reset]; + + [info setDragDataType:NSURLPboardType]; + EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); + EXPECT_TRUE([view_ performDragOperation:(id)info.get()]); + EXPECT_FALSE([info dragButtonToPong]); + EXPECT_TRUE([info dragURLsPong]); + EXPECT_TRUE([info dragBookmarkDataPong]); + [info reset]; + + [info setDragDataType:NSFilenamesPboardType]; + EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); + EXPECT_TRUE([view_ performDragOperation:(id)info.get()]); + EXPECT_FALSE([info dragButtonToPong]); + EXPECT_TRUE([info dragURLsPong]); + EXPECT_TRUE([info dragBookmarkDataPong]); + [info reset]; + + [info setDragDataType:NSStringPboardType]; + EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); + EXPECT_TRUE([view_ performDragOperation:(id)info.get()]); + EXPECT_FALSE([info dragButtonToPong]); + EXPECT_TRUE([info dragURLsPong]); + EXPECT_TRUE([info dragBookmarkDataPong]); +} + +TEST_F(BookmarkBarFolderViewTest, BookmarkButtonDropIndicator) { + [view_ awakeFromNib]; + scoped_nsobject<FakeDraggingInfo> info([[FakeDraggingInfo alloc] init]); + [view_ setController:info.get()]; + [info reset]; + + [info setDragDataType:kBookmarkButtonDragType]; + EXPECT_FALSE([info draggingEnteredCalled]); + EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); + EXPECT_TRUE([info draggingEnteredCalled]); // Ensure controller pinged. + EXPECT_TRUE([view_ dropIndicatorShown]); + EXPECT_EQ([view_ dropIndicatorPosition], kFakeIndicatorPos); + + [info setDropIndicatorShown:NO]; + EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); + EXPECT_FALSE([view_ dropIndicatorShown]); } } // namespace diff --git a/chrome/browser/cocoa/bookmark_bar_unittest_helper.h b/chrome/browser/cocoa/bookmark_bar_unittest_helper.h index 402f8a1..c122dff 100644 --- a/chrome/browser/cocoa/bookmark_bar_unittest_helper.h +++ b/chrome/browser/cocoa/bookmark_bar_unittest_helper.h @@ -39,10 +39,18 @@ // containing window. - (NSPoint)top; +// Return the bottom of the button in the base coordinate system of the +// containing window. +- (NSPoint)bottom; + // Return the center-left point of the button in the base coordinate system // of the containing window. - (NSPoint)left; +// Return the center-right point of the button in the base coordinate system +// of the containing window. +- (NSPoint)right; + @end #endif // CHROME_BROWSER_COCOA_AUTOCOMPLETE_TEXT_FIELD_UNITTEST_HELPER_H_ diff --git a/chrome/browser/cocoa/bookmark_bar_unittest_helper.mm b/chrome/browser/cocoa/bookmark_bar_unittest_helper.mm index 114197e..5453057 100644 --- a/chrome/browser/cocoa/bookmark_bar_unittest_helper.mm +++ b/chrome/browser/cocoa/bookmark_bar_unittest_helper.mm @@ -57,6 +57,13 @@ return top; } +- (NSPoint)bottom { + NSRect frame = [self frame]; + NSPoint bottom = NSMakePoint(NSMidX(frame), NSMinY(frame)); + bottom = [[self superview] convertPoint:bottom toView:nil]; + return bottom; +} + - (NSPoint)left { NSRect frame = [self frame]; NSPoint left = NSMakePoint(NSMinX(frame), NSMidY(frame)); @@ -64,4 +71,11 @@ return left; } +- (NSPoint)right { + NSRect frame = [self frame]; + NSPoint right = NSMakePoint(NSMaxX(frame), NSMidY(frame)); + right = [[self superview] convertPoint:right toView:nil]; + return right; +} + @end diff --git a/chrome/browser/cocoa/bookmark_bar_view.mm b/chrome/browser/cocoa/bookmark_bar_view.mm index c66348e..f4c9f15 100644 --- a/chrome/browser/cocoa/bookmark_bar_view.mm +++ b/chrome/browser/cocoa/bookmark_bar_view.mm @@ -4,9 +4,11 @@ #import "chrome/browser/cocoa/bookmark_bar_view.h" +#include "chrome/browser/bookmarks/bookmark_pasteboard_helper_mac.h" #import "chrome/browser/browser_theme_provider.h" #import "chrome/browser/cocoa/bookmark_bar_controller.h" #import "chrome/browser/cocoa/bookmark_button.h" +#import "chrome/browser/cocoa/bookmark_folder_target.h" #import "chrome/browser/cocoa/themed_window.h" #import "third_party/mozilla/NSPasteboard+Utils.h" @@ -42,11 +44,12 @@ DCHECK(controller_ && "Expected this to be hooked up via Interface Builder"); NSArray* types = [NSArray arrayWithObjects: - NSStringPboardType, - NSHTMLPboardType, - NSURLPboardType, - kBookmarkButtonDragType, - nil]; + NSStringPboardType, + NSHTMLPboardType, + NSURLPboardType, + kBookmarkButtonDragType, + kBookmarkDictionaryListPboardType, + nil]; [self registerForDraggedTypes:types]; } @@ -119,48 +122,41 @@ } } +// Shim function to assist in unit testing. +- (BOOL)dragClipboardContainsBookmarks { + return bookmark_pasteboard_helper_mac::DragClipboardContainsBookmarks(); +} + // NSDraggingDestination methods - (NSDragOperation)draggingEntered:(id<NSDraggingInfo>)info { - if ([[info draggingPasteboard] dataForType:kBookmarkButtonDragType]) { - NSData* data = [[info draggingPasteboard] - dataForType:kBookmarkButtonDragType]; - // [info draggingSource] is nil if not the same application. - if (data && [info draggingSource]) { - // Find the position of the drop indicator. - BookmarkButton* button = nil; - [data getBytes:&button length:sizeof(button)]; - - // We only show the drop indicator if we're not in a position to - // perform a hover-open since it doesn't make sense to do both. - BOOL showIt = - [controller_ shouldShowIndicatorShownForPoint: - [info draggingLocation]]; - if (!showIt) { - if (dropIndicatorShown_) { - dropIndicatorShown_ = NO; - [self setNeedsDisplay:YES]; - } - } else { - CGFloat x = - [controller_ indicatorPosForDragOfButton:button - toPoint:[info draggingLocation]]; - // Need an update if the indicator wasn't previously shown or if it has - // moved. - if (!dropIndicatorShown_ || dropIndicatorPosition_ != x) { - dropIndicatorShown_ = YES; - dropIndicatorPosition_ = x; - [self setNeedsDisplay:YES]; - } + if ([[info draggingPasteboard] dataForType:kBookmarkButtonDragType] || + [self dragClipboardContainsBookmarks] || + [[info draggingPasteboard] containsURLData]) { + // We only show the drop indicator if we're not in a position to + // perform a hover-open since it doesn't make sense to do both. + BOOL showIt = [controller_ shouldShowIndicatorShownForPoint: + [info draggingLocation]]; + if (!showIt) { + if (dropIndicatorShown_) { + dropIndicatorShown_ = NO; + [self setNeedsDisplay:YES]; + } + } else { + CGFloat x = + [controller_ indicatorPosForDragToPoint:[info draggingLocation]]; + // Need an update if the indicator wasn't previously shown or if it has + // moved. + if (!dropIndicatorShown_ || dropIndicatorPosition_ != x) { + dropIndicatorShown_ = YES; + dropIndicatorPosition_ = x; + [self setNeedsDisplay:YES]; } - - [controller_ draggingEntered:info]; // allow hover-open to work. - return NSDragOperationMove; } - // Fall through otherwise. + + [controller_ draggingEntered:info]; // allow hover-open to work. + return [info draggingSource] ? NSDragOperationMove : NSDragOperationCopy; } - if ([[info draggingPasteboard] containsURLData]) - return NSDragOperationCopy; return NSDragOperationNone; } @@ -212,11 +208,11 @@ } // Implement NSDraggingDestination protocol method -// performDragOperation: for bookmarks. -- (BOOL)performDragOperationForBookmark:(id<NSDraggingInfo>)info { +// performDragOperation: for bookmark buttons. +- (BOOL)performDragOperationForBookmarkButton:(id<NSDraggingInfo>)info { BOOL rtn = NO; NSData* data = [[info draggingPasteboard] - dataForType:kBookmarkButtonDragType]; + dataForType:kBookmarkButtonDragType]; // [info draggingSource] is nil if not the same application. if (data && [info draggingSource]) { BookmarkButton* button = nil; @@ -230,9 +226,11 @@ } - (BOOL)performDragOperation:(id<NSDraggingInfo>)info { + if ([controller_ dragBookmarkData:info]) + return YES; NSPasteboard* pboard = [info draggingPasteboard]; if ([pboard dataForType:kBookmarkButtonDragType]) { - if ([self performDragOperationForBookmark:info]) + if ([self performDragOperationForBookmarkButton:info]) return YES; // Fall through.... } diff --git a/chrome/browser/cocoa/bookmark_bar_view_unittest.mm b/chrome/browser/cocoa/bookmark_bar_view_unittest.mm index f1506d6..88168ff 100644 --- a/chrome/browser/cocoa/bookmark_bar_view_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_view_unittest.mm @@ -6,41 +6,62 @@ #import "chrome/browser/cocoa/bookmark_bar_controller.h" #import "chrome/browser/cocoa/bookmark_bar_view.h" #import "chrome/browser/cocoa/bookmark_button.h" +#import "chrome/browser/cocoa/bookmark_folder_target.h" #import "chrome/browser/cocoa/cocoa_test_helper.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" +#import "third_party/mozilla/NSPasteboard+Utils.h" namespace { -const CGFloat kFakeIndicatorPos = 7.0; + const CGFloat kFakeIndicatorPos = 7.0; }; -// Fake DraggingInfo, fake BookmarkBarController, fake pasteboard... +// Fake DraggingInfo, fake BookmarkBarController, fake NSPasteboard... @interface FakeBookmarkDraggingInfo : NSObject { @public - scoped_nsobject<NSData> data_; - BOOL pong_; + BOOL dragButtonToPong_; + BOOL dragURLsPong_; + BOOL dragBookmarkDataPong_; BOOL dropIndicatorShown_; BOOL draggingEnteredCalled_; + // Only mock one type of drag data at a time. + NSString* dragDataType_; } @property (readwrite) BOOL dropIndicatorShown; @property (readwrite) BOOL draggingEnteredCalled; +@property (copy) NSString* dragDataType; @end @implementation FakeBookmarkDraggingInfo @synthesize dropIndicatorShown = dropIndicatorShown_; @synthesize draggingEnteredCalled = draggingEnteredCalled_; +@synthesize dragDataType = dragDataType_; - (id)init { if ((self = [super init])) { dropIndicatorShown_ = YES; - draggingEnteredCalled_ = NO; - data_.reset([[NSData dataWithBytes:&self length:sizeof(self)] retain]); } return self; } -// So we can be both info and pasteboard. +- (void)dealloc { + [dragDataType_ release]; + [super dealloc]; +} + +- (void)reset { + [dragDataType_ release]; + dragDataType_ = nil; + dragButtonToPong_ = NO; + dragURLsPong_ = NO; + dragBookmarkDataPong_ = NO; + dropIndicatorShown_ = YES; + draggingEnteredCalled_ = NO; +} + +// NSDragInfo mocking functions. + - (id)draggingPasteboard { return self; } @@ -50,33 +71,69 @@ const CGFloat kFakeIndicatorPos = 7.0; return self; } -- (BOOL)containsURLData { - return NO; +- (NSDragOperation)draggingSourceOperationMask { + return NSDragOperationCopy | NSDragOperationMove; } - (NSPoint)draggingLocation { return NSMakePoint(10, 10); } +// NSPasteboard mocking functions. + +- (BOOL)containsURLData { + NSArray* urlTypes = [NSArray arrayWithObjects: + kWebURLsWithTitlesPboardType, + NSURLPboardType, + NSFilenamesPboardType, + NSStringPboardType, + nil]; + if (dragDataType_) + return [urlTypes containsObject:dragDataType_]; + return NO; +} + - (NSData*)dataForType:(NSString*)type { - if ([type isEqual:kBookmarkButtonDragType]) - return data_.get(); + if (dragDataType_ && [dragDataType_ isEqualToString:type]) + return [NSData data]; // Return something, anything. return nil; } // Fake a controller for callback ponging + - (BOOL)dragButton:(BookmarkButton*)button to:(NSPoint)point copy:(BOOL)copy { - pong_ = YES; + dragButtonToPong_ = YES; + return YES; +} + +- (BOOL)addURLs:(NSArray*)urls withTitles:(NSArray*)titles at:(NSPoint)point { + dragURLsPong_ = YES; return YES; } -// Confirm the pong. +- (void)getURLs:(NSArray**)outUrls andTitles:(NSArray**)outTitles { +} + +- (BOOL)dragBookmarkData:(id<NSDraggingInfo>)info { + dragBookmarkDataPong_ = YES; + return NO; +} + +// Confirm the pongs. + - (BOOL)dragButtonToPong { - return pong_; + return dragButtonToPong_; } -- (CGFloat)indicatorPosForDragOfButton:(BookmarkButton*)sourceButton - toPoint:(NSPoint)point { +- (BOOL)dragURLsPong { + return dragURLsPong_; +} + +- (BOOL)dragBookmarkDataPong { + return dragBookmarkDataPong_; +} + +- (CGFloat)indicatorPosForDragToPoint:(NSPoint)point { return kFakeIndicatorPos; } @@ -89,10 +146,6 @@ const CGFloat kFakeIndicatorPos = 7.0; return NSDragOperationNone; } -- (NSDragOperation)draggingSourceOperationMask { - return NSDragOperationCopy | NSDragOperationMove; -} - @end namespace { @@ -114,21 +167,65 @@ TEST_F(BookmarkBarViewTest, CanDragWindow) { TEST_F(BookmarkBarViewTest, BookmarkButtonDragAndDrop) { scoped_nsobject<FakeBookmarkDraggingInfo> info([[FakeBookmarkDraggingInfo alloc] init]); - [view_ setController:info.get()]; + [info reset]; + + [info setDragDataType:kBookmarkButtonDragType]; EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); EXPECT_TRUE([view_ performDragOperation:(id)info.get()]); EXPECT_TRUE([info dragButtonToPong]); + EXPECT_FALSE([info dragURLsPong]); + EXPECT_TRUE([info dragBookmarkDataPong]); } -TEST_F(BookmarkBarViewTest, BookmarkButtonDropIndicator) { +TEST_F(BookmarkBarViewTest, URLDragAndDrop) { scoped_nsobject<FakeBookmarkDraggingInfo> info([[FakeBookmarkDraggingInfo alloc] init]); + [view_ setController:info.get()]; + [info reset]; + + [info setDragDataType:kWebURLsWithTitlesPboardType]; + EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); + EXPECT_TRUE([view_ performDragOperation:(id)info.get()]); + EXPECT_FALSE([info dragButtonToPong]); + EXPECT_TRUE([info dragURLsPong]); + EXPECT_TRUE([info dragBookmarkDataPong]); + [info reset]; + + [info setDragDataType:NSURLPboardType]; + EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); + EXPECT_TRUE([view_ performDragOperation:(id)info.get()]); + EXPECT_FALSE([info dragButtonToPong]); + EXPECT_TRUE([info dragURLsPong]); + EXPECT_TRUE([info dragBookmarkDataPong]); + [info reset]; + [info setDragDataType:NSFilenamesPboardType]; + EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); + EXPECT_TRUE([view_ performDragOperation:(id)info.get()]); + EXPECT_FALSE([info dragButtonToPong]); + EXPECT_TRUE([info dragURLsPong]); + EXPECT_TRUE([info dragBookmarkDataPong]); + [info reset]; + + [info setDragDataType:NSStringPboardType]; + EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); + EXPECT_TRUE([view_ performDragOperation:(id)info.get()]); + EXPECT_FALSE([info dragButtonToPong]); + EXPECT_TRUE([info dragURLsPong]); + EXPECT_TRUE([info dragBookmarkDataPong]); +} + +TEST_F(BookmarkBarViewTest, BookmarkButtonDropIndicator) { + scoped_nsobject<FakeBookmarkDraggingInfo> + info([[FakeBookmarkDraggingInfo alloc] init]); [view_ setController:info.get()]; + + [info reset]; + [info setDragDataType:kBookmarkButtonDragType]; EXPECT_FALSE([info draggingEnteredCalled]); EXPECT_EQ([view_ draggingEntered:(id)info.get()], NSDragOperationMove); - EXPECT_TRUE([info draggingEnteredCalled]); // Ensure controller pingged. + EXPECT_TRUE([info draggingEnteredCalled]); // Ensure controller pinged. EXPECT_TRUE([view_ dropIndicatorShown]); EXPECT_EQ([view_ dropIndicatorPosition], kFakeIndicatorPos); diff --git a/chrome/browser/cocoa/bookmark_button.h b/chrome/browser/cocoa/bookmark_button.h index 62acadc..a40f23c 100644 --- a/chrome/browser/cocoa/bookmark_button.h +++ b/chrome/browser/cocoa/bookmark_button.h @@ -3,11 +3,13 @@ // found in the LICENSE file. #import <Cocoa/Cocoa.h> +#include <vector> #import "chrome/browser/cocoa/draggable_button.h" #include "webkit/glue/window_open_disposition.h" @class BookmarkBarFolderController; @class BookmarkButton; +class BookmarkDragData; class BookmarkModel; class BookmarkNode; @class BrowserWindowController; @@ -70,7 +72,7 @@ class ThemeProvider; // mode on the NTP. - (BOOL)dragShouldLockBarVisibility; -// Perform the actual DnD of a bookmark button. +// Perform the actual DnD of a bookmark or bookmark button. // |point| is in the base coordinate system of the destination window; // |it comes from an id<NSDraggingInfo>. |copy| is YES if a copy is to be @@ -81,6 +83,15 @@ class ThemeProvider; to:(NSPoint)point copy:(BOOL)copy; +// Determine if the pasteboard from |info| has dragging data containing +// bookmark(s) and perform the drag and return YES, otherwise return NO. +- (BOOL)dragBookmarkData:(id<NSDraggingInfo>)info; + +// Determine if the drag pasteboard has any drag data of type +// kBookmarkDictionaryListPboardType and, if so, return those elements +// otherwise return an empty vector. +- (std::vector<const BookmarkNode*>)retrieveBookmarkDragDataNodes; + // Return YES if we should show the drop indicator, else NO. In some // cases (e.g. hover open) we don't want to show the drop indicator. // |point| is in the base coordinate system of the destination window; @@ -94,8 +105,7 @@ class ThemeProvider; // |it comes from an id<NSDraggingInfo>. // TODO(viettrungluu,jrg): instead of this, make buttons move around. // http://crbug.com/35968 -- (CGFloat)indicatorPosForDragOfButton:(BookmarkButton*)sourceButton - toPoint:(NSPoint)point; +- (CGFloat)indicatorPosForDragToPoint:(NSPoint)point; // Return the theme provider associated with this browser window. - (ThemeProvider*)themeProvider; @@ -133,6 +143,15 @@ class ThemeProvider; - (void)addButtonForNode:(const BookmarkNode*)node atIndex:(NSInteger)buttonIndex; +// Given a list or |urls| and |titles|, create new bookmark nodes and add +// them to the bookmark model such that they will be 1) added to the folder +// represented by the button at |point| if it is a folder, or 2) inserted +// into the parent of the non-folder bookmark at |point| in front of that +// button. Returns YES if at least one bookmark was added. +// TODO(mrossetti): Change function to use a pair-like structure for +// URLs and titles. http://crbug.com/44411 +- (BOOL)addURLs:(NSArray*)urls withTitles:(NSArray*)titles at:(NSPoint)point; + // Move a button from one place in the menu to another. This is safe // to call when a folder menu window is open as that window will be updated. - (void)moveButtonFromIndex:(NSInteger)fromIndex toIndex:(NSInteger)toIndex; diff --git a/chrome/browser/cocoa/bookmark_folder_target.h b/chrome/browser/cocoa/bookmark_folder_target.h index 60ca820..2c02c45 100644 --- a/chrome/browser/cocoa/bookmark_folder_target.h +++ b/chrome/browser/cocoa/bookmark_folder_target.h @@ -41,4 +41,9 @@ class BookmarkNode; @end +// The (internal) |NSPasteboard| type string for bookmark button drags, used for +// dragging buttons around the bookmark bar. The data for this type is just a +// pointer to the |BookmarkButton| being dragged. +extern NSString* kBookmarkButtonDragType; + #endif // CHROME_BROWSER_COCOA_BOOKMARK_FOLDER_TARGET_CONTROLLER_H_ diff --git a/chrome/browser/cocoa/cocoa_test_helper.h b/chrome/browser/cocoa/cocoa_test_helper.h index 7ef1775..50d0707 100644 --- a/chrome/browser/cocoa/cocoa_test_helper.h +++ b/chrome/browser/cocoa/cocoa_test_helper.h @@ -188,4 +188,33 @@ class CocoaTestHelper : public CocoaNoWindowTestHelper { scoped_nsobject<CocoaTestHelperWindow> window_; }; +// A macro which determines the proper float epsilon for a CGFloat. +#if CGFLOAT_IS_DOUBLE +#define CGFLOAT_EPSILON DBL_EPSILON +#else +#define CGFLOAT_EPSILON FLT_EPSILON +#endif + +// A macro which which determines if two CGFloats are equal taking a +// proper epsilon into consideration. +#define CGFLOAT_EQ(expected, actual) \ + (actual >= (expected - CGFLOAT_EPSILON) && \ + actual <= (expected + CGFLOAT_EPSILON)) + +// A test support macro which ascertains if two CGFloats are equal. +#define EXPECT_CGFLOAT_EQ(expected, actual) \ + EXPECT_TRUE(CGFLOAT_EQ(expected, actual)) << \ + expected << " != " << actual + +// A test support macro which compares two NSRects for equality taking +// the float epsilon into consideration. +#define EXPECT_NSRECT_EQ(expected, actual) \ + EXPECT_TRUE(CGFLOAT_EQ(expected.origin.x, actual.origin.x) && \ + CGFLOAT_EQ(expected.origin.y, actual.origin.y) && \ + CGFLOAT_EQ(expected.size.width, actual.size.width) && \ + CGFLOAT_EQ(expected.size.height, actual.size.height)) << \ + "Rects do not match: " << \ + [NSStringFromRect(expected) UTF8String] << \ + " != " << [NSStringFromRect(actual) UTF8String] + #endif // CHROME_BROWSER_COCOA_COCOA_TEST_HELPER_H_ |