Commit 0ea81e6e authored by Marti Wong's avatar Marti Wong Committed by Commit Bot

Implements the 'Open All' on context menu of the new iOS bookmark ui.

Demo: https://drive.google.com/open?id=0B1O0Z7eoZMuGbjM3TnJ3eVRoYmM

Bug: 695749
Change-Id: Id1cf2cb9877ccedf12a2f76dffa671bc3ae88439
Reviewed-on: https://chromium-review.googlesource.com/658164
Commit-Queue: Marti Wong <martiw@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Reviewed-by: default avatarRamya Sharma <ramyasharma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501601}
parent 5630fe0b
...@@ -26,11 +26,12 @@ class BookmarkNode; ...@@ -26,11 +26,12 @@ class BookmarkNode;
@class BookmarkHomeViewController; @class BookmarkHomeViewController;
@protocol BookmarkHomeViewControllerDelegate @protocol BookmarkHomeViewControllerDelegate
// The view controller wants to be dismissed. // The view controller wants to be dismissed. If |urls| is not empty, then
// If |url| != GURL(), then the user has selected |url| for navigation. // the user has selected to navigate to those URLs.
- (void)bookmarkHomeViewControllerWantsDismissal: - (void)bookmarkHomeViewControllerWantsDismissal:
(BookmarkHomeViewController*)controller (BookmarkHomeViewController*)controller
navigationToUrl:(const GURL&)url; navigationToUrls:(const std::vector<GURL>&)urls;
@end @end
// Class to navigate the bookmark hierarchy, needs subclassing for tablet / // Class to navigate the bookmark hierarchy, needs subclassing for tablet /
......
...@@ -53,7 +53,18 @@ const CGFloat kMenuWidth = 264; ...@@ -53,7 +53,18 @@ const CGFloat kMenuWidth = 264;
// The spacer between title and done button on the navigation bar. // The spacer between title and done button on the navigation bar.
const CGFloat kSpacer = 50; const CGFloat kSpacer = 50;
// Returns a vector of all URLs in |nodes|.
std::vector<GURL> GetUrlsToOpen(const std::vector<const BookmarkNode*>& nodes) {
std::vector<GURL> urls;
for (const BookmarkNode* node : nodes) {
if (node->is_url()) {
urls.push_back(node->url());
}
}
return urls;
} }
} // namespace
@interface BookmarkHomeViewController ()< @interface BookmarkHomeViewController ()<
BookmarkCollectionViewDelegate, BookmarkCollectionViewDelegate,
...@@ -339,6 +350,12 @@ const CGFloat kSpacer = 50; ...@@ -339,6 +350,12 @@ const CGFloat kSpacer = 50;
[self presentViewController:navController animated:YES completion:NULL]; [self presentViewController:navController animated:YES completion:NULL];
} }
- (void)openAllNodes:(const std::vector<const bookmarks::BookmarkNode*>&)nodes {
std::vector<GURL> urls = GetUrlsToOpen(nodes);
[self.homeDelegate bookmarkHomeViewControllerWantsDismissal:self
navigationToUrls:urls];
}
#pragma mark - Navigation Bar Callbacks #pragma mark - Navigation Bar Callbacks
- (void)navigationBarCancel:(id)sender { - (void)navigationBarCancel:(id)sender {
...@@ -437,9 +454,7 @@ const CGFloat kSpacer = 50; ...@@ -437,9 +454,7 @@ const CGFloat kSpacer = 50;
BOOL foundURL = NO; BOOL foundURL = NO;
BOOL foundFolder = NO; BOOL foundFolder = NO;
for (std::set<const bookmarks::BookmarkNode*>::iterator i = nodes.begin(); for (const BookmarkNode* node : nodes) {
i != nodes.end(); ++i) {
const bookmarks::BookmarkNode* node = *i;
if (!foundURL && node->is_url()) { if (!foundURL && node->is_url()) {
foundURL = YES; foundURL = YES;
} else if (!foundFolder && node->is_folder()) { } else if (!foundFolder && node->is_folder()) {
...@@ -1024,8 +1039,11 @@ const CGFloat kSpacer = 50; ...@@ -1024,8 +1039,11 @@ const CGFloat kSpacer = 50;
- (void)dismissWithURL:(const GURL&)url { - (void)dismissWithURL:(const GURL&)url {
[self cachePosition]; [self cachePosition];
if (self.homeDelegate) { if (self.homeDelegate) {
std::vector<GURL> urls;
if (url.is_valid())
urls.push_back(url);
[self.homeDelegate bookmarkHomeViewControllerWantsDismissal:self [self.homeDelegate bookmarkHomeViewControllerWantsDismissal:self
navigationToUrl:url]; navigationToUrls:urls];
} else { } else {
// Before passing the URL to the block, make sure the block has a copy of // Before passing the URL to the block, make sure the block has a copy of
// the URL and not just a reference. // the URL and not just a reference.
...@@ -1285,7 +1303,11 @@ const CGFloat kSpacer = 50; ...@@ -1285,7 +1303,11 @@ const CGFloat kSpacer = 50;
UIAlertAction* openAllAction = [UIAlertAction UIAlertAction* openAllAction = [UIAlertAction
actionWithTitle:l10n_util::GetNSString(IDS_IOS_BOOKMARK_CONTEXT_MENU_OPEN) actionWithTitle:l10n_util::GetNSString(IDS_IOS_BOOKMARK_CONTEXT_MENU_OPEN)
style:UIAlertActionStyleDefault style:UIAlertActionStyleDefault
handler:nil]; handler:^(UIAlertAction* _Nonnull action) {
std::vector<const BookmarkNode*> nodes =
[weakSelf.bookmarksTableView getEditNodesInVector];
[weakSelf openAllNodes:nodes];
}];
UIAlertAction* openInIncognitoAction = [UIAlertAction UIAlertAction* openInIncognitoAction = [UIAlertAction
actionWithTitle:l10n_util::GetNSString( actionWithTitle:l10n_util::GetNSString(
......
...@@ -240,17 +240,39 @@ using bookmarks::BookmarkNode; ...@@ -240,17 +240,39 @@ using bookmarks::BookmarkNode;
#pragma mark - BookmarkHomeViewControllerDelegate #pragma mark - BookmarkHomeViewControllerDelegate
- (void)bookmarkHomeViewControllerWantsDismissal: - (void)
(BookmarkHomeViewController*)controller bookmarkHomeViewControllerWantsDismissal:(BookmarkHomeViewController*)controller
navigationToUrl:(const GURL&)url { navigationToUrls:(const std::vector<GURL>&)urls {
[self dismissBookmarkBrowserAnimated:YES]; [self dismissBookmarkBrowserAnimated:YES];
if (url != GURL()) { if (urls.empty())
return;
BOOL openInCurrentTab = YES;
for (const GURL& url : urls) {
DCHECK(url.is_valid());
if (openInCurrentTab) {
// Only open the first URL in the current tab.
openInCurrentTab = NO;
// TODO(crbug.com/695749): See if we need different metrics for 'Open
// All'.
new_tab_page_uma::RecordAction(_browserState, new_tab_page_uma::RecordAction(_browserState,
new_tab_page_uma::ACTION_OPENED_BOOKMARK); new_tab_page_uma::ACTION_OPENED_BOOKMARK);
base::RecordAction( base::RecordAction(
base::UserMetricsAction("MobileBookmarkManagerEntryOpened")); base::UserMetricsAction("MobileBookmarkManagerEntryOpened"));
[self openURLInCurrentTab:url];
} else {
// Open other URLs (if any) in background tabs.
[self openURLInBackgroundTab:url];
}
} // end for
}
#pragma mark - Private
- (void)openURLInCurrentTab:(const GURL&)url {
if (url.SchemeIs(url::kJavaScriptScheme)) { // bookmarklet if (url.SchemeIs(url::kJavaScriptScheme)) { // bookmarklet
NSString* jsToEval = [base::SysUTF8ToNSString(url.GetContent()) NSString* jsToEval = [base::SysUTF8ToNSString(url.GetContent())
stringByRemovingPercentEncoding]; stringByRemovingPercentEncoding];
...@@ -261,7 +283,13 @@ using bookmarks::BookmarkNode; ...@@ -261,7 +283,13 @@ using bookmarks::BookmarkNode;
transition:ui::PAGE_TRANSITION_AUTO_BOOKMARK transition:ui::PAGE_TRANSITION_AUTO_BOOKMARK
rendererInitiated:NO]; rendererInitiated:NO];
} }
} }
- (void)openURLInBackgroundTab:(const GURL&)url {
[_loader webPageOrderedOpen:url
referrer:web::Referrer()
inBackground:YES
appendTo:kLastTab];
} }
@end @end
...@@ -95,6 +95,9 @@ class PrefRegistrySyncable; ...@@ -95,6 +95,9 @@ class PrefRegistrySyncable;
// Returns the currently selected edit nodes. // Returns the currently selected edit nodes.
- (const std::set<const bookmarks::BookmarkNode*>&)editNodes; - (const std::set<const bookmarks::BookmarkNode*>&)editNodes;
// Returns a vector of edit nodes.
- (std::vector<const bookmarks::BookmarkNode*>)getEditNodesInVector;
@end @end
#endif // IOS_CHROME_BROWSER_UI_BOOKMARKS_BOOKMARK_TABLE_VIEW_H_ #endif // IOS_CHROME_BROWSER_UI_BOOKMARKS_BOOKMARK_TABLE_VIEW_H_
...@@ -252,6 +252,21 @@ using IntegerPair = std::pair<NSInteger, NSInteger>; ...@@ -252,6 +252,21 @@ using IntegerPair = std::pair<NSInteger, NSInteger>;
return _editNodes; return _editNodes;
} }
- (std::vector<const bookmarks::BookmarkNode*>)getEditNodesInVector {
// Create a vector of edit nodes in the same order as the nodes in folder.
// TODO(crbug.com/695749): Check if this opening order is our desired
// behavior.
std::vector<const bookmarks::BookmarkNode*> nodes;
int childCount = _currentRootNode->child_count();
for (int i = 0; i < childCount; ++i) {
const BookmarkNode* node = _currentRootNode->GetChild(i);
if (_editNodes.find(node) != _editNodes.end()) {
nodes.push_back(node);
}
}
return nodes;
}
#pragma mark - UITableViewDataSource #pragma mark - UITableViewDataSource
- (NSInteger)numberOfSectionsInTableView:(UITableView*)tableView { - (NSInteger)numberOfSectionsInTableView:(UITableView*)tableView {
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "ios/chrome/grit/ios_strings.h" #include "ios/chrome/grit/ios_strings.h"
#import "ios/chrome/test/app/bookmarks_test_util.h" #import "ios/chrome/test/app/bookmarks_test_util.h"
#import "ios/chrome/test/app/chrome_test_util.h" #import "ios/chrome/test/app/chrome_test_util.h"
#import "ios/chrome/test/app/tab_test_util.h"
#import "ios/chrome/test/earl_grey/accessibility_util.h" #import "ios/chrome/test/earl_grey/accessibility_util.h"
#import "ios/chrome/test/earl_grey/chrome_earl_grey.h" #import "ios/chrome/test/earl_grey/chrome_earl_grey.h"
#import "ios/chrome/test/earl_grey/chrome_earl_grey_ui.h" #import "ios/chrome/test/earl_grey/chrome_earl_grey_ui.h"
...@@ -46,6 +47,24 @@ using chrome_test_util::PrimarySignInButton; ...@@ -46,6 +47,24 @@ using chrome_test_util::PrimarySignInButton;
using chrome_test_util::SecondarySignInButton; using chrome_test_util::SecondarySignInButton;
namespace { namespace {
// GURL for the testing bookmark "First URL".
const GURL getFirstURL() {
return web::test::HttpServer::MakeUrl(
"http://ios/testing/data/http_server_files/pony.html");
}
// GURL for the testing bookmark "Second URL".
const GURL getSecondURL() {
return web::test::HttpServer::MakeUrl(
"http://ios/testing/data/http_server_files/destination.html");
}
// GURL for the testing bookmark "French URL".
const GURL getFrenchURL() {
return web::test::HttpServer::MakeUrl("http://www.a.fr/");
}
// Matcher for the Delete button on the bookmarks UI. // Matcher for the Delete button on the bookmarks UI.
id<GREYMatcher> BookmarksDeleteSwipeButton() { id<GREYMatcher> BookmarksDeleteSwipeButton() {
return ButtonWithAccessibilityLabelId(IDS_IOS_BOOKMARK_ACTION_DELETE); return ButtonWithAccessibilityLabelId(IDS_IOS_BOOKMARK_ACTION_DELETE);
...@@ -582,6 +601,66 @@ id<GREYMatcher> ContextBarTrailingButtonWithLabel(NSString* label) { ...@@ -582,6 +601,66 @@ id<GREYMatcher> ContextBarTrailingButtonWithLabel(NSString* label) {
assertWithMatcher:grey_sufficientlyVisible()]; assertWithMatcher:grey_sufficientlyVisible()];
} }
- (void)testContextMenuForMultipleURLOpenAll {
if (IsIPadIdiom()) {
EARL_GREY_TEST_DISABLED(@"Test disabled on iPad.");
}
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kBookmarkNewGeneration);
[BookmarksNewGenTestCase setupStandardBookmarks];
[BookmarksNewGenTestCase openBookmarks];
[BookmarksNewGenTestCase openMobileBookmarks];
// Change to edit mode
[[EarlGrey selectElementWithMatcher:grey_accessibilityID(
@"context_bar_trailing_button")]
performAction:grey_tap()];
// Select URLs.
[[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"First URL")]
performAction:grey_tap()];
[[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Second URL")]
performAction:grey_tap()];
[[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"French URL")]
performAction:grey_tap()];
// Tap context menu.
[[EarlGrey selectElementWithMatcher:ContextBarCenterButtonWithLabel(
[BookmarksNewGenTestCase
contextBarMoreString])]
performAction:grey_tap()];
// Tap on Open All.
[[EarlGrey selectElementWithMatcher:ButtonWithAccessibilityLabelId(
IDS_IOS_BOOKMARK_CONTEXT_MENU_OPEN)]
performAction:grey_tap()];
// Verify there are 3 tabs.
[ChromeEarlGrey waitForMainTabCount:3];
[ChromeEarlGrey waitForIncognitoTabCount:0];
// The following verifies the selected bookmarks are open in the same order as
// in folder.
// Verify "French URL" appears in the omnibox.
[[EarlGrey selectElementWithMatcher:chrome_test_util::OmniboxText(
getFrenchURL().GetContent())]
assertWithMatcher:grey_notNil()];
// Switch to the 2nd Tab and verify "Second URL" appears.
chrome_test_util::SelectTabAtIndexInCurrentMode(1);
[[EarlGrey selectElementWithMatcher:chrome_test_util::OmniboxText(
getSecondURL().GetContent())]
assertWithMatcher:grey_notNil()];
// Switch to the 3rd Tab and verify "First URL" appears.
chrome_test_util::SelectTabAtIndexInCurrentMode(2);
[[EarlGrey selectElementWithMatcher:chrome_test_util::OmniboxText(
getFirstURL().GetContent())]
assertWithMatcher:grey_notNil()];
}
- (void)testContextBarForSingleFolderSelection { - (void)testContextBarForSingleFolderSelection {
if (IsIPadIdiom()) { if (IsIPadIdiom()) {
EARL_GREY_TEST_DISABLED(@"Test disabled on iPad."); EARL_GREY_TEST_DISABLED(@"Test disabled on iPad.");
...@@ -1590,22 +1669,17 @@ id<GREYMatcher> ContextBarTrailingButtonWithLabel(NSString* label) { ...@@ -1590,22 +1669,17 @@ id<GREYMatcher> ContextBarTrailingButtonWithLabel(NSString* label) {
ios::BookmarkModelFactory::GetForBrowserState( ios::BookmarkModelFactory::GetForBrowserState(
chrome_test_util::GetOriginalBrowserState()); chrome_test_util::GetOriginalBrowserState());
const GURL firstURL = web::test::HttpServer::MakeUrl(
"http://ios/testing/data/http_server_files/pony.html");
NSString* firstTitle = @"First URL"; NSString* firstTitle = @"First URL";
bookmark_model->AddURL(bookmark_model->mobile_node(), 0, bookmark_model->AddURL(bookmark_model->mobile_node(), 0,
base::SysNSStringToUTF16(firstTitle), firstURL); base::SysNSStringToUTF16(firstTitle), getFirstURL());
const GURL secondURL = web::test::HttpServer::MakeUrl(
"http://ios/testing/data/http_server_files/destination.html");
NSString* secondTitle = @"Second URL"; NSString* secondTitle = @"Second URL";
bookmark_model->AddURL(bookmark_model->mobile_node(), 0, bookmark_model->AddURL(bookmark_model->mobile_node(), 0,
base::SysNSStringToUTF16(secondTitle), secondURL); base::SysNSStringToUTF16(secondTitle), getSecondURL());
const GURL frenchURL = web::test::HttpServer::MakeUrl("http://www.a.fr/");
NSString* frenchTitle = @"French URL"; NSString* frenchTitle = @"French URL";
bookmark_model->AddURL(bookmark_model->mobile_node(), 0, bookmark_model->AddURL(bookmark_model->mobile_node(), 0,
base::SysNSStringToUTF16(frenchTitle), frenchURL); base::SysNSStringToUTF16(frenchTitle), getFrenchURL());
NSString* folderTitle = @"Folder 1"; NSString* folderTitle = @"Folder 1";
const bookmarks::BookmarkNode* folder1 = bookmark_model->AddFolder( const bookmarks::BookmarkNode* folder1 = bookmark_model->AddFolder(
......
...@@ -91,28 +91,39 @@ ...@@ -91,28 +91,39 @@
#pragma mark - BookmarkHomeViewControllerDelegate #pragma mark - BookmarkHomeViewControllerDelegate
- (void)bookmarkHomeViewControllerWantsDismissal: - (void)
(BookmarkHomeViewController*)controller bookmarkHomeViewControllerWantsDismissal:(BookmarkHomeViewController*)controller
navigationToUrl:(const GURL&)url { navigationToUrls:(const std::vector<GURL>&)urls {
// TODO(crbug.com/695749): Test if the following works when adding bookmark
// becomes available in chrome_clean_skeleton.
// TODO(crbug.com/695749): See if we can share the code below from
// bookmark_home_view_controller.
[self dismissBookmarkBrowser]; [self dismissBookmarkBrowser];
if (url != GURL()) { if (urls.empty())
return;
BOOL openInCurrentTab = YES;
for (const GURL& url : urls) {
DCHECK(url.is_valid());
if (openInCurrentTab) {
// Only open the first URL in the current tab.
openInCurrentTab = NO;
// TODO(crbug.com/695749): See if we need different metrics for 'Open
// All'.
new_tab_page_uma::RecordAction(self.browser->browser_state(), new_tab_page_uma::RecordAction(self.browser->browser_state(),
new_tab_page_uma::ACTION_OPENED_BOOKMARK); new_tab_page_uma::ACTION_OPENED_BOOKMARK);
base::RecordAction( base::RecordAction(
base::UserMetricsAction("MobileBookmarkManagerEntryOpened")); base::UserMetricsAction("MobileBookmarkManagerEntryOpened"));
if (url.SchemeIs(url::kJavaScriptScheme)) { // bookmarklet [self openURLInCurrentTab:url];
NSString* jsToEval = [base::SysUTF8ToNSString(url.GetContent())
stringByRemovingPercentEncoding];
[self.loader loadJavaScriptFromLocationBar:jsToEval];
} else { } else {
[self.loader loadURL:url // Open other URLs (if any) in background tabs.
referrer:web::Referrer() [self openURLInBackgroundTab:url];
transition:ui::PAGE_TRANSITION_AUTO_BOOKMARK
rendererInitiated:NO];
}
} }
} // end for
} }
#pragma mark - Private #pragma mark - Private
...@@ -126,4 +137,24 @@ ...@@ -126,4 +137,24 @@
[self stop]; [self stop];
} }
- (void)openURLInCurrentTab:(const GURL&)url {
if (url.SchemeIs(url::kJavaScriptScheme)) { // bookmarklet
NSString* jsToEval = [base::SysUTF8ToNSString(url.GetContent())
stringByRemovingPercentEncoding];
[_loader loadJavaScriptFromLocationBar:jsToEval];
} else {
[_loader loadURL:url
referrer:web::Referrer()
transition:ui::PAGE_TRANSITION_AUTO_BOOKMARK
rendererInitiated:NO];
}
}
- (void)openURLInBackgroundTab:(const GURL&)url {
[_loader webPageOrderedOpen:url
referrer:web::Referrer()
inBackground:YES
appendTo:kLastTab];
}
@end @end
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