diff options
author | joaodasilva@chromium.org <joaodasilva@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-07 10:45:20 +0000 |
---|---|---|
committer | joaodasilva@chromium.org <joaodasilva@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-07 10:45:20 +0000 |
commit | 25b51679f7b38c11d6c2a05f29265458443d8cbf (patch) | |
tree | 5a9da3305c509eddabbe132362230dd820f63c3c | |
parent | eab49a654e4ada73169925dd0f64779373b44a30 (diff) | |
download | chromium_src-25b51679f7b38c11d6c2a05f29265458443d8cbf.zip chromium_src-25b51679f7b38c11d6c2a05f29265458443d8cbf.tar.gz chromium_src-25b51679f7b38c11d6c2a05f29265458443d8cbf.tar.bz2 |
Show the Managed Bookmarks folder in the views UI.
This change makes the Managed Bookmarks folder visible in the bookmarks
bar if it contains at least one bookmark:
- this folder is always shown floating to the left, next to the Apps button;
- this folder also appears in the bookmarks menu from the hotdog menu, as if
it were part of the bookmarks bar;
- bookmarks can't be dragged into this folder or any of its subfolders;
- dragging bookmarks out of this folder always creates a new copy and never
moves (the mouse cursor is decorated with the + "copy" indicator);
- the bookmark editor can't select managed folders for new bookmarks;
- bookmarks a page that already has a managed bookmarks creates a new
bookmark instead.
BUG=49598
R=sky@chromium.org
Review URL: https://codereview.chromium.org/302313005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275676 0039d316-1c4b-4281-b951-d872f2087c98
12 files changed, 236 insertions, 70 deletions
diff --git a/chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api.cc b/chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api.cc index 36e0dc6..b6e4c02 100644 --- a/chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api.cc +++ b/chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api.cc @@ -594,7 +594,9 @@ bool BookmarkManagerPrivateDropFunction::RunOnReady() { NOTREACHED() <<"Somehow we're dropping null bookmark data"; return false; } - chrome::DropBookmarks(GetProfile(), *drag_data, drop_parent, drop_index); + const bool copy = false; + chrome::DropBookmarks( + GetProfile(), *drag_data, drop_parent, drop_index, copy); router->ClearBookmarkNodeData(); return true; diff --git a/chrome/browser/ui/bookmarks/bookmark_drag_drop.cc b/chrome/browser/ui/bookmarks/bookmark_drag_drop.cc index ba5f05d..79e5543 100644 --- a/chrome/browser/ui/bookmarks/bookmark_drag_drop.cc +++ b/chrome/browser/ui/bookmarks/bookmark_drag_drop.cc @@ -8,6 +8,7 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/undo/bookmark_undo_service.h" #include "chrome/browser/undo/bookmark_undo_service_factory.h" +#include "components/bookmarks/browser/bookmark_client.h" #include "components/bookmarks/browser/bookmark_model.h" #include "components/bookmarks/browser/bookmark_node_data.h" #include "components/bookmarks/browser/bookmark_utils.h" @@ -19,7 +20,8 @@ namespace chrome { int DropBookmarks(Profile* profile, const BookmarkNodeData& data, const BookmarkNode* parent_node, - int index) { + int index, + bool copy) { BookmarkModel* model = BookmarkModelFactory::GetForProfile(profile); #if !defined(OS_ANDROID) bookmarks::ScopedGroupBookmarkActions group_drops(model); @@ -27,13 +29,20 @@ int DropBookmarks(Profile* profile, if (data.IsFromProfilePath(profile->GetPath())) { const std::vector<const BookmarkNode*> dragged_nodes = data.GetNodes(model, profile->GetPath()); + DCHECK(model->client()->CanBeEditedByUser(parent_node)); + DCHECK(copy || bookmark_utils::CanAllBeEditedByUser(model->client(), + dragged_nodes)); if (!dragged_nodes.empty()) { - // Drag from same profile. Move nodes. + // Drag from same profile. Copy or move nodes. for (size_t i = 0; i < dragged_nodes.size(); ++i) { - model->Move(dragged_nodes[i], parent_node, index); + if (copy) { + model->Copy(dragged_nodes[i], parent_node, index); + } else { + model->Move(dragged_nodes[i], parent_node, index); + } index = parent_node->GetIndexOf(dragged_nodes[i]) + 1; } - return ui::DragDropTypes::DRAG_MOVE; + return copy ? ui::DragDropTypes::DRAG_COPY : ui::DragDropTypes::DRAG_MOVE; } return ui::DragDropTypes::DRAG_NONE; } diff --git a/chrome/browser/ui/bookmarks/bookmark_drag_drop.h b/chrome/browser/ui/bookmarks/bookmark_drag_drop.h index 1f604b2..fbce748 100644 --- a/chrome/browser/ui/bookmarks/bookmark_drag_drop.h +++ b/chrome/browser/ui/bookmarks/bookmark_drag_drop.h @@ -23,11 +23,14 @@ void DragBookmarks(Profile* profile, ui::DragDropTypes::DragEventSource source); // Drops the bookmark nodes that are in |data| onto |parent_node| at |index|. +// |copy| indicates the source operation: if true then the bookmarks in |data| +// are copied, otherwise they are moved if they belong to the same |profile|. // Returns the drop type used. int DropBookmarks(Profile* profile, const BookmarkNodeData& data, const BookmarkNode* parent_node, - int index); + int index, + bool copy); } // namespace chrome diff --git a/chrome/browser/ui/browser_commands.cc b/chrome/browser/ui/browser_commands.cc index 38cff6b..22c2428 100644 --- a/chrome/browser/ui/browser_commands.cc +++ b/chrome/browser/ui/browser_commands.cc @@ -174,19 +174,22 @@ void BookmarkCurrentPageInternal(Browser* browser) { WebContents* web_contents = browser->tab_strip_model()->GetActiveWebContents(); GetURLAndTitleToBookmark(web_contents, &url, &title); - bool was_bookmarked = model->IsBookmarked(url); - if (!was_bookmarked && web_contents->GetBrowserContext()->IsOffTheRecord()) { + bool is_bookmarked_by_any = model->IsBookmarked(url); + if (!is_bookmarked_by_any && + web_contents->GetBrowserContext()->IsOffTheRecord()) { // If we're incognito the favicon may not have been saved. Save it now // so that bookmarks have an icon for the page. FaviconTabHelper::FromWebContents(web_contents)->SaveFavicon(); } + bool was_bookmarked_by_user = bookmark_utils::IsBookmarkedByUser(model, url); bookmark_utils::AddIfNotBookmarked(model, url, title); + bool is_bookmarked_by_user = bookmark_utils::IsBookmarkedByUser(model, url); // Make sure the model actually added a bookmark before showing the star. A // bookmark isn't created if the url is invalid. - if (browser->window()->IsActive() && model->IsBookmarked(url)) { + if (browser->window()->IsActive() && is_bookmarked_by_user) { // Only show the bubble if the window is active, otherwise we may get into // weird situations where the bubble is deleted as soon as it is shown. - browser->window()->ShowBookmarkBubble(url, was_bookmarked); + browser->window()->ShowBookmarkBubble(url, was_bookmarked_by_user); } } diff --git a/chrome/browser/ui/cocoa/view_id_util_browsertest.mm b/chrome/browser/ui/cocoa/view_id_util_browsertest.mm index 0c191d0..c64695a 100644 --- a/chrome/browser/ui/cocoa/view_id_util_browsertest.mm +++ b/chrome/browser/ui/cocoa/view_id_util_browsertest.mm @@ -72,6 +72,7 @@ class ViewIDTest : public InProcessBrowserTest { i == VIEW_ID_FEEDBACK_BUTTON || i == VIEW_ID_SCRIPT_BUBBLE || i == VIEW_ID_MIC_SEARCH_BUTTON || + i == VIEW_ID_MANAGED_BOOKMARKS || i == VIEW_ID_TRANSLATE_BUTTON) { continue; } diff --git a/chrome/browser/ui/view_ids.h b/chrome/browser/ui/view_ids.h index f5ce29e..ad8739c 100644 --- a/chrome/browser/ui/view_ids.h +++ b/chrome/browser/ui/view_ids.h @@ -62,6 +62,7 @@ enum ViewID { // The Bookmark Bar. VIEW_ID_BOOKMARK_BAR, VIEW_ID_OTHER_BOOKMARKS, + VIEW_ID_MANAGED_BOOKMARKS, // Used for bookmarks/folders on the bookmark bar. VIEW_ID_BOOKMARK_BAR_ELEMENT, diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc index 41cae67..a844393 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc +++ b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc @@ -16,6 +16,7 @@ #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h" +#include "chrome/browser/bookmarks/chrome_bookmark_client.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/defaults.h" @@ -135,6 +136,9 @@ static const int kOtherFolderButtonTag = 1; // Tag for the 'Apps Shortcut' button. static const int kAppsShortcutButtonTag = 2; +// Tag for the 'Managed bookmarks' button. +static const int kManagedFolderButtonTag = 3; + namespace { // To enable/disable BookmarkBar animations during testing. In production @@ -436,9 +440,11 @@ static const gfx::ImageSkia& GetFolderIcon() { BookmarkBarView::BookmarkBarView(Browser* browser, BrowserView* browser_view) : page_navigator_(NULL), model_(NULL), + client_(NULL), bookmark_menu_(NULL), bookmark_drop_menu_(NULL), other_bookmarked_button_(NULL), + managed_bookmarks_button_(NULL), apps_page_shortcut_(NULL), show_folder_method_factory_(this), overflow_button_(NULL), @@ -526,7 +532,13 @@ const BookmarkNode* BookmarkBarView::GetNodeForButtonAtModelIndex( gfx::Point adjusted_loc(GetMirroredXInView(loc.x()), loc.y()); - // Check the buttons first. + // Check the managed button first. + if (managed_bookmarks_button_->visible() && + managed_bookmarks_button_->bounds().Contains(adjusted_loc)) { + return client_->managed_node(); + } + + // Then check the bookmark buttons. for (int i = 0; i < GetBookmarkButtonCount(); ++i) { views::View* child = child_at(i); if (!child->visible()) @@ -553,6 +565,8 @@ const BookmarkNode* BookmarkBarView::GetNodeForButtonAtModelIndex( views::MenuButton* BookmarkBarView::GetMenuButtonForNode( const BookmarkNode* node) { + if (node == client_->managed_node()) + return managed_bookmarks_button_; if (node == model_->other_node()) return other_bookmarked_button_; if (node == model_->bookmark_bar_node()) @@ -696,6 +710,7 @@ gfx::Size BookmarkBarView::GetMinimumSize() const { // The minimum width of the bookmark bar should at least contain the overflow // button, by which one can access all the Bookmark Bar items, and the "Other // Bookmarks" folder, along with appropriate margins and button padding. + // It should also contain the Managed Bookmarks folder, if it's visible. int width = kLeftMargin; int height = chrome::kBookmarkBarHeight; @@ -707,23 +722,26 @@ gfx::Size BookmarkBarView::GetMinimumSize() const { current_state); } - gfx::Size other_bookmarked_pref; - if (other_bookmarked_button_->visible()) - other_bookmarked_pref = other_bookmarked_button_->GetPreferredSize(); - gfx::Size overflow_pref; - if (overflow_button_->visible()) - overflow_pref = overflow_button_->GetPreferredSize(); - gfx::Size bookmarks_separator_pref; - if (bookmarks_separator_view_->visible()) - bookmarks_separator_pref = bookmarks_separator_view_->GetPreferredSize(); - - gfx::Size apps_page_shortcut_pref; - if (apps_page_shortcut_->visible()) - apps_page_shortcut_pref = apps_page_shortcut_->GetPreferredSize(); - width += other_bookmarked_pref.width() + kButtonPadding + - apps_page_shortcut_pref.width() + kButtonPadding + - overflow_pref.width() + kButtonPadding + - bookmarks_separator_pref.width(); + if (managed_bookmarks_button_->visible()) { + gfx::Size size = managed_bookmarks_button_->GetPreferredSize(); + width += size.width() + kButtonPadding; + } + if (other_bookmarked_button_->visible()) { + gfx::Size size = other_bookmarked_button_->GetPreferredSize(); + width += size.width() + kButtonPadding; + } + if (overflow_button_->visible()) { + gfx::Size size = overflow_button_->GetPreferredSize(); + width += size.width() + kButtonPadding; + } + if (bookmarks_separator_view_->visible()) { + gfx::Size size = bookmarks_separator_view_->GetPreferredSize(); + width += size.width(); + } + if (apps_page_shortcut_->visible()) { + gfx::Size size = apps_page_shortcut_->GetPreferredSize(); + width += size.width() + kButtonPadding; + } return gfx::Size(width, height); } @@ -918,8 +936,10 @@ int BookmarkBarView::OnPerformDrop(const DropTargetEvent& event) { } const BookmarkNodeData data = drop_info_->data; DCHECK(data.is_valid()); + bool copy = drop_info_->location.operation == ui::DragDropTypes::DRAG_COPY; drop_info_.reset(); - return chrome::DropBookmarks(browser_->profile(), data, parent_node, index); + return chrome::DropBookmarks( + browser_->profile(), data, parent_node, index, copy); } void BookmarkBarView::OnThemeChanged() { @@ -997,9 +1017,13 @@ void BookmarkBarView::BookmarkModelLoaded(BookmarkModel* model, DCHECK(model_->other_node()); other_bookmarked_button_->SetAccessibleName(model_->other_node()->GetTitle()); other_bookmarked_button_->SetText(model_->other_node()->GetTitle()); + managed_bookmarks_button_->SetAccessibleName( + client_->managed_node()->GetTitle()); + managed_bookmarks_button_->SetText(client_->managed_node()->GetTitle()); UpdateColors(); - UpdateOtherBookmarksVisibility(); + UpdateButtonsVisibility(); other_bookmarked_button_->SetEnabled(true); + managed_bookmarks_button_->SetEnabled(true); Layout(); SchedulePaint(); @@ -1010,6 +1034,7 @@ void BookmarkBarView::BookmarkModelBeingDeleted(BookmarkModel* model) { // Do minimal cleanup, presumably we'll be deleted shortly. model_->RemoveObserver(this); model_ = NULL; + client_ = NULL; } void BookmarkBarView::BookmarkNodeMoved(BookmarkModel* model, @@ -1047,7 +1072,7 @@ void BookmarkBarView::BookmarkNodeRemoved(BookmarkModel* model, void BookmarkBarView::BookmarkAllNodesRemoved( BookmarkModel* model, const std::set<GURL>& removed_urls) { - UpdateOtherBookmarksVisibility(); + UpdateButtonsVisibility(); StopThrobbing(true); @@ -1166,6 +1191,8 @@ void BookmarkBarView::OnMenuButtonClicked(views::View* view, int start_index = 0; if (view == other_bookmarked_button_) { node = model_->other_node(); + } else if (view == managed_bookmarks_button_) { + node = client_->managed_node(); } else if (view == overflow_button_) { node = model_->bookmark_bar_node(); start_index = GetFirstHiddenNodeIndex(); @@ -1201,6 +1228,8 @@ void BookmarkBarView::ButtonPressed(views::Button* sender, const BookmarkNode* node; if (sender->tag() == kOtherFolderButtonTag) { node = model_->other_node(); + } else if (sender->tag() == kManagedFolderButtonTag) { + node = client_->managed_node(); } else { int index = GetIndexOf(sender); DCHECK_NE(-1, index); @@ -1237,6 +1266,9 @@ void BookmarkBarView::ShowContextMenuForView(views::View* source, // Do this so the user can open all bookmarks. BookmarkContextMenu makes // sure the user can't edit/delete the node in this case. nodes.push_back(parent); + } else if (source == managed_bookmarks_button_) { + parent = client_->managed_node(); + nodes.push_back(parent); } else if (source != this && source != apps_page_shortcut_) { // User clicked on one of the bookmark buttons, find which one they // clicked on, except for the apps page shortcut, which must behave as if @@ -1278,6 +1310,11 @@ void BookmarkBarView::Init() { other_bookmarked_button_->SetEnabled(false); AddChildView(other_bookmarked_button_); + managed_bookmarks_button_ = CreateOtherBookmarkedButton(); + // Also re-enabled when the model is loaded. + managed_bookmarks_button_->SetEnabled(false); + AddChildView(managed_bookmarks_button_); + apps_page_shortcut_ = CreateAppsPageShortcutButton(); AddChildView(apps_page_shortcut_); profile_pref_registrar_.Init(browser_->profile()->GetPrefs()); @@ -1300,8 +1337,10 @@ void BookmarkBarView::Init() { size_animation_.reset(new gfx::SlideAnimation(this)); - model_ = BookmarkModelFactory::GetForProfile(browser_->profile()); - if (model_) { + client_ = BookmarkModelFactory::GetChromeBookmarkClientForProfile( + browser_->profile()); + if (client_) { + model_ = client_->model(); model_->AddObserver(this); if (model_->loaded()) BookmarkModelLoaded(model_, false); @@ -1311,9 +1350,10 @@ void BookmarkBarView::Init() { } int BookmarkBarView::GetBookmarkButtonCount() const { - // We contain four non-bookmark button views: other bookmarks, bookmarks - // separator, chevrons (for overflow), apps page, and the instruction label. - return child_count() - 5; + // We contain six non-bookmark button views: managed bookmarks, + // other bookmarks, bookmarks separator, chevrons (for overflow), apps page, + // and the instruction label. + return child_count() - 6; } views::TextButton* BookmarkBarView::GetBookmarkButton(int index) { @@ -1346,6 +1386,19 @@ MenuButton* BookmarkBarView::CreateOtherBookmarkedButton() { return button; } +MenuButton* BookmarkBarView::CreateManagedBookmarksButton() { + // Title is set in Loaded. + MenuButton* button = + new BookmarkFolderButton(this, base::string16(), this, false); + button->set_id(VIEW_ID_MANAGED_BOOKMARKS); + // TODO(joaodasilva): replace with a "managed folder" icon. + // http://crbug.com/49598 + button->SetIcon(GetFolderIcon()); + button->set_context_menu_controller(this); + button->set_tag(kManagedFolderButtonTag); + return button; +} + MenuButton* BookmarkBarView::CreateOverflowButton() { ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance(); MenuButton* button = new OverFlowButton(this); @@ -1423,7 +1476,7 @@ void BookmarkBarView::ConfigureButton(const BookmarkNode* node, void BookmarkBarView::BookmarkNodeAddedImpl(BookmarkModel* model, const BookmarkNode* parent, int index) { - UpdateOtherBookmarksVisibility(); + UpdateButtonsVisibility(); if (parent != model_->bookmark_bar_node()) { // We only care about nodes on the bookmark bar. return; @@ -1443,7 +1496,7 @@ void BookmarkBarView::BookmarkNodeAddedImpl(BookmarkModel* model, void BookmarkBarView::BookmarkNodeRemovedImpl(BookmarkModel* model, const BookmarkNode* parent, int index) { - UpdateOtherBookmarksVisibility(); + UpdateButtonsVisibility(); StopThrobbing(true); // No need to start throbbing again as the bookmark bubble can't be up at @@ -1553,7 +1606,8 @@ void BookmarkBarView::CalculateDropLocation(const DropTargetEvent& event, } else if (!GetBookmarkButtonCount()) { // No bookmarks, accept the drop. location->index = 0; - int ops = data.GetFirstNode(model_, profile->GetPath()) ? + const BookmarkNode* node = data.GetFirstNode(model_, profile->GetPath()); + int ops = node && client_->CanBeEditedByUser(node) ? ui::DragDropTypes::DRAG_MOVE : ui::DragDropTypes::DRAG_COPY | ui::DragDropTypes::DRAG_LINK; location->operation = chrome::GetPreferredBookmarkDropOperation( @@ -1659,6 +1713,8 @@ void BookmarkBarView::StartThrobbing(const BookmarkNode* node, } else if (!overflow_only) { throbbing_view_ = static_cast<CustomButton*>(child_at(index)); } + } else if (client_->IsDescendantOfManagedNode(node)) { + throbbing_view_ = managed_bookmarks_button_; } else if (!overflow_only) { throbbing_view_ = other_bookmarked_button_; } @@ -1689,6 +1745,8 @@ views::CustomButton* BookmarkBarView::DetermineViewToThrobFromRemove( } return static_cast<CustomButton*>(child_at(old_index_on_bb)); } + if (client_->IsDescendantOfManagedNode(parent)) + return managed_bookmarks_button_; // Node wasn't on the bookmark bar, use the other bookmark button. return other_bookmarked_button_; } @@ -1702,19 +1760,30 @@ void BookmarkBarView::UpdateColors() { theme_provider->GetColor(ThemeProperties::COLOR_BOOKMARK_TEXT); for (int i = 0; i < GetBookmarkButtonCount(); ++i) GetBookmarkButton(i)->SetEnabledColor(text_color); - other_bookmarked_button()->SetEnabledColor(text_color); + other_bookmarked_button_->SetEnabledColor(text_color); + managed_bookmarks_button_->SetEnabledColor(text_color); if (apps_page_shortcut_->visible()) apps_page_shortcut_->SetEnabledColor(text_color); } -void BookmarkBarView::UpdateOtherBookmarksVisibility() { +void BookmarkBarView::UpdateButtonsVisibility() { bool has_other_children = !model_->other_node()->empty(); - if (has_other_children == other_bookmarked_button_->visible()) - return; - other_bookmarked_button_->SetVisible(has_other_children); - UpdateBookmarksSeparatorVisibility(); - Layout(); - SchedulePaint(); + bool update_other = has_other_children != other_bookmarked_button_->visible(); + if (update_other) { + other_bookmarked_button_->SetVisible(has_other_children); + UpdateBookmarksSeparatorVisibility(); + } + + bool has_managed_children = !client_->managed_node()->empty(); + bool update_managed = + has_managed_children != managed_bookmarks_button_->visible(); + if (update_managed) + managed_bookmarks_button_->SetVisible(has_managed_children); + + if (update_other || update_managed) { + Layout(); + SchedulePaint(); + } } void BookmarkBarView::UpdateBookmarksSeparatorVisibility() { @@ -1763,7 +1832,7 @@ void BookmarkBarView::LayoutItems() { max_x -= other_bookmarked_pref.width() + kButtonPadding; // Next, layout out the buttons. Any buttons that are placed beyond the - // visible region and made invisible. + // visible region are made invisible. // Start with the apps page shortcut button. if (apps_page_shortcut_->visible()) { @@ -1772,6 +1841,15 @@ void BookmarkBarView::LayoutItems() { x += apps_page_shortcut_pref.width() + kButtonPadding; } + // Then comes the managed bookmarks folder, if visible. + if (managed_bookmarks_button_->visible()) { + gfx::Size managed_bookmarks_pref = managed_bookmarks_button_->visible() ? + managed_bookmarks_button_->GetPreferredSize() : gfx::Size(); + managed_bookmarks_button_->SetBounds(x, y, managed_bookmarks_pref.width(), + height); + x += managed_bookmarks_pref.width() + kButtonPadding; + } + // Then go through the bookmark buttons. if (GetBookmarkButtonCount() == 0 && model_ && model_->loaded()) { gfx::Size pref = instructions_->GetPreferredSize(); diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bar_view.h b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.h index b8e98b6b..b572d13 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_bar_view.h +++ b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.h @@ -31,6 +31,7 @@ class BookmarkContextMenu; class Browser; class BrowserView; +class ChromeBookmarkClient; class Profile; namespace content { @@ -301,6 +302,9 @@ class BookmarkBarView : public DetachableToolbarView, // Creates the button showing the other bookmarked items. views::MenuButton* CreateOtherBookmarkedButton(); + // Creates the button showing the managed bookmarks items. + views::MenuButton* CreateManagedBookmarksButton(); + // Creates the button used when not all bookmark buttons fit. views::MenuButton* CreateOverflowButton(); @@ -362,9 +366,9 @@ class BookmarkBarView : public DetachableToolbarView, // Updates the colors for all the child objects in the bookmarks bar. void UpdateColors(); - // Updates the visibility of |other_bookmarked_button_|. Also shows or hide - // the separator if required. - void UpdateOtherBookmarksVisibility(); + // Updates the visibility of |other_bookmarked_button_| and + // |managed_bookmarks_button_|. Also shows or hides the separator if required. + void UpdateButtonsVisibility(); // Updates the visibility of |bookmarks_separator_view_|. void UpdateBookmarksSeparatorVisibility(); @@ -385,6 +389,9 @@ class BookmarkBarView : public DetachableToolbarView, // shown. This is owned by the Profile. BookmarkModel* model_; + // The ChromeBookmarkClient that owns the |model_|. + ChromeBookmarkClient* client_; + // Used to manage showing a Menu, either for the most recently bookmarked // entries, or for the starred folder. BookmarkMenuController* bookmark_menu_; @@ -401,6 +408,9 @@ class BookmarkBarView : public DetachableToolbarView, // Shows the other bookmark entries. views::MenuButton* other_bookmarked_button_; + // Shows the managed bookmarks entries. + views::MenuButton* managed_bookmarks_button_; + // Shows the Apps page shortcut. views::TextButton* apps_page_shortcut_; diff --git a/chrome/browser/ui/views/bookmarks/bookmark_drag_drop_views.cc b/chrome/browser/ui/views/bookmarks/bookmark_drag_drop_views.cc index 5c1c3ed..557a5a6 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_drag_drop_views.cc +++ b/chrome/browser/ui/views/bookmarks/bookmark_drag_drop_views.cc @@ -12,6 +12,7 @@ #include "chrome/common/pref_names.h" #include "components/bookmarks/browser/bookmark_model.h" #include "components/bookmarks/browser/bookmark_node_data.h" +#include "components/bookmarks/browser/bookmark_utils.h" #include "components/user_prefs/user_prefs.h" #include "ui/base/dragdrop/drag_drop_types.h" #include "ui/base/dragdrop/os_exchange_data.h" @@ -27,7 +28,7 @@ void DragBookmarks(Profile* profile, ui::DragDropTypes::DragEventSource source) { DCHECK(!nodes.empty()); - // Set up our OLE machinery + // Set up our OLE machinery. ui::OSExchangeData data; BookmarkNodeData drag_data(nodes); drag_data.Write(profile->GetPath(), &data); @@ -36,9 +37,11 @@ void DragBookmarks(Profile* profile, bool was_nested = base::MessageLoop::current()->IsNested(); base::MessageLoop::current()->SetNestableTasksAllowed(true); - int operation = ui::DragDropTypes::DRAG_COPY | - ui::DragDropTypes::DRAG_MOVE | - ui::DragDropTypes::DRAG_LINK; + int operation = ui::DragDropTypes::DRAG_COPY | ui::DragDropTypes::DRAG_LINK; + BookmarkModel* model = BookmarkModelFactory::GetForProfile(profile); + if (bookmark_utils::CanAllBeEditedByUser(model->client(), nodes)) + operation |= ui::DragDropTypes::DRAG_MOVE; + views::Widget* widget = views::Widget::GetWidgetForNativeView(view); if (widget) { @@ -55,10 +58,14 @@ void DragBookmarks(Profile* profile, int GetBookmarkDragOperation(content::BrowserContext* browser_context, const BookmarkNode* node) { PrefService* prefs = user_prefs::UserPrefs::Get(browser_context); + Profile* profile = Profile::FromBrowserContext(browser_context); + BookmarkModel* model = BookmarkModelFactory::GetForProfile(profile); int move = ui::DragDropTypes::DRAG_MOVE; - if (!prefs->GetBoolean(prefs::kEditBookmarksEnabled)) + if (!prefs->GetBoolean(prefs::kEditBookmarksEnabled) || + !model->client()->CanBeEditedByUser(node)) { move = ui::DragDropTypes::DRAG_NONE; + } if (node->is_url()) return ui::DragDropTypes::DRAG_COPY | ui::DragDropTypes::DRAG_LINK | move; return ui::DragDropTypes::DRAG_COPY | move; @@ -91,10 +98,21 @@ int GetBookmarkDropOperation(Profile* profile, if (!IsValidBookmarkDropLocation(profile, data, parent, index)) return ui::DragDropTypes::DRAG_NONE; - if (data.GetFirstNode(BookmarkModelFactory::GetForProfile(profile), - profile_path)) - // User is dragging from this profile: move. + BookmarkModel* model = BookmarkModelFactory::GetForProfile(profile); + if (!model->client()->CanBeEditedByUser(parent)) + return ui::DragDropTypes::DRAG_NONE; + + const BookmarkNode* dragged_node = + data.GetFirstNode(model, profile->GetPath()); + if (dragged_node) { + // User is dragging from this profile. + if (!model->client()->CanBeEditedByUser(dragged_node)) { + // Do a copy instead of a move when dragging bookmarks that the user can't + // modify. + return ui::DragDropTypes::DRAG_COPY; + } return ui::DragDropTypes::DRAG_MOVE; + } // User is dragging from another app, copy. return GetPreferredBookmarkDropOperation(event.source_operations(), @@ -113,10 +131,13 @@ bool IsValidBookmarkDropLocation(Profile* profile, if (!data.is_valid()) return false; + BookmarkModel* model = BookmarkModelFactory::GetForProfile(profile); + if (!model->client()->CanBeEditedByUser(drop_parent)) + return false; + const base::FilePath& profile_path = profile->GetPath(); if (data.IsFromProfilePath(profile_path)) { - std::vector<const BookmarkNode*> nodes = data.GetNodes( - BookmarkModelFactory::GetForProfile(profile), profile_path); + std::vector<const BookmarkNode*> nodes = data.GetNodes(model, profile_path); for (size_t i = 0; i < nodes.size(); ++i) { // Don't allow the drop if the user is attempting to drop on one of the // nodes being dragged. @@ -132,7 +153,7 @@ bool IsValidBookmarkDropLocation(Profile* profile, } return true; } - // From the same profile, always accept. + // From another profile, always accept. return true; } diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc index d9be571..4730052 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc +++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc @@ -72,9 +72,12 @@ BookmarkEditorView::BookmarkEditorView( title_tf_(NULL), parent_(parent), details_(details), + bb_model_(BookmarkModelFactory::GetForProfile(profile)), running_menu_for_root_(false), show_tree_(configuration == SHOW_TREE) { DCHECK(profile); + DCHECK(bb_model_); + DCHECK(bb_model_->client()->CanBeEditedByUser(parent)); Init(); } @@ -262,8 +265,6 @@ void BookmarkEditorView::ShowContextMenuForView( } void BookmarkEditorView::Init() { - bb_model_ = BookmarkModelFactory::GetForProfile(profile_); - DCHECK(bb_model_); bb_model_->AddObserver(this); title_label_ = new views::Label( @@ -507,7 +508,8 @@ void BookmarkEditorView::CreateNodes(const BookmarkNode* bb_node, BookmarkEditorView::EditorNode* b_node) { for (int i = 0; i < bb_node->child_count(); ++i) { const BookmarkNode* child_bb_node = bb_node->GetChild(i); - if (child_bb_node->IsVisible() && child_bb_node->is_folder()) { + if (child_bb_node->IsVisible() && child_bb_node->is_folder() && + bb_model_->client()->CanBeEditedByUser(child_bb_node)) { EditorNode* new_b_node = new EditorNode(child_bb_node->GetTitle(), child_bb_node->id()); b_node->Add(new_b_node, b_node->child_count()); diff --git a/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc b/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc index a7aa7da..876a6aa 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc +++ b/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc @@ -7,6 +7,7 @@ #include "base/prefs/pref_service.h" #include "base/strings/utf_string_conversions.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h" +#include "chrome/browser/bookmarks/chrome_bookmark_client.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/bookmarks/bookmark_drag_drop.h" #include "chrome/browser/ui/bookmarks/bookmark_utils.h" @@ -71,12 +72,23 @@ void BookmarkMenuDelegate::Init(views::MenuDelegate* real_delegate, location_ = location; if (parent) { parent_menu_item_ = parent; + + // Add a separator if there are existing items in the menu, and if the + // current node has children. If |node| is the bookmark bar then the + // managed node is shown as its first child, if it's not empty. + BookmarkModel* model = GetBookmarkModel(); + ChromeBookmarkClient* client = GetChromeBookmarkClient(); + bool show_managed = show_options == SHOW_PERMANENT_FOLDERS && + node == model->bookmark_bar_node() && + !client->managed_node()->empty(); + bool has_children = + (start_child_index < node->child_count()) || show_managed; int initial_count = parent->GetSubmenu() ? parent->GetSubmenu()->GetMenuItemCount() : 0; - if ((start_child_index < node->child_count()) && - (initial_count > 0)) { + if (has_children && initial_count > 0) parent->AppendSeparator(); - } + if (show_managed) + BuildMenuForManagedNode(parent, &next_menu_id_); BuildMenu(node, start_child_index, parent, &next_menu_id_); if (show_options == SHOW_PERMANENT_FOLDERS) BuildMenusForPermanentNodes(parent, &next_menu_id_); @@ -95,6 +107,10 @@ BookmarkModel* BookmarkMenuDelegate::GetBookmarkModel() { return BookmarkModelFactory::GetForProfile(profile_); } +ChromeBookmarkClient* BookmarkMenuDelegate::GetChromeBookmarkClient() { + return BookmarkModelFactory::GetChromeBookmarkClientForProfile(profile_); +} + void BookmarkMenuDelegate::SetActiveMenu(const BookmarkNode* node, int start_index) { DCHECK(!parent_menu_item_); @@ -270,8 +286,9 @@ int BookmarkMenuDelegate::OnPerformDrop( break; } + bool copy = event.source_operations() == ui::DragDropTypes::DRAG_COPY; return chrome::DropBookmarks(profile_, drop_data_, - drop_parent, index_to_drop_at); + drop_parent, index_to_drop_at, copy); } bool BookmarkMenuDelegate::ShowContextMenu(MenuItemView* source, @@ -409,8 +426,11 @@ MenuItemView* BookmarkMenuDelegate::CreateMenu(const BookmarkNode* parent, menu->SetCommand(next_menu_id_++); menu_id_to_node_map_[menu->GetCommand()] = parent; menu->set_has_icons(true); + bool show_permanent = show_options == SHOW_PERMANENT_FOLDERS; + if (show_permanent && parent == GetBookmarkModel()->bookmark_bar_node()) + BuildMenuForManagedNode(menu, &next_menu_id_); BuildMenu(parent, start_child_index, menu, &next_menu_id_); - if (show_options == SHOW_PERMANENT_FOLDERS) + if (show_permanent) BuildMenusForPermanentNodes(menu, &next_menu_id_); return menu; } @@ -453,6 +473,17 @@ void BookmarkMenuDelegate::BuildMenuForPermanentNode( menu_id_to_node_map_[id] = node; } +void BookmarkMenuDelegate::BuildMenuForManagedNode( + MenuItemView* menu, + int* next_menu_id) { + // Don't add a separator for this menu. + bool added_separator = true; + const BookmarkNode* node = GetChromeBookmarkClient()->managed_node(); + // TODO(joaodasilva): use the "managed bookmark folder" icon here. + // http://crbug.com/49598 + BuildMenuForPermanentNode(node, menu, next_menu_id, &added_separator); +} + void BookmarkMenuDelegate::BuildMenu(const BookmarkNode* parent, int start_child_index, MenuItemView* menu, diff --git a/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.h b/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.h index 3d07b1e..609531d 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.h +++ b/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.h @@ -17,6 +17,7 @@ class BookmarkNode; class Browser; +class ChromeBookmarkClient; class Profile; namespace content { @@ -76,6 +77,7 @@ class BookmarkMenuDelegate : public BaseBookmarkModelObserver, void SetActiveMenu(const BookmarkNode* node, int start_index); BookmarkModel* GetBookmarkModel(); + ChromeBookmarkClient* GetChromeBookmarkClient(); // Returns the menu. views::MenuItemView* menu() { return menu_; } @@ -152,6 +154,9 @@ class BookmarkMenuDelegate : public BaseBookmarkModelObserver, int* next_menu_id, bool* added_separator); + void BuildMenuForManagedNode(views::MenuItemView* menu, + int* next_menu_id); + // Creates an entry in menu for each child node of |parent| starting at // |start_child_index|. void BuildMenu(const BookmarkNode* parent, |