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
parent eab49a65
......@@ -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;
......
......@@ -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;
}
......
......@@ -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
......
......@@ -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);
}
}
......
......@@ -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;
}
......
......@@ -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,
......
......@@ -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_;
......
......@@ -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;
}
......
......@@ -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());
......
......@@ -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,
......
......@@ -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,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment