Commit 91171439 authored by Vaclav Brozek's avatar Vaclav Brozek Committed by Commit Bot

Always show saved entries in passwords settings on iOS

On iOS, unlike on Android or desktop platforms, toggling the "Save password"
switch to off hides the stored passwords (including blacklisted entries). Those
are still present in the profile, and are still filled in forms. The user needs
to enable saving to be able to delete them from the passwords settings.

The UI guidance (https://crbug.com/515462#c7) and product guidance
(https://crbug.com/515462#c8) is that iOS should instead mirror the other
platforms here and display the stored entries no matter whether saving is
enabled or disabled. This CL implements that change.

Bug: 515462
Change-Id: Iae1d35fc6b1eda273dc8efa068454462c1649110
Reviewed-on: https://chromium-review.googlesource.com/573381
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488183}
parent 763959aa
...@@ -14,7 +14,10 @@ ...@@ -14,7 +14,10 @@
#include "components/keyed_service/core/service_access_type.h" #include "components/keyed_service/core/service_access_type.h"
#include "components/password_manager/core/browser/password_store.h" #include "components/password_manager/core/browser/password_store.h"
#include "components/password_manager/core/common/password_manager_features.h" #include "components/password_manager/core/common/password_manager_features.h"
#include "components/password_manager/core/common/password_manager_pref_names.h"
#include "components/prefs/pref_service.h"
#include "components/strings/grit/components_strings.h" #include "components/strings/grit/components_strings.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/passwords/ios_chrome_password_store_factory.h" #include "ios/chrome/browser/passwords/ios_chrome_password_store_factory.h"
#import "ios/chrome/browser/ui/settings/password_details_collection_view_controller_for_testing.h" #import "ios/chrome/browser/ui/settings/password_details_collection_view_controller_for_testing.h"
#import "ios/chrome/browser/ui/settings/reauthentication_module.h" #import "ios/chrome/browser/ui/settings/reauthentication_module.h"
...@@ -24,6 +27,7 @@ ...@@ -24,6 +27,7 @@
#include "ios/chrome/grit/ios_strings.h" #include "ios/chrome/grit/ios_strings.h"
#import "ios/chrome/test/app/chrome_test_util.h" #import "ios/chrome/test/app/chrome_test_util.h"
#include "ios/chrome/test/earl_grey/accessibility_util.h" #include "ios/chrome/test/earl_grey/accessibility_util.h"
#import "ios/chrome/test/earl_grey/chrome_actions.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"
#import "ios/chrome/test/earl_grey/chrome_matchers.h" #import "ios/chrome/test/earl_grey/chrome_matchers.h"
...@@ -56,19 +60,26 @@ namespace { ...@@ -56,19 +60,26 @@ namespace {
// it too high could result in scrolling way past the searched element. // it too high could result in scrolling way past the searched element.
constexpr int kScrollAmount = 150; constexpr int kScrollAmount = 150;
// Returns the GREYElementInteraction* for the item on the password list with
// the given |matcher|. It scrolls in |direction| if necessary to ensure that
// the matched item is interactable. The result can be used to perform user
// actions or checks.
GREYElementInteraction* GetInteractionForListItem(id<GREYMatcher> matcher,
GREYDirection direction) {
return [[EarlGrey
selectElementWithMatcher:grey_allOf(matcher, grey_interactable(), nil)]
usingSearchAction:grey_scrollInDirection(direction, kScrollAmount)
onElementWithMatcher:grey_accessibilityID(
@"SavePasswordsCollectionViewController")];
}
// Returns the GREYElementInteraction* for the cell on the password list with // Returns the GREYElementInteraction* for the cell on the password list with
// the given |username|. It scrolls down if necessary to ensure that the matched // the given |username|. It scrolls down if necessary to ensure that the matched
// cell is interactable. The result can be used to perform user actions or // cell is interactable. The result can be used to perform user actions or
// checks. // checks.
GREYElementInteraction* GetInteractionForPasswordEntry(NSString* username) { GREYElementInteraction* GetInteractionForPasswordEntry(NSString* username) {
return [[EarlGrey return GetInteractionForListItem(ButtonWithAccessibilityLabel(username),
selectElementWithMatcher:grey_allOf( kGREYDirectionDown);
ButtonWithAccessibilityLabel(username),
grey_interactable(), nil)]
usingSearchAction:grey_scrollInDirection(kGREYDirectionDown,
kScrollAmount)
onElementWithMatcher:grey_accessibilityID(
@"SavePasswordsCollectionViewController")];
} }
// Returns the GREYElementInteraction* for the item on the detail view // Returns the GREYElementInteraction* for the item on the detail view
...@@ -980,4 +991,70 @@ MockReauthenticationModule* SetUpAndReturnMockReauthenticationModule() { ...@@ -980,4 +991,70 @@ MockReauthenticationModule* SetUpAndReturnMockReauthenticationModule() {
ClearPasswordStore(); ClearPasswordStore();
} }
// Check that stored entries are shown no matter what the preference for saving
// passwords is.
- (void)testStoredEntriesAlwaysShown {
SaveExamplePasswordForm();
PasswordForm blacklisted;
blacklisted.origin = GURL("https://blacklisted.com");
blacklisted.signon_realm = blacklisted.origin.spec();
blacklisted.blacklisted_by_user = true;
SaveToPasswordStore(blacklisted);
OpenPasswordSettings();
// Toggle the "Save Passwords" control off and back on and check that stored
// items are still present.
constexpr BOOL kExpectedState[] = {YES, NO};
for (BOOL expected_state : kExpectedState) {
// Toggle the switch. It is located near the top, so if not interactable,
// try scrolling up.
[GetInteractionForListItem(chrome_test_util::CollectionViewSwitchCell(
@"savePasswordsItem_switch", expected_state),
kGREYDirectionUp)
performAction:chrome_test_util::TurnCollectionViewSwitchOn(
!expected_state)];
// Check the stored items. Scroll down if needed.
[GetInteractionForPasswordEntry(@"example.com, concrete username")
assertWithMatcher:grey_notNil()];
[GetInteractionForPasswordEntry(@"blacklisted.com")
assertWithMatcher:grey_notNil()];
}
[[EarlGrey selectElementWithMatcher:SettingsMenuBackButton()]
performAction:grey_tap()];
TapDone();
ClearPasswordStore();
}
// Check that toggling the switch for the "save passwords" preference changes
// the settings.
- (void)testPrefToggle {
OpenPasswordSettings();
// Toggle the "Save Passwords" control off and back on and check the
// preferences.
constexpr BOOL kExpectedState[] = {YES, NO};
for (BOOL expected_initial_state : kExpectedState) {
[[EarlGrey
selectElementWithMatcher:chrome_test_util::CollectionViewSwitchCell(
@"savePasswordsItem_switch",
expected_initial_state)]
performAction:chrome_test_util::TurnCollectionViewSwitchOn(
!expected_initial_state)];
ios::ChromeBrowserState* browserState =
chrome_test_util::GetOriginalBrowserState();
const bool expected_final_state = !expected_initial_state;
GREYAssertEqual(expected_final_state,
browserState->GetPrefs()->GetBoolean(
password_manager::prefs::kPasswordManagerSavingEnabled),
@"State of the UI toggle differs from real preferences.");
}
[[EarlGrey selectElementWithMatcher:SettingsMenuBackButton()]
performAction:grey_tap()];
TapDone();
}
@end @end
...@@ -204,7 +204,6 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults( ...@@ -204,7 +204,6 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults(
toSectionWithIdentifier:SectionIdentifierSavePasswordsSwitch]; toSectionWithIdentifier:SectionIdentifierSavePasswordsSwitch];
// Saved passwords. // Saved passwords.
if ([passwordManagerEnabled_ value]) {
if (!savedForms_.empty()) { if (!savedForms_.empty()) {
[model addSectionWithIdentifier:SectionIdentifierSavedPasswords]; [model addSectionWithIdentifier:SectionIdentifierSavedPasswords];
CollectionViewTextItem* headerItem = CollectionViewTextItem* headerItem =
...@@ -233,7 +232,6 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults( ...@@ -233,7 +232,6 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults(
toSectionWithIdentifier:SectionIdentifierBlacklist]; toSectionWithIdentifier:SectionIdentifierBlacklist];
} }
} }
}
} }
#pragma mark - Items #pragma mark - Items
...@@ -256,6 +254,7 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults( ...@@ -256,6 +254,7 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults(
initWithType:ItemTypeSavePasswordsSwitch]; initWithType:ItemTypeSavePasswordsSwitch];
savePasswordsItem.text = l10n_util::GetNSString(IDS_IOS_SAVE_PASSWORDS); savePasswordsItem.text = l10n_util::GetNSString(IDS_IOS_SAVE_PASSWORDS);
savePasswordsItem.on = [passwordManagerEnabled_ value]; savePasswordsItem.on = [passwordManagerEnabled_ value];
savePasswordsItem.accessibilityIdentifier = @"savePasswordsItem_switch";
return savePasswordsItem; return savePasswordsItem;
} }
...@@ -378,10 +377,9 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults( ...@@ -378,10 +377,9 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults(
// Update the cell. // Update the cell.
[self reconfigureCellsForItems:@[ savePasswordsItem_ ]]; [self reconfigureCellsForItems:@[ savePasswordsItem_ ]];
// Update the rest of the UI. // Update the edit button.
[self.editor setEditing:NO]; [self.editor setEditing:NO];
[self updateEditButton]; [self updateEditButton];
[self reloadData];
} }
#pragma mark - Actions #pragma mark - Actions
...@@ -393,10 +391,9 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults( ...@@ -393,10 +391,9 @@ void SavePasswordsConsumer::OnGetPasswordStoreResults(
// Update the item. // Update the item.
savePasswordsItem_.on = [passwordManagerEnabled_ value]; savePasswordsItem_.on = [passwordManagerEnabled_ value];
// Update the rest of the UI. // Update the edit button.
[self.editor setEditing:NO]; [self.editor setEditing:NO];
[self updateEditButton]; [self updateEditButton];
[self reloadData];
} }
#pragma mark - Private methods #pragma mark - Private methods
......
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