summaryrefslogtreecommitdiffstats
path: root/chrome/browser/views
diff options
context:
space:
mode:
Diffstat (limited to 'chrome/browser/views')
-rw-r--r--chrome/browser/views/bookmark_bar_view.cc161
-rw-r--r--chrome/browser/views/bookmark_bar_view.h3
-rw-r--r--chrome/browser/views/bookmark_bubble_view.cc17
-rw-r--r--chrome/browser/views/bookmark_editor_view.cc113
-rw-r--r--chrome/browser/views/bookmark_editor_view.h31
-rw-r--r--chrome/browser/views/bookmark_editor_view_unittest.cc61
-rw-r--r--chrome/browser/views/toolbar_view.cc19
7 files changed, 205 insertions, 200 deletions
diff --git a/chrome/browser/views/bookmark_bar_view.cc b/chrome/browser/views/bookmark_bar_view.cc
index b7475ee..8d2531a 100644
--- a/chrome/browser/views/bookmark_bar_view.cc
+++ b/chrome/browser/views/bookmark_bar_view.cc
@@ -145,6 +145,21 @@ static const SkColor kInstructionsColor = SkColorSetRGB(128, 128, 142);
namespace {
+// Calculates the drop operation given the event and supported set of
+// operations.
+int PreferredDropOperation(const DropTargetEvent& event, int operation) {
+ int common_ops = (event.GetSourceOperations() & operation);
+ if (!common_ops)
+ return 0;
+ if (DragDropTypes::DRAG_COPY & common_ops)
+ return DragDropTypes::DRAG_COPY;
+ if (DragDropTypes::DRAG_LINK & common_ops)
+ return DragDropTypes::DRAG_LINK;
+ if (DragDropTypes::DRAG_MOVE & common_ops)
+ return DragDropTypes::DRAG_MOVE;
+ return DragDropTypes::DRAG_NONE;
+}
+
// Returns the tooltip text for the specified url and title. The returned
// text is clipped to fit within the bounds of the monitor.
//
@@ -420,22 +435,15 @@ class MenuRunner : public ChromeViews::MenuDelegate,
if (drop_data_.is_url)
return true;
- if (drop_data_.profile_id != view_->GetProfile()->GetID()) {
- // Always accept drags of bookmark groups from other profiles.
+ BookmarkNode* drag_node = drop_data_.GetNode(view_->GetProfile());
+ if (!drag_node) {
+ // Dragging a group from another profile, always accept.
return true;
}
// Drag originated from same profile and is not a URL. Only accept it if
// the dragged node is not a parent of the node menu represents.
BookmarkNode* drop_node = menu_id_to_node_map_[menu->GetCommand()];
DCHECK(drop_node);
- BookmarkNode* drag_node =
- drop_data_.GetNode(view_->GetProfile()->GetBookmarkModel());
- if (!drag_node) {
- // Hmmm, can't find the dragged node. This is generally an error
- // condition and we won't try and do anything fancy.
- NOTREACHED();
- return false;
- }
BookmarkNode* node = drop_node;
while (drop_node && drop_node != drag_node)
drop_node = drop_node->GetParent();
@@ -456,7 +464,7 @@ class MenuRunner : public ChromeViews::MenuDelegate,
index_to_drop_at = node->GetChildCount();
}
DCHECK(drop_parent);
- return view_->CalculateDropOperation(drop_data_, drop_parent,
+ return view_->CalculateDropOperation(event, drop_data_, drop_parent,
index_to_drop_at);
}
@@ -929,6 +937,7 @@ int BookmarkBarView::OnDragUpdated(const DropTargetEvent& event) {
if (drop_info_->valid &&
(drop_info_->x == event.x() && drop_info_->y == event.y())) {
+ // The location of the mouse didn't change, return the last operation.
return drop_info_->drag_operation;
}
@@ -948,6 +957,8 @@ int BookmarkBarView::OnDragUpdated(const DropTargetEvent& event) {
drop_info_->drop_on == drop_on &&
drop_info_->is_over_overflow == is_over_overflow &&
drop_info_->is_over_other == is_over_other) {
+ // The position we're going to drop didn't change, return the last drag
+ // operation we calculated.
return drop_info_->drag_operation;
}
@@ -1290,8 +1301,7 @@ void BookmarkBarView::WriteDragData(BookmarkNode* node,
OSExchangeData* data) {
DCHECK(node && data);
BookmarkDragData drag_data(node);
- drag_data.profile_id = GetProfile()->GetID();
- drag_data.Write(data);
+ drag_data.Write(profile_, data);
}
int BookmarkBarView::GetDragOperations(View* sender, int x, int y) {
@@ -1591,7 +1601,10 @@ int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
} else if (!GetBookmarkButtonCount()) {
// No bookmarks, accept the drop.
*index = 0;
- return DragDropTypes::DRAG_COPY;
+ int ops = data.GetNode(profile_)
+ ? DragDropTypes::DRAG_MOVE
+ : DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK;
+ return PreferredDropOperation(event, ops);
}
for (int i = 0; i < GetBookmarkButtonCount() &&
@@ -1650,49 +1663,33 @@ int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
*is_over_other ? model_->other_node() :
model_->GetBookmarkBarNode()->GetChild(*index);
int operation =
- CalculateDropOperation(data, parent, parent->GetChildCount());
- if (!operation && !data.is_url &&
- data.profile_id == GetProfile()->GetID()) {
- if (data.GetNode(model_) == parent) {
- // Don't open a menu if the node being dragged is the the menu to
- // open.
- *drop_on = false;
- }
+ CalculateDropOperation(event, data, parent, parent->GetChildCount());
+ if (!operation && !data.is_url && data.GetNode(profile_) == parent) {
+ // Don't open a menu if the node being dragged is the the menu to
+ // open.
+ *drop_on = false;
}
return operation;
} else {
- return CalculateDropOperation(data, model_->GetBookmarkBarNode(), *index);
+ return CalculateDropOperation(event, data, model_->GetBookmarkBarNode(),
+ *index);
}
}
-int BookmarkBarView::CalculateDropOperation(const BookmarkDragData& data,
+int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
+ const BookmarkDragData& data,
BookmarkNode* parent,
int index) {
if (!CanDropAt(data, parent, index))
return DragDropTypes::DRAG_NONE;
- if (data.is_url) {
- // User is dragging a URL.
- BookmarkNode* node = model_->GetNodeByURL(data.url);
- if (!node) {
- // We don't have a node with this url.
- return DragDropTypes::DRAG_COPY;
- }
- // Technically we're going to move, but most sources export as copy so that
- // if we don't accept copy we won't accept the drop.
- return DragDropTypes::DRAG_MOVE | DragDropTypes::DRAG_COPY;
- } else if (data.profile_id == GetProfile()->GetID()) {
- // Dropping a group from the same profile results in a move.
- BookmarkNode* node = data.GetNode(model_);
- if (!node) {
- // Generally shouldn't get here, we originated the drag but couldn't
- // find the node.
- return DragDropTypes::DRAG_NONE;
- }
+ if (data.GetNode(profile_)) {
+ // User is dragging from this profile: move.
return DragDropTypes::DRAG_MOVE;
} else {
- // Dropping a group from different profile. Always accept.
- return DragDropTypes::DRAG_COPY;
+ // User is dragging from another app, copy.
+ return PreferredDropOperation(
+ event, DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK);
}
}
@@ -1700,29 +1697,19 @@ bool BookmarkBarView::CanDropAt(const BookmarkDragData& data,
BookmarkNode* parent,
int index) {
DCHECK(data.is_valid);
- if (data.is_url) {
- BookmarkNode* existing_node = model_->GetNodeByURL(data.url);
- if (existing_node && existing_node->GetParent() == parent) {
- const int existing_index = parent->IndexOfChild(existing_node);
+ BookmarkNode* dragged_node = data.GetNode(profile_);
+ if (dragged_node) {
+ if (dragged_node->GetParent() == parent) {
+ const int existing_index = parent->IndexOfChild(dragged_node);
if (index == existing_index || existing_index + 1 == index)
return false;
}
- return true;
- } else if (data.profile_id == profile_->GetID()) {
- BookmarkNode* existing_node = data.GetNode(model_);
- if (existing_node) {
- if (existing_node->GetParent() == parent) {
- const int existing_index = parent->IndexOfChild(existing_node);
- if (index == existing_index || existing_index + 1 == index)
- return false;
- }
- // Allow the drop only if the node we're going to drop on isn't a
- // descendant of the dragged node.
- BookmarkNode* test_node = parent;
- while (test_node && test_node != existing_node)
- test_node = test_node->GetParent();
- return (test_node == NULL);
- }
+ // Allow the drop only if the node we're going to drop on isn't a
+ // descendant of the dragged node.
+ BookmarkNode* test_node = parent;
+ while (test_node && test_node != dragged_node)
+ test_node = test_node->GetParent();
+ return (test_node == NULL);
} // else case clones, always allow.
return true;
}
@@ -1731,31 +1718,22 @@ bool BookmarkBarView::CanDropAt(const BookmarkDragData& data,
int BookmarkBarView::PerformDropImpl(const BookmarkDragData& data,
BookmarkNode* parent_node,
int index) {
- if (data.is_url) {
- // User is dragging a URL.
- BookmarkNode* node = model_->GetNodeByURL(data.url);
- if (!node) {
- std::wstring title = data.title;
- if (title.empty()) {
- // No title, use the host.
- title = UTF8ToWide(data.url.host());
- if (title.empty())
- title = l10n_util::GetString(IDS_BOOMARK_BAR_UNKNOWN_DRAG_TITLE);
- }
- model_->AddURL(parent_node, index, title, data.url);
- return DragDropTypes::DRAG_COPY;
- }
- model_->Move(node, parent_node, index);
+ BookmarkNode* dragged_node = data.GetNode(profile_);
+ if (dragged_node) {
+ // Drag from same profile, do a move.
+ model_->Move(dragged_node, parent_node, index);
return DragDropTypes::DRAG_MOVE;
- } else if (data.profile_id == GetProfile()->GetID()) {
- BookmarkNode* node = data.GetNode(model_);
- if (!node) {
- // Generally shouldn't get here, we originated the drag but couldn't
- // find the node. Do nothing.
- return DragDropTypes::DRAG_COPY;
+ } else if (data.is_url) {
+ // New URL, add it at the specified location.
+ std::wstring title = data.title;
+ if (title.empty()) {
+ // No title, use the host.
+ title = UTF8ToWide(data.url.host());
+ if (title.empty())
+ title = l10n_util::GetString(IDS_BOOMARK_BAR_UNKNOWN_DRAG_TITLE);
}
- model_->Move(node, parent_node, index);
- return DragDropTypes::DRAG_MOVE;
+ model_->AddURL(parent_node, index, title, data.url);
+ return DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK;
} else {
// Dropping a group from different profile. Always accept.
CloneDragData(data, parent_node, index);
@@ -1768,12 +1746,7 @@ void BookmarkBarView::CloneDragData(const BookmarkDragData& data,
int index_to_add_at) {
DCHECK(data.is_valid && model_);
if (data.is_url) {
- BookmarkNode* node = model_->GetNodeByURL(data.url);
- if (node) {
- model_->Move(node, parent, index_to_add_at);
- } else {
- model_->AddURL(parent, index_to_add_at, data.title, data.url);
- }
+ model_->AddURL(parent, index_to_add_at, data.title, data.url);
} else {
BookmarkNode* new_folder = model_->AddGroup(parent, index_to_add_at,
data.title);
@@ -1800,7 +1773,7 @@ void BookmarkBarView::StartThrobbing() {
if (!GetViewContainer())
return; // We're not showing, don't do anything.
- BookmarkNode* node = model_->GetNodeByURL(bubble_url_);
+ BookmarkNode* node = model_->GetMostRecentlyAddedNodeForURL(bubble_url_);
if (!node)
return; // Generally shouldn't happen.
diff --git a/chrome/browser/views/bookmark_bar_view.h b/chrome/browser/views/bookmark_bar_view.h
index 552dbaf..605322b 100644
--- a/chrome/browser/views/bookmark_bar_view.h
+++ b/chrome/browser/views/bookmark_bar_view.h
@@ -346,7 +346,8 @@ class BookmarkBarView : public ChromeViews::View,
// Invokes CanDropAt to determine if this is a valid location for the data,
// then returns the appropriate drag operation based on the data.
- int CalculateDropOperation(const BookmarkDragData& data,
+ int CalculateDropOperation(const ChromeViews::DropTargetEvent& event,
+ const BookmarkDragData& data,
BookmarkNode* parent,
int index);
diff --git a/chrome/browser/views/bookmark_bubble_view.cc b/chrome/browser/views/bookmark_bubble_view.cc
index 186defe..b70416c 100644
--- a/chrome/browser/views/bookmark_bubble_view.cc
+++ b/chrome/browser/views/bookmark_bubble_view.cc
@@ -163,8 +163,9 @@ BookmarkBubbleView::BookmarkBubbleView(InfoBubbleDelegate* delegate,
profile_(profile),
url_(url),
newly_bookmarked_(newly_bookmarked),
- parent_model_(profile_->GetBookmarkModel(),
- profile_->GetBookmarkModel()->GetNodeByURL(url)) {
+ parent_model_(
+ profile_->GetBookmarkModel(),
+ profile_->GetBookmarkModel()->GetMostRecentlyAddedNodeForURL(url)) {
Init();
}
@@ -255,7 +256,7 @@ void BookmarkBubbleView::Init() {
std::wstring BookmarkBubbleView::GetTitle() {
BookmarkModel* bookmark_model= profile_->GetBookmarkModel();
- BookmarkNode* node = bookmark_model->GetNodeByURL(url_);
+ BookmarkNode* node = bookmark_model->GetMostRecentlyAddedNodeForURL(url_);
if (node)
return node->GetTitle();
else
@@ -289,7 +290,7 @@ void BookmarkBubbleView::ItemChanged(ComboBox* combo_box,
return;
}
BookmarkModel* model = profile_->GetBookmarkModel();
- BookmarkNode* node = model->GetNodeByURL(url_);
+ BookmarkNode* node = model->GetMostRecentlyAddedNodeForURL(url_);
if (node) {
BookmarkNode* new_parent = parent_model_.GetNodeAt(new_index);
if (new_parent != node->GetParent()) {
@@ -331,6 +332,9 @@ void BookmarkBubbleView::RemoveBookmark() {
}
void BookmarkBubbleView::ShowEditor() {
+ BookmarkNode* node =
+ profile_->GetBookmarkModel()->GetMostRecentlyAddedNodeForURL(url_);
+
// The user may have edited the title, commit it now.
SetNodeTitleFromTextField();
@@ -351,12 +355,13 @@ void BookmarkBubbleView::ShowEditor() {
// the delete and all that.
Close();
- BookmarkEditorView::Show(parent, profile_, url_, title_);
+ if (node)
+ BookmarkEditorView::Show(parent, profile_, NULL, node);
}
void BookmarkBubbleView::SetNodeTitleFromTextField() {
BookmarkModel* model = profile_->GetBookmarkModel();
- BookmarkNode* node = model->GetNodeByURL(url_);
+ BookmarkNode* node = model->GetMostRecentlyAddedNodeForURL(url_);
if (node) {
const std::wstring new_title = title_tf_->GetText();
if (new_title != node->GetTitle()) {
diff --git a/chrome/browser/views/bookmark_editor_view.cc b/chrome/browser/views/bookmark_editor_view.cc
index 2d777b3..e963b6b 100644
--- a/chrome/browser/views/bookmark_editor_view.cc
+++ b/chrome/browser/views/bookmark_editor_view.cc
@@ -42,22 +42,22 @@ static const int kNewGroupButtonID = 1002;
// static
void BookmarkEditorView::Show(HWND parent_hwnd,
Profile* profile,
- const GURL& url,
- const std::wstring& title) {
+ BookmarkNode* parent,
+ BookmarkNode* node) {
DCHECK(profile);
- BookmarkEditorView* editor = new BookmarkEditorView(profile, url, title);
+ BookmarkEditorView* editor = new BookmarkEditorView(profile, parent, node);
editor->Show(parent_hwnd);
}
BookmarkEditorView::BookmarkEditorView(Profile* profile,
- const GURL& url,
- const std::wstring& title)
+ BookmarkNode* parent,
+ BookmarkNode* node)
: profile_(profile),
#pragma warning(suppress: 4355) // Okay to pass "this" here.
new_group_button_(
l10n_util::GetString(IDS_BOOMARK_EDITOR_NEW_FOLDER_BUTTON)),
- url_(url),
- title_(title),
+ parent_(parent),
+ node_(node),
running_menu_for_root_(false) {
DCHECK(profile);
Init();
@@ -238,10 +238,10 @@ void BookmarkEditorView::Init() {
new_group_button_.SetListener(this);
new_group_button_.SetID(kNewGroupButtonID);
- title_tf_.SetText(title_);
+ title_tf_.SetText(node_ ? node_->GetTitle() : std::wstring());
title_tf_.SetController(this);
- url_tf_.SetText(UTF8ToWide(url_.spec()));
+ url_tf_.SetText(node_ ? UTF8ToWide(node_->GetURL().spec()) : std::wstring());
url_tf_.SetController(this);
// Yummy layout code.
@@ -292,11 +292,7 @@ void BookmarkEditorView::Init() {
layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
if (bb_model_->IsLoaded())
- Loaded(bb_model_);
-}
-
-void BookmarkEditorView::Loaded(BookmarkModel* model) {
- Reset(true);
+ Reset();
}
void BookmarkEditorView::BookmarkNodeMoved(BookmarkModel* model,
@@ -304,32 +300,27 @@ void BookmarkEditorView::BookmarkNodeMoved(BookmarkModel* model,
int old_index,
BookmarkNode* new_parent,
int new_index) {
- Reset(false);
+ Reset();
}
void BookmarkEditorView::BookmarkNodeAdded(BookmarkModel* model,
BookmarkNode* parent,
int index) {
- Reset(false);
+ Reset();
}
void BookmarkEditorView::BookmarkNodeRemoved(BookmarkModel* model,
BookmarkNode* parent,
int index) {
- Reset(false);
-}
-
-void BookmarkEditorView::Reset(bool first_time) {
- BookmarkNode* node_editing = bb_model_->GetNodeByURL(url_);
-
- // If the title is empty we need to fetch it from the node.
- if (first_time && title_.empty()) {
- if (node_editing) {
- title_ = node_editing->GetTitle();
- title_tf_.SetText(title_);
- }
+ if ((node_ && !node_->GetParent()) || (parent_ && !parent_->GetParent())) {
+ // The node, or its parent was removed. Close the dialog.
+ window()->Close();
+ } else {
+ Reset();
}
+}
+void BookmarkEditorView::Reset() {
// Do this first, otherwise when we invoke SetModel with the real one
// tree_view will try to invoke something on the model we just deleted.
tree_view_.SetModel(NULL);
@@ -346,9 +337,7 @@ void BookmarkEditorView::Reset(bool first_time) {
if (GetParent()) {
ExpandAndSelect();
-
- if(!first_time)
- UserInputChanged();
+ UserInputChanged();
} else if (GetParent()) {
tree_view_.ExpandAll();
}
@@ -396,11 +385,8 @@ BookmarkEditorView::EditorNode* BookmarkEditorView::AddNewGroup(
void BookmarkEditorView::ExpandAndSelect() {
tree_view_.ExpandAll();
- BookmarkNode* to_select = bb_model_->GetNodeByURL(url_);
- int group_id_to_select =
- to_select ? to_select->GetParent()->id() :
- bb_model_->GetParentForNewNodes()->id();
-
+ BookmarkNode* to_select = node_ ? node_->GetParent() : parent_;
+ int group_id_to_select = to_select->id();
DCHECK(group_id_to_select); // GetMostRecentParent should never return NULL.
EditorNode* b_node =
FindNodeWithID(tree_model_->GetRoot(), group_id_to_select);
@@ -468,14 +454,8 @@ void BookmarkEditorView::ApplyEdits(EditorNode* parent) {
GURL new_url(GetInputURL());
std::wstring new_title(GetInputTitle());
- BookmarkNode* old_node = bb_model_->GetNodeByURL(url_);
- BookmarkNode* old_parent = old_node ? old_node->GetParent() : NULL;
- const int old_index = old_parent ? old_parent->IndexOfChild(old_node) : -1;
-
- if (url_ != new_url) {
- // The URL has changed, unstar the old url.
- bb_model_->SetURLStarred(url_, std::wstring(), false);
- }
+ BookmarkNode* old_parent = node_ ? node_->GetParent() : NULL;
+ const int old_index = old_parent ? old_parent->IndexOfChild(node_) : -1;
// Create the new groups and update the titles.
BookmarkNode* new_parent = NULL;
@@ -488,29 +468,34 @@ void BookmarkEditorView::ApplyEdits(EditorNode* parent) {
return;
}
- BookmarkNode* current_node = bb_model_->GetNodeByURL(new_url);
-
- if (current_node) {
- // There's already a node with the URL.
- bb_model_->SetTitle(current_node, new_title);
- if (new_parent == old_parent) {
- // Parent hasn't changed.
- bb_model_->Move(current_node, new_parent, old_index);
+ if (node_) {
+ Time date_added = node_->date_added();
+ if (new_parent == node_->GetParent()) {
+ // The parent is the same.
+ if (new_url != node_->GetURL()) {
+ bb_model_->Remove(old_parent, old_index);
+ BookmarkNode* new_node =
+ bb_model_->AddURL(old_parent, old_index, new_title, new_url);
+ new_node->date_added_ = date_added;
+ } else {
+ bb_model_->SetTitle(node_, new_title);
+ }
+ } else if (new_url != node_->GetURL()) {
+ // The parent and URL changed.
+ bb_model_->Remove(old_parent, old_index);
+ BookmarkNode* new_node =
+ bb_model_->AddURL(new_parent, new_parent->GetChildCount(), new_title,
+ new_url);
+ new_node->date_added_ = date_added;
} else {
- // Parent changed, move to end of new parent.
- bb_model_->Move(current_node, new_parent, new_parent->GetChildCount());
+ // The parent and title changed. Move the node and change the title.
+ bb_model_->Move(node_, new_parent, new_parent->GetChildCount());
+ bb_model_->SetTitle(node_, new_title);
}
} else {
- // Adding a new URL.
- if (new_parent == old_parent) {
- // Parent hasn't changed. Place newly created bookmark at the same
- // location as last bookmark.
- bb_model_->AddURL(new_parent, old_index, new_title, new_url);
- } else {
- // Parent changed, put bookmark at end of new parent.
- bb_model_->AddURL(new_parent, new_parent->GetChildCount(), new_title,
- new_url);
- }
+ // We're adding a new URL.
+ bb_model_->AddURL(new_parent, new_parent->GetChildCount(), new_title,
+ new_url);
}
}
diff --git a/chrome/browser/views/bookmark_editor_view.h b/chrome/browser/views/bookmark_editor_view.h
index b928dc9..1f2f3fe 100644
--- a/chrome/browser/views/bookmark_editor_view.h
+++ b/chrome/browser/views/bookmark_editor_view.h
@@ -7,13 +7,13 @@
#include <set>
-#include "chrome/views/tree_node_model.h"
#include "chrome/browser/bookmarks/bookmark_model.h"
#include "chrome/views/checkbox.h"
#include "chrome/views/dialog_delegate.h"
#include "chrome/views/menu.h"
#include "chrome/views/native_button.h"
#include "chrome/views/text_field.h"
+#include "chrome/views/tree_node_model.h"
namespace ChromeViews {
class Window;
@@ -40,21 +40,24 @@ class BookmarkEditorView : public ChromeViews::View,
public Menu::Delegate,
public BookmarkModelObserver {
FRIEND_TEST(BookmarkEditorViewTest, ChangeParent);
+ FRIEND_TEST(BookmarkEditorViewTest, ChangeParentAndURL);
FRIEND_TEST(BookmarkEditorViewTest, ChangeURLToExistingURL);
FRIEND_TEST(BookmarkEditorViewTest, EditTitleKeepsPosition);
FRIEND_TEST(BookmarkEditorViewTest, EditURLKeepsPosition);
FRIEND_TEST(BookmarkEditorViewTest, ModelsMatch);
FRIEND_TEST(BookmarkEditorViewTest, MoveToNewParent);
+ FRIEND_TEST(BookmarkEditorViewTest, NewURL);
public:
- // Shows the BookmarkEditorView editing the specified entry.
+ // Shows the BookmarkEditorView editing |node|. If |node| is NULL a new entry
+ // is created initially parented to |parent|.
static void Show(HWND parent_window,
Profile* profile,
- const GURL& url,
- const std::wstring& title);
+ BookmarkNode* parent,
+ BookmarkNode* node);
BookmarkEditorView(Profile* profile,
- const GURL& url,
- const std::wstring& title);
+ BookmarkNode* parent,
+ BookmarkNode* node);
virtual ~BookmarkEditorView();
@@ -135,7 +138,7 @@ class BookmarkEditorView : public ChromeViews::View,
// BookmarkModel observer methods. Any structural change results in
// resetting the tree model.
- virtual void Loaded(BookmarkModel* model);
+ virtual void Loaded(BookmarkModel* model) { }
virtual void BookmarkNodeMoved(BookmarkModel* model,
BookmarkNode* old_parent,
int old_index,
@@ -153,9 +156,7 @@ class BookmarkEditorView : public ChromeViews::View,
BookmarkNode* node) {}
// Resets the model of the tree and updates the various buttons appropriately.
- // If first_time is true, Reset is being invoked from the constructor or
- // once the bookmark bar has finished loading.
- void Reset(bool first_time);
+ void Reset();
// Expands all the nodes in the tree and selects the parent node of the
// url we're editing or the most recent parent if the url being editted isn't
@@ -233,15 +234,15 @@ class BookmarkEditorView : public ChromeViews::View,
// Used for editing the title.
ChromeViews::TextField title_tf_;
- // URL we were created with.
- const GURL url_;
+ // Initial parent to select. Is only used if node_ is NULL.
+ BookmarkNode* parent_;
+
+ // Node being edited. Is NULL if creating a new node.
+ BookmarkNode* node_;
// The context menu.
scoped_ptr<Menu> context_menu_;
- // Title of the url to display.
- std::wstring title_;
-
// Mode used to create nodes from.
BookmarkModel* bb_model_;
diff --git a/chrome/browser/views/bookmark_editor_view_unittest.cc b/chrome/browser/views/bookmark_editor_view_unittest.cc
index 74baaf5..767d501 100644
--- a/chrome/browser/views/bookmark_editor_view_unittest.cc
+++ b/chrome/browser/views/bookmark_editor_view_unittest.cc
@@ -38,6 +38,10 @@ class BookmarkEditorViewTest : public testing::Test {
std::string base_path() const { return "file:///c:/tmp/"; }
+ BookmarkNode* GetNode(const std::string& name) {
+ return model_->GetMostRecentlyAddedNodeForURL(GURL(base_path() + name));
+ }
+
private:
// Creates the following structure:
// bookmark bar node
@@ -72,8 +76,7 @@ class BookmarkEditorViewTest : public testing::Test {
// Makes sure the tree model matches that of the bookmark bar model.
TEST_F(BookmarkEditorViewTest, ModelsMatch) {
- BookmarkEditorView editor(profile_.get(), GURL(base_path() + "xxx"),
- L"xxx");
+ BookmarkEditorView editor(profile_.get(), NULL, NULL);
BookmarkEditorView::EditorNode* editor_root = editor.tree_model_->GetRoot();
// The root should have two children, one for the bookmark bar node,
// the other for the 'other bookmarks' folder.
@@ -97,7 +100,8 @@ TEST_F(BookmarkEditorViewTest, ModelsMatch) {
// Changes the title and makes sure parent/visual order doesn't change.
TEST_F(BookmarkEditorViewTest, EditTitleKeepsPosition) {
- BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"new_a");
+ BookmarkEditorView editor(profile_.get(), NULL, GetNode("a"));
+ editor.title_tf_.SetText(L"new_a");
editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(0));
@@ -109,7 +113,9 @@ TEST_F(BookmarkEditorViewTest, EditTitleKeepsPosition) {
// Changes the url and makes sure parent/visual order doesn't change.
TEST_F(BookmarkEditorViewTest, EditURLKeepsPosition) {
- BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
+ Time node_time = Time::Now() + TimeDelta::FromDays(2);
+ GetNode("a")->date_added_ = node_time;
+ BookmarkEditorView editor(profile_.get(), NULL, GetNode("a"));
editor.url_tf_.SetText(UTF8ToWide(GURL(base_path() + "new_a").spec()));
@@ -119,11 +125,12 @@ TEST_F(BookmarkEditorViewTest, EditURLKeepsPosition) {
ASSERT_EQ(L"a", bb_node->GetChild(0)->GetTitle());
// The URL should have changed.
ASSERT_TRUE(GURL(base_path() + "new_a") == bb_node->GetChild(0)->GetURL());
+ ASSERT_TRUE(node_time == bb_node->GetChild(0)->date_added());
}
// Moves 'a' to be a child of the other node.
TEST_F(BookmarkEditorViewTest, ChangeParent) {
- BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
+ BookmarkEditorView editor(profile_.get(), NULL, GetNode("a"));
editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(1));
@@ -132,28 +139,25 @@ TEST_F(BookmarkEditorViewTest, ChangeParent) {
ASSERT_TRUE(GURL(base_path() + "a") == other_node->GetChild(2)->GetURL());
}
-// Changes the URL to a URL that is already starred.
-TEST_F(BookmarkEditorViewTest, ChangeURLToExistingURL) {
- BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
-
- editor.url_tf_.SetText(UTF8ToWide(GURL(base_path() + "f1a").spec()));
+// Moves 'a' to be a child of the other node and changes its url to new_a.
+TEST_F(BookmarkEditorViewTest, ChangeParentAndURL) {
+ Time node_time = Time::Now() + TimeDelta::FromDays(2);
+ GetNode("a")->date_added_ = node_time;
+ BookmarkEditorView editor(profile_.get(), NULL, GetNode("a"));
- editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(0));
+ editor.url_tf_.SetText(UTF8ToWide(GURL(base_path() + "new_a").spec()));
- // Position shouldn't have changed.
- BookmarkNode* bb_node = profile_->GetBookmarkModel()->GetBookmarkBarNode();
- ASSERT_EQ(L"a", bb_node->GetChild(0)->GetTitle());
- // The URL should have changed.
- ASSERT_TRUE(GURL(base_path() + "f1a") == bb_node->GetChild(0)->GetURL());
+ editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(1));
- // And F1 should have one last child (f1a was removed from it).
- ASSERT_EQ(1, bb_node->GetChild(1)->GetChildCount());
- ASSERT_NE(L"f1a", bb_node->GetChild(1)->GetChild(0)->GetTitle());
+ BookmarkNode* other_node = profile_->GetBookmarkModel()->other_node();
+ ASSERT_EQ(L"a", other_node->GetChild(2)->GetTitle());
+ ASSERT_TRUE(GURL(base_path() + "new_a") == other_node->GetChild(2)->GetURL());
+ ASSERT_TRUE(node_time == other_node->GetChild(2)->date_added());
}
// Creates a new folder and moves a node to it.
TEST_F(BookmarkEditorViewTest, MoveToNewParent) {
- BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
+ BookmarkEditorView editor(profile_.get(), NULL, GetNode("a"));
// Create two nodes: "F21" as a child of "F2" and "F211" as a child of "F21".
BookmarkEditorView::EditorNode* f2 =
@@ -182,3 +186,20 @@ TEST_F(BookmarkEditorViewTest, MoveToNewParent) {
ASSERT_EQ(L"F211", mf21->GetChild(0)->GetTitle());
}
+// Brings up the editor, creating a new URL on the bookmark bar.
+TEST_F(BookmarkEditorViewTest, NewURL) {
+ BookmarkEditorView editor(profile_.get(), NULL, NULL);
+
+ editor.url_tf_.SetText(UTF8ToWide(GURL(base_path() + "a").spec()));
+ editor.title_tf_.SetText(L"new_a");
+
+ editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(0));
+
+ BookmarkNode* bb_node = profile_->GetBookmarkModel()->GetBookmarkBarNode();
+ ASSERT_EQ(4, bb_node->GetChildCount());
+
+ BookmarkNode* new_node = bb_node->GetChild(3);
+
+ EXPECT_EQ(L"new_a", new_node->GetTitle());
+ EXPECT_TRUE(GURL(base_path() + "a") == new_node->GetURL());
+}
diff --git a/chrome/browser/views/toolbar_view.cc b/chrome/browser/views/toolbar_view.cc
index abbad21..8d7ce07 100644
--- a/chrome/browser/views/toolbar_view.cc
+++ b/chrome/browser/views/toolbar_view.cc
@@ -10,6 +10,8 @@
#include "base/path_service.h"
#include "chrome/app/chrome_dll_resource.h"
#include "chrome/app/theme/theme_resources.h"
+#include "chrome/browser/bookmarks/bookmark_drag_data.h"
+#include "chrome/browser/bookmarks/bookmark_model.h"
#include "chrome/browser/browser.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/character_encoding.h"
@@ -668,6 +670,11 @@ int BrowserToolbarView::GetDragOperations(ChromeViews::View* sender,
!tab_->GetURL().is_valid()) {
return DragDropTypes::DRAG_NONE;
}
+ if (profile_ && profile_->GetBookmarkModel() &&
+ profile_->GetBookmarkModel()->IsBookmarked(tab_->GetURL())) {
+ return DragDropTypes::DRAG_MOVE | DragDropTypes::DRAG_COPY |
+ DragDropTypes::DRAG_LINK;
+ }
return DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK;
}
@@ -680,6 +687,18 @@ void BrowserToolbarView::WriteDragData(ChromeViews::View* sender,
UserMetrics::RecordAction(L"Toolbar_DragStar", profile_);
+ // If there is a bookmark for the URL, add the bookmark drag data for it. We
+ // do this to ensure the bookmark is moved, rather than creating an new
+ // bookmark.
+ if (profile_ && profile_->GetBookmarkModel()) {
+ BookmarkNode* node = profile_->GetBookmarkModel()->
+ GetMostRecentlyAddedNodeForURL(tab_->GetURL());
+ if (node) {
+ BookmarkDragData bookmark_data(node);
+ bookmark_data.Write(profile_, data);
+ }
+ }
+
drag_utils::SetURLAndDragImage(tab_->GetURL(), tab_->GetTitle(),
tab_->GetFavIcon(), data);
}
generated by cgit v1.1 at 2025-01-22 07:53:29 +0000