Commit 6f4d43b6 authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

[Passwords] Leak check button in settings changes according to check status

This CL transforms the button according to the current state of the leak
check:
- "Check again" when IDLE, or CANCELLED,
- "Cancel" when RUNNING
- No button when out of quota or too many local passwords
- "Try again" in every other case

Translation screenshots:
IDS_SETTINGS_CHECK_PASSWORDS_AGAIN_AFTER_ERROR:
https://storage.cloud.google.com/chromium-translation-screenshots/49e6f4a55042471f270619ab6f9902ac6e778eac
IDS_SETTINGS_CHECK_PASSWORDS_AGAIN_IN_ACCOUNT:
https://storage.cloud.google.com/chromium-translation-screenshots/5fd2c1fe645c994d11e0df148b735532296784de
IDS_SETTINGS_CHECK_PASSWORDS_STOP:
https://storage.cloud.google.com/chromium-translation-screenshots/04fbc6de892ca0637efc2492057a1d39607d6a9b

Bug: 1047726
Change-Id: I685cac2a04572a41a5dc0815fb236192fd5e9524
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2095535
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748813}
parent b80ae603
......@@ -365,9 +365,18 @@
<message name="IDS_SETTINGS_CHECK_PASSWORDS_AGAIN" desc="Button to start bulk password check manually in passwords check section.">
Check again
</message>
<message name="IDS_SETTINGS_CHECK_PASSWORDS_AGAIN_AFTER_ERROR" desc="Button to start bulk password check manually in passwords check section after it already failed once.">
Try again
</message>
<message name="IDS_SETTINGS_CHECK_PASSWORDS_AGAIN_IN_ACCOUNT" desc="Link suggesting to perform a bulk leak check in the Google Account page.">
Check in Google Account.
</message>
<message name="IDS_SETTINGS_CHECK_PASSWORDS_PROGRESS" desc="Text for a label showing how many passwords were checked so far and how many passwords need to be checked in total.">
Checking passwords (<ph name="CHECKED_PASSWORDS">$1<ex>6</ex></ph> of <ph name="TOTAL_PASSWORDS">$2<ex>31</ex></ph>)…
</message>
<message name="IDS_SETTINGS_CHECK_PASSWORDS_STOP" desc="Button to manually stop an ongoing bulk password check.">
Cancel
</message>
<message name="IDS_SETTINGS_PASSWORDS_JUST_NOW" desc="Label shown when a compromised credential was found less than a minute ago">
Just now
</message>
......
49e6f4a55042471f270619ab6f9902ac6e778eac
\ No newline at end of file
5fd2c1fe645c994d11e0df148b735532296784de
\ No newline at end of file
04fbc6de892ca0637efc2492057a1d39607d6a9b
\ No newline at end of file
......@@ -47,6 +47,12 @@
<div class="start settings-box-text">
<div id="title">
[[getTitle_(status_)]]
<a class="secondary inline" id="linkToGoogleAccount"
href="[[passwordCheckUrl_]]"
target="_blank"
hidden$="[[!hasTooManyPasswords_(status_)]]">
$i18n{checkPasswordsAgainInAccount}
</a>
<span class="secondary inline" id="lastCompletedCheck"
hidden$="[[!showsTimestamp_(status_)]]">
&bull; [[lastCompletedCheck_]]
......@@ -58,8 +64,9 @@
</div>
</div>
<cr-button id="controlPasswordCheckButton"
on-click="onPasswordCheckButtonClick_">
$i18n{checkPasswordsAgain}
on-click="onPasswordCheckButtonClick_"
hidden$="[[isButtonHidden_(status_)]]">
[[getButtonText_(status_)]]
</cr-button>
</div>
<div id="noCompromisedCredentials"
......
......@@ -13,6 +13,17 @@ Polymer({
behaviors: [I18nBehavior],
properties: {
/**
* This URL redirects to the passwords check for sync users.
* @type {!string}
* @private
*/
passwordCheckUrl_: {
type: String,
value:
'https://passwords.google.com/checkup/start?utm_source=chrome&utm_medium=desktop&utm_campaign=leak_dialog&hideExplanation=true',
},
/** @private */
lastCompletedCheck_: String,
......@@ -119,9 +130,22 @@ Polymer({
* @private
*/
onPasswordCheckButtonClick_() {
// TODO(https://crbug.com/1047726): By click on this button user should be
// able to 'Cancel' current check.
this.passwordManager_.startBulkPasswordCheck();
switch (this.status_.state) {
case CheckState.RUNNING:
this.passwordManager_.stopBulkPasswordCheck();
return;
case CheckState.IDLE:
case CheckState.CANCELED:
case CheckState.OFFLINE:
case CheckState.SIGNED_OUT:
case CheckState.NO_PASSWORDS:
case CheckState.OTHER_ERROR:
this.passwordManager_.startBulkPasswordCheck();
return;
case CheckState.TOO_MANY_PASSWORDS:
case CheckState.QUOTA_LIMIT:
}
throw 'Can\'t trigger an action for state: ' + this.status_.state;
},
/**
......@@ -253,6 +277,64 @@ Polymer({
return status.state == CheckState.IDLE;
},
/**
* Returns the button caption indicating it's current functionality.
* @param {!PasswordManagerProxy.PasswordCheckStatus} status
* @return {string}
* @private
*/
getButtonText_(status) {
switch (status.state) {
case CheckState.IDLE:
case CheckState.CANCELED:
return this.i18n('checkPasswordsAgain');
case CheckState.RUNNING:
return this.i18n('checkPasswordsStop');
case CheckState.OFFLINE:
case CheckState.SIGNED_OUT:
case CheckState.NO_PASSWORDS:
case CheckState.OTHER_ERROR:
return this.i18n('checkPasswordsAgainAfterError');
case CheckState.QUOTA_LIMIT:
case CheckState.TOO_MANY_PASSWORDS:
return ''; // Undefined behavior. Don't show any misleading text.
}
throw 'Can\'t find a button text for state: ' + status.state;
},
/**
* Returns true iff the check/stop button should be visible for a given state.
* @param {!PasswordManagerProxy.PasswordCheckStatus} status
* @return {!boolean}
* @private
*/
isButtonHidden_(status) {
switch (status.state) {
case CheckState.IDLE:
case CheckState.CANCELED:
case CheckState.RUNNING:
case CheckState.OFFLINE:
case CheckState.SIGNED_OUT:
case CheckState.NO_PASSWORDS:
case CheckState.OTHER_ERROR:
return false;
case CheckState.TOO_MANY_PASSWORDS:
case CheckState.QUOTA_LIMIT:
return true;
}
throw 'Can\'t determine button visibility for state: ' + status.state;
},
/**
* Returns true iff the backend communicated that there are too many saved
* passwords to check them locally.
* @param {!PasswordManagerProxy.PasswordCheckStatus} status
* @return {boolean}
* @private
*/
hasTooManyPasswords_(status) {
return status.state == CheckState.TOO_MANY_PASSWORDS;
},
/**
* Returns true if there are leaked credentials or the status is unexpected
......
......@@ -154,6 +154,11 @@ class PasswordManagerProxy {
*/
startBulkPasswordCheck() {}
/**
* Requests to interrupt an ongoing bulk password check.
*/
stopBulkPasswordCheck() {}
/**
* Requests the latest information about compromised credentials.
* @return {!Promise<(PasswordManagerProxy.CompromisedCredentialsInfo)>}
......@@ -354,6 +359,11 @@ class PasswordManagerImpl {
chrome.passwordsPrivate.startPasswordCheck();
}
/** @override */
stopBulkPasswordCheck() {
chrome.passwordsPrivate.stopPasswordCheck();
}
/** @override */
getCompromisedCredentialsInfo() {
return new Promise(resolve => {
......
......@@ -749,7 +749,12 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
{"checkPasswordsErrorGeneric",
IDS_SETTINGS_CHECK_PASSWORDS_ERROR_GENERIC},
{"checkPasswordsAgain", IDS_SETTINGS_CHECK_PASSWORDS_AGAIN},
{"checkPasswordsAgainAfterError",
IDS_SETTINGS_CHECK_PASSWORDS_AGAIN_AFTER_ERROR},
{"checkPasswordsProgress", IDS_SETTINGS_CHECK_PASSWORDS_PROGRESS},
{"checkPasswordsAgainInAccount",
IDS_SETTINGS_CHECK_PASSWORDS_AGAIN_IN_ACCOUNT},
{"checkPasswordsStop", IDS_SETTINGS_CHECK_PASSWORDS_STOP},
{"compromisedPasswords", IDS_SETTINGS_COMPROMISED_PASSWORDS},
{"compromisedPasswordsDescription",
IDS_SETTINGS_COMPROMISED_PASSWORDS_ADVICE},
......
......@@ -77,10 +77,135 @@ cr.define('settings_passwords_check', function() {
// Test verifies that clicking 'Check again' make proper function call to
// password manager
test('testCheckAgainButton', function() {
const checkPasswordSection = createCheckPasswordSection();
checkPasswordSection.$.controlPasswordCheckButton.click();
return passwordManager.whenCalled('startBulkPasswordCheck');
test('testCheckAgainButtonWhenIdle', function() {
assertEquals(
PasswordCheckState.IDLE, passwordManager.data.checkStatus.state);
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus')
.then(() => {
assert(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals(
section.i18n('checkPasswordsAgain'),
section.$.controlPasswordCheckButton.innerText);
section.$.controlPasswordCheckButton.click();
})
.then(() => passwordManager.whenCalled('startBulkPasswordCheck'));
});
// Test verifies that clicking 'Check again' make proper function call to
// password manager
test('testStopButtonWhenRunning', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.RUNNING,
/*checked=*/ 0,
/*remaining=*/ 2);
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus')
.then(() => {
assert(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals(
section.i18n('checkPasswordsStop'),
section.$.controlPasswordCheckButton.innerText);
section.$.controlPasswordCheckButton.click();
})
.then(() => passwordManager.whenCalled('stopBulkPasswordCheck'));
});
// Test verifies that 'Try again' is not visible if users are out of quota.
test('testNoRetryAfterHittingQuota', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.QUOTA_LIMIT);
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
expectFalse(isElementVisible(section.$.controlPasswordCheckButton));
});
});
// Test verifies that 'Try again' is not visible if users have too many
// passwords. Instead, there should be a link to their Google Account.
test('testNoRetryForTooManyPasswords', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.TOO_MANY_PASSWORDS);
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
expectFalse(isElementVisible(section.$.controlPasswordCheckButton));
expectTrue(isElementVisible(section.$.linkToGoogleAccount));
});
});
// Test verifies that 'Try again' visible and working when users encounter a
// generic error.
test('testShowRetryAfterGenericError', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.OTHER_ERROR);
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus')
.then(() => {
assert(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals(
section.i18n('checkPasswordsAgainAfterError'),
section.$.controlPasswordCheckButton.innerText);
section.$.controlPasswordCheckButton.click();
})
.then(() => passwordManager.whenCalled('startBulkPasswordCheck'));
});
// Test verifies that 'Try again' visible and working when users encounter a
// not-signed-in error.
test('testShowRetryAfterSignOutError', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.SIGNED_OUT);
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus')
.then(() => {
assert(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals(
section.i18n('checkPasswordsAgainAfterError'),
section.$.controlPasswordCheckButton.innerText);
section.$.controlPasswordCheckButton.click();
})
.then(() => passwordManager.whenCalled('startBulkPasswordCheck'));
});
// Test verifies that 'Try again' visible and working when users encounter a
// no-saved-passwords error.
test('testShowRetryAfterNoPasswordsError', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.NO_PASSWORDS);
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus')
.then(() => {
assert(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals(
section.i18n('checkPasswordsAgainAfterError'),
section.$.controlPasswordCheckButton.innerText);
section.$.controlPasswordCheckButton.click();
})
.then(() => passwordManager.whenCalled('startBulkPasswordCheck'));
});
// Test verifies that 'Try again' visible and working when users encounter a
// connection error.
test('testShowRetryAfterNoConnectionError', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.OFFLINE);
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus')
.then(() => {
assert(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals(
section.i18n('checkPasswordsAgainAfterError'),
section.$.controlPasswordCheckButton.innerText);
section.$.controlPasswordCheckButton.click();
})
.then(() => passwordManager.whenCalled('startBulkPasswordCheck'));
});
// Test verifies that if no compromised credentials found than list is not
......@@ -446,7 +571,8 @@ cr.define('settings_passwords_check', function() {
assert(isElementVisible(title));
// TODO(crbug.com/1047726): Check for account redirection.
expectEquals(
section.i18n('checkPasswordsErrorTooManyPasswords'),
section.i18n('checkPasswordsErrorTooManyPasswords') + ' ' +
section.i18n('checkPasswordsAgainInAccount'),
title.innerText);
expectFalse(isElementVisible(section.$.subtitle));
});
......@@ -484,5 +610,32 @@ cr.define('settings_passwords_check', function() {
expectFalse(isElementVisible(section.$.subtitle));
});
});
// Transform check-button to stop-button if a check is running.
test('testButtonChangesTextAccordingToStatus', function() {
passwordManager.data.checkStatus =
autofill_test_util.makePasswordCheckStatus(PasswordCheckState.IDLE);
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
assert(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals(
section.i18n('checkPasswordsAgain'),
section.$.controlPasswordCheckButton.innerText);
// Change status from running to IDLE.
assert(!!passwordManager.lastCallback.addPasswordCheckStatusListener);
passwordManager.lastCallback.addPasswordCheckStatusListener(
autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.RUNNING,
/*checked=*/ 0,
/*remaining=*/ 2));
assert(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals(
section.i18n('checkPasswordsStop'),
section.$.controlPasswordCheckButton.innerText);
});
});
});
});
......@@ -14,6 +14,7 @@ class TestPasswordManagerProxy extends TestBrowserProxy {
super([
'requestPlaintextPassword',
'startBulkPasswordCheck',
'stopBulkPasswordCheck',
'getCompromisedCredentialsInfo',
'getPasswordCheckStatus',
]);
......@@ -152,6 +153,11 @@ class TestPasswordManagerProxy extends TestBrowserProxy {
this.methodCalled('startBulkPasswordCheck');
}
/** @override */
stopBulkPasswordCheck() {
this.methodCalled('stopBulkPasswordCheck');
}
/** @override */
getCompromisedCredentialsInfo() {
this.methodCalled('getCompromisedCredentialsInfo');
......
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