Skip to content

Commit

Permalink
List site in password settings
Browse files Browse the repository at this point in the history
Design doc: go/jgdrc, screenshots https://crbug.com/159166#c39

This also marks a lot of constants indeed as const in the unittest, and removes one leak (allocation of PasswordForm by new without calling delete on it).

BUG=159166

Review-Url: https://codereview.chromium.org/2722853003
Cr-Commit-Position: refs/heads/master@{#468927}
  • Loading branch information
vabr authored and Commit bot committed May 3, 2017
1 parent 2e1c195 commit b9e34e3
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 28 deletions.
9 changes: 9 additions & 0 deletions ios/chrome/app/strings/ios_strings.grd
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,9 @@ Handoff must also be enabled in the General section of Settings, and your device
<message name="IDS_IOS_SEARCH_ENGINE_SETTING_TITLE" desc="The title for Search Engine selection setting [iOS only]">
Search Engine
</message>
<message name="IDS_IOS_SETTINGS_SITE_COPY_BUTTON" desc="Button that the user can press to copy the site to clipboard. [Length: 12em]">
Copy
</message>
<message name="IDS_IOS_SETTINGS_USERNAME_COPY_BUTTON" desc="Button that the user can press to copy the username to clipboard. [Length: 12em]">
Copy
</message>
Expand All @@ -1131,6 +1134,9 @@ Handoff must also be enabled in the General section of Settings, and your device
<message name="IDS_IOS_SETTINGS_PASSWORD_SHOW_BUTTON" desc="Button that the user can press in order to have the password displayed in plain text. [Length: 12em]">
Show
</message>
<message name="IDS_IOS_SETTINGS_SITE_WAS_COPIED_MESSAGE" desc="Confirmation that the site URL of a saved credential was copied. This is shown alone in an infobar at the bottom of the screen. [Length: 40em]">
The site URL was copied.
</message>
<message name="IDS_IOS_SETTINGS_USERNAME_WAS_COPIED_MESSAGE" desc="Confirmation that the username was copied. This is shown alone in an infobar at the bottom of the screen. [Length: 40em]">
Your username was copied.
</message>
Expand Down Expand Up @@ -1197,6 +1203,9 @@ Handoff must also be enabled in the General section of Settings, and your device
<message name="IDS_IOS_SHOW_PASSWORD_VIEW_PASSWORD" desc="Label indicating that the text displayed below is a password. [Length: 20em] [iOS only]">
Password
</message>
<message name="IDS_IOS_SHOW_PASSWORD_VIEW_SITE" desc="Label indicating that the text displayed below is the site on which the associated password was saved. The password is displayed in the same view, but under a diferent label. [Length: 20em] [iOS only]">
Site
</message>
<message name="IDS_IOS_SHOW_PASSWORD_VIEW_USERNAME" desc="Label indicating that the text displayed below is the username to which a saved password corresponds. The password is displayed in the same view, but under a diferent label. [Length: 20em] [iOS only]">
Username
</message>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,15 @@
namespace {

typedef NS_ENUM(NSInteger, SectionIdentifier) {
SectionIdentifierUsername = kSectionIdentifierEnumZero,
SectionIdentifierSite = kSectionIdentifierEnumZero,
SectionIdentifierUsername,
SectionIdentifierPassword,
};

typedef NS_ENUM(NSInteger, ItemType) {
ItemTypeHeader = kItemTypeEnumZero,
ItemTypeSite,
ItemTypeCopySite,
ItemTypeUsername,
ItemTypeCopyUsername,
ItemTypePassword,
Expand All @@ -53,6 +56,8 @@ @interface PasswordDetailsCollectionViewController () {
NSString* _username;
// The saved password.
NSString* _password;
// The origin site of the saved credential.
NSString* _site;
// Whether the password is shown in plain text form or in obscured form.
BOOL _plainTextPasswordShown;
// The password form.
Expand Down Expand Up @@ -88,6 +93,7 @@ @implementation PasswordDetailsCollectionViewController
_passwordForm = passwordForm;
_username = [username copy];
_password = [password copy];
_site = base::SysUTF8ToNSString(_passwordForm.origin.spec());
self.title =
[PasswordDetailsCollectionViewController simplifyOrigin:origin];
NSNotificationCenter* defaultCenter = [NSNotificationCenter defaultCenter];
Expand Down Expand Up @@ -119,6 +125,19 @@ - (void)loadModel {
[super loadModel];
CollectionViewModel* model = self.collectionViewModel;

[model addSectionWithIdentifier:SectionIdentifierSite];
CollectionViewTextItem* siteHeader =
[[CollectionViewTextItem alloc] initWithType:ItemTypeHeader];
siteHeader.text = l10n_util::GetNSString(IDS_IOS_SHOW_PASSWORD_VIEW_SITE);
[model setHeader:siteHeader forSectionWithIdentifier:SectionIdentifierSite];
PasswordDetailsItem* siteItem =
[[PasswordDetailsItem alloc] initWithType:ItemTypeSite];
siteItem.text = _site;
siteItem.showingText = YES;
[model addItem:siteItem toSectionWithIdentifier:SectionIdentifierSite];
[model addItem:[self siteCopyButtonItem]
toSectionWithIdentifier:SectionIdentifierSite];

[model addSectionWithIdentifier:SectionIdentifierUsername];
CollectionViewTextItem* usernameHeader =
[[CollectionViewTextItem alloc] initWithType:ItemTypeHeader];
Expand Down Expand Up @@ -166,6 +185,23 @@ - (void)dealloc {

#pragma mark - Items

- (CollectionViewItem*)siteCopyButtonItem {
CollectionViewTextItem* item =
[[CollectionViewTextItem alloc] initWithType:ItemTypeCopySite];
item.text = l10n_util::GetNSString(IDS_IOS_SETTINGS_SITE_COPY_BUTTON);
item.textColor = [[MDCPalette cr_bluePalette] tint500];
// Accessibility label adds the header to the text, so that accessibility
// users do not have to rely on the visual grouping to understand which part
// of the credential is being copied.
item.accessibilityLabel = [NSString
stringWithFormat:@"%@: %@",
l10n_util::GetNSString(IDS_IOS_SHOW_PASSWORD_VIEW_SITE),
l10n_util::GetNSString(
IDS_IOS_SETTINGS_SITE_COPY_BUTTON)];
item.accessibilityTraits |= UIAccessibilityTraitButton;
return item;
}

- (CollectionViewItem*)usernameCopyButtonItem {
CollectionViewTextItem* item =
[[CollectionViewTextItem alloc] initWithType:ItemTypeCopyUsername];
Expand Down Expand Up @@ -222,6 +258,13 @@ - (CollectionViewItem*)deletePasswordButtonItem {

#pragma mark - Actions

- (void)copySite {
UIPasteboard* generalPasteboard = [UIPasteboard generalPasteboard];
generalPasteboard.string = _site;
[self showCopyResultToast:l10n_util::GetNSString(
IDS_IOS_SETTINGS_SITE_WAS_COPIED_MESSAGE)];
}

- (void)copyUsername {
UIPasteboard* generalPasteboard = [UIPasteboard generalPasteboard];
generalPasteboard.string = _username;
Expand Down Expand Up @@ -349,6 +392,9 @@ - (void)collectionView:(UICollectionView*)collectionView
NSInteger itemType =
[self.collectionViewModel itemTypeForIndexPath:indexPath];
switch (itemType) {
case ItemTypeCopySite:
[self copySite];
break;
case ItemTypeCopyUsername:
[self copyUsername];
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,23 @@ - (void)deletePassword:(const autofill::PasswordForm&)passwordForm {

namespace {

NSString* kUsername = @"testusername";
NSString* kPassword = @"testpassword";
NSString* const kSite = @"https://testorigin.com/";
NSString* const kUsername = @"testusername";
NSString* const kPassword = @"testpassword";

int kUsernameSection = 0;
int kUsernameItem = 0;
int kCopyUsernameButtonItem = 1;
const int kSiteSection = 0;
const int kSiteItem = 0;
const int kCopySiteButtonItem = 1;

int kPasswordSection = 1;
int kPasswordItem = 0;
int kShowHideButtonItem = 1;
int kCopyPasswordButtonItem = 2;
int kDeleteButtonItem = 3;
const int kUsernameSection = 1;
const int kUsernameItem = 0;
const int kCopyUsernameButtonItem = 1;

const int kPasswordSection = 2;
const int kPasswordItem = 0;
const int kShowHideButtonItem = 1;
const int kCopyPasswordButtonItem = 2;
const int kDeleteButtonItem = 3;

class PasswordDetailsCollectionViewControllerTest
: public CollectionViewControllerTest {
Expand All @@ -86,14 +91,19 @@ - (void)deletePassword:(const autofill::PasswordForm&)passwordForm {
: thread_bundle_(web::TestWebThreadBundle::REAL_DB_THREAD) {}
void SetUp() override {
CollectionViewControllerTest::SetUp();
origin_ = @"testorigin.com";
origin_ = kSite;
delegate_ = [[MockSavePasswordsCollectionViewController alloc] init];
reauthenticationModule_ = [[MockReauthenticationModule alloc] init];
}

CollectionViewController* InstantiateController() override {
autofill::PasswordForm form;
form.username_value = base::SysNSStringToUTF16(kUsername);
form.password_value = base::SysNSStringToUTF16(kPassword);
form.signon_realm = base::SysNSStringToUTF8(origin_);
form.origin = GURL(form.signon_realm);
return [[PasswordDetailsCollectionViewController alloc]
initWithPasswordForm:*(new autofill::PasswordForm())
initWithPasswordForm:form
delegate:delegate_
reauthenticationModule:reauthenticationModule_
username:kUsername
Expand All @@ -115,7 +125,16 @@ void CreateControllerWithOrigin(NSString* test_origin) {
TEST_F(PasswordDetailsCollectionViewControllerTest, TestInitialization) {
CreateController();
CheckController();
EXPECT_EQ(2, NumberOfSections());
EXPECT_EQ(3, NumberOfSections());
// Site section
EXPECT_EQ(2, NumberOfItemsInSection(kUsernameSection));
CheckSectionHeaderWithId(IDS_IOS_SHOW_PASSWORD_VIEW_SITE, kSiteSection);
PasswordDetailsItem* siteItem =
GetCollectionViewItem(kSiteSection, kSiteItem);
EXPECT_NSEQ(origin_, siteItem.text);
EXPECT_TRUE(siteItem.showingText);
CheckTextCellTitleWithId(IDS_IOS_SETTINGS_SITE_COPY_BUTTON, kSiteSection,
kCopySiteButtonItem);
// Username section
EXPECT_EQ(2, NumberOfItemsInSection(kUsernameSection));
CheckSectionHeaderWithId(IDS_IOS_SHOW_PASSWORD_VIEW_USERNAME,
Expand Down Expand Up @@ -162,6 +181,15 @@ void CreateControllerWithOrigin(NSString* test_origin) {
}
}

TEST_F(PasswordDetailsCollectionViewControllerTest, CopySite) {
CreateController();
[controller() collectionView:[controller() collectionView]
didSelectItemAtIndexPath:[NSIndexPath indexPathForRow:kCopySiteButtonItem
inSection:kSiteSection]];
UIPasteboard* generalPasteboard = [UIPasteboard generalPasteboard];
EXPECT_NSEQ(origin_, generalPasteboard.string);
}

TEST_F(PasswordDetailsCollectionViewControllerTest, CopyUsername) {
CreateController();
[controller() collectionView:[controller() collectionView]
Expand Down
93 changes: 79 additions & 14 deletions ios/chrome/browser/ui/settings/passwords_settings_egtest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -72,54 +72,92 @@
// same for multiple types of copied items (just "Copy"). Therefore the
// matchers here check the relative position of the Copy buttons to their
// respective section headers as well. The scheme of the vertical order is:
// Site header
// Copy (site) button
// Username header
// Copy (username) button
// Password header
// Copy (password) button

id<GREYMatcher> SiteHeader() {
return grey_allOf(
grey_accessibilityLabel(
l10n_util::GetNSString(IDS_IOS_SHOW_PASSWORD_VIEW_SITE)),
grey_accessibilityTrait(UIAccessibilityTraitHeader), nullptr);
}

id<GREYMatcher> UsernameHeader() {
return grey_allOf(
grey_accessibilityLabel(
l10n_util::GetNSString(IDS_IOS_SHOW_PASSWORD_VIEW_USERNAME)),
grey_accessibilityTrait(UIAccessibilityTraitHeader), nullptr);
}

id<GREYMatcher> PasswordHeader() {
return grey_allOf(
grey_accessibilityLabel(
l10n_util::GetNSString(IDS_IOS_SHOW_PASSWORD_VIEW_PASSWORD)),
grey_accessibilityTrait(UIAccessibilityTraitHeader), nullptr);
}

// Matcher for the Copy username button in Password Details view. This is the
// button above the Password section header.
id<GREYMatcher> CopyUsernameButton() {
GREYLayoutConstraint* above = [GREYLayoutConstraint
GREYLayoutConstraint* Above() {
return [GREYLayoutConstraint
layoutConstraintWithAttribute:kGREYLayoutAttributeBottom
relatedBy:kGREYLayoutRelationLessThanOrEqual
toReferenceAttribute:kGREYLayoutAttributeTop
multiplier:1.0
constant:0.0];
}

GREYLayoutConstraint* Below() {
return [GREYLayoutConstraint
layoutConstraintWithAttribute:kGREYLayoutAttributeTop
relatedBy:kGREYLayoutRelationGreaterThanOrEqual
toReferenceAttribute:kGREYLayoutAttributeBottom
multiplier:1.0
constant:0.0];
}

// Matcher for the Copy site button in Password Details view.
id<GREYMatcher> CopySiteButton() {
return grey_allOf(
ButtonWithAccessibilityLabel(
[NSString stringWithFormat:@"%@: %@",
l10n_util::GetNSString(
IDS_IOS_SHOW_PASSWORD_VIEW_SITE),
l10n_util::GetNSString(
IDS_IOS_SETTINGS_SITE_COPY_BUTTON)]),
grey_layout(@[ Below() ], SiteHeader()),
grey_layout(@[ Above() ], UsernameHeader()),
grey_layout(@[ Above() ], PasswordHeader()), nullptr);
}

// Matcher for the Copy username button in Password Details view.
id<GREYMatcher> CopyUsernameButton() {
return grey_allOf(
ButtonWithAccessibilityLabel([NSString
stringWithFormat:@"%@: %@",
l10n_util::GetNSString(
IDS_IOS_SHOW_PASSWORD_VIEW_USERNAME),
l10n_util::GetNSString(
IDS_IOS_SETTINGS_USERNAME_COPY_BUTTON)]),
grey_layout(@[ above ], PasswordHeader()), nullptr);
grey_layout(@[ Below() ], SiteHeader()),
grey_layout(@[ Below() ], UsernameHeader()),
grey_layout(@[ Above() ], PasswordHeader()), nullptr);
}

// Matcher for the Copy password button in Password Details view. This is the
// button below the Password section header.
// Matcher for the Copy password button in Password Details view.
id<GREYMatcher> CopyPasswordButton() {
GREYLayoutConstraint* below = [GREYLayoutConstraint
layoutConstraintWithAttribute:kGREYLayoutAttributeTop
relatedBy:kGREYLayoutRelationGreaterThanOrEqual
toReferenceAttribute:kGREYLayoutAttributeBottom
multiplier:1.0
constant:0.0];
return grey_allOf(
ButtonWithAccessibilityLabel([NSString
stringWithFormat:@"%@: %@",
l10n_util::GetNSString(
IDS_IOS_SHOW_PASSWORD_VIEW_PASSWORD),
l10n_util::GetNSString(
IDS_IOS_SETTINGS_PASSWORD_COPY_BUTTON)]),
grey_layout(@[ below ], PasswordHeader()), nullptr);
grey_layout(@[ Below() ], SiteHeader()),
grey_layout(@[ Below() ], UsernameHeader()),
grey_layout(@[ Below() ], PasswordHeader()), nullptr);
}

} // namespace
Expand Down Expand Up @@ -380,4 +418,31 @@ - (void)testCopyUsernameToast {
[self clearPasswordStore];
}

// Checks that attempts to copy a site URL provide appropriate feedback.
- (void)testCopySiteToast {
[self scopedEnablePasswordManagementAndViewingUI];

// Saving a form is needed for using the "password details" view.
[self saveExamplePasswordForm];

[self openPasswordSettings];

[[EarlGrey selectElementWithMatcher:Entry(@"https://example.com, user")]
performAction:grey_tap()];

// Check the snackbar.
[[EarlGrey selectElementWithMatcher:CopySiteButton()]
performAction:grey_tap()];
[[GREYUIThreadExecutor sharedInstance] drainUntilIdle];
NSString* snackbarLabel =
l10n_util::GetNSString(IDS_IOS_SETTINGS_SITE_WAS_COPIED_MESSAGE);
[[EarlGrey selectElementWithMatcher:grey_accessibilityLabel(snackbarLabel)]
assertWithMatcher:grey_notNil()];

[self tapBackArrow];
[self tapBackArrow];
[self tapDone];
[self clearPasswordStore];
}

@end

0 comments on commit b9e34e3

Please sign in to comment.