Commit 0747522c authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

[Passwords] Leak check new button and title for idle state

With this CL, the title during the successful IDLE state changes to
"Checked Passwords" (not added since it exists for the same context).

Also, the IDLE state shows "Check again" only if a check happened before
and defaults to "Check Passwords" if there hasn't been a check yet.
Without an initial check, we also show a different message and banner
and hide icon and subtitle.

Screenshots are in the linked bug.

Bug: 1062216
Change-Id: I484504fdb37e0043b553813ff51008085d58488f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2106087
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751103}
parent 7cfbc823
...@@ -48,6 +48,10 @@ ...@@ -48,6 +48,10 @@
background-size: 16px 16px; background-size: 16px 16px;
} }
iron-icon.hidden {
display: none;
}
</style> </style>
<!-- The banner is visible if no compromised password was found (yet). --> <!-- The banner is visible if no compromised password was found (yet). -->
<template is="dom-if" if="[[shouldShowBanner_(status_, leakedPasswords)]]"> <template is="dom-if" if="[[shouldShowBanner_(status_, leakedPasswords)]]">
...@@ -96,6 +100,7 @@ ...@@ -96,6 +100,7 @@
</div> </div>
<cr-button id="controlPasswordCheckButton" <cr-button id="controlPasswordCheckButton"
on-click="onPasswordCheckButtonClick_" on-click="onPasswordCheckButtonClick_"
class$="[[getButtonTypeClass_(status_)]]"
hidden$="[[isButtonHidden_(status_, suppressCheckupLink_)]]"> hidden$="[[isButtonHidden_(status_, suppressCheckupLink_)]]">
[[getButtonText_(status_)]] [[getButtonText_(status_)]]
</cr-button> </cr-button>
......
...@@ -261,7 +261,7 @@ Polymer({ ...@@ -261,7 +261,7 @@ Polymer({
*/ */
getStatusIconClass_(status, leakedPasswords) { getStatusIconClass_(status, leakedPasswords) {
if (!this.hasLeaksOrErrors_(status, leakedPasswords)) { if (!this.hasLeaksOrErrors_(status, leakedPasswords)) {
return 'no-leaks'; return this.waitsForFirstCheck_() ? 'hidden' : 'no-leaks';
} }
if (this.hasLeakedCredentials_(leakedPasswords)) { if (this.hasLeakedCredentials_(leakedPasswords)) {
return 'has-leaks'; return 'has-leaks';
...@@ -277,7 +277,9 @@ Polymer({ ...@@ -277,7 +277,9 @@ Polymer({
getTitle_() { getTitle_() {
switch (this.status_.state) { switch (this.status_.state) {
case CheckState.IDLE: case CheckState.IDLE:
return this.i18n('checkPasswords'); return this.waitsForFirstCheck_() ?
this.i18n('checkPasswordsDescription') :
this.i18n('checkedPasswords');
case CheckState.CANCELED: case CheckState.CANCELED:
return this.i18n('checkPasswordsCanceled'); return this.i18n('checkPasswordsCanceled');
case CheckState.RUNNING: case CheckState.RUNNING:
...@@ -332,6 +334,8 @@ Polymer({ ...@@ -332,6 +334,8 @@ Polymer({
getButtonText_(status) { getButtonText_(status) {
switch (status.state) { switch (status.state) {
case CheckState.IDLE: case CheckState.IDLE:
return this.waitsForFirstCheck_() ? this.i18n('checkPasswords') :
this.i18n('checkPasswordsAgain');
case CheckState.CANCELED: case CheckState.CANCELED:
case CheckState.TOO_MANY_PASSWORDS: case CheckState.TOO_MANY_PASSWORDS:
return this.i18n('checkPasswordsAgain'); return this.i18n('checkPasswordsAgain');
...@@ -349,6 +353,15 @@ Polymer({ ...@@ -349,6 +353,15 @@ Polymer({
assertNotReached('Can\'t find a button text for state: ' + status.state); assertNotReached('Can\'t find a button text for state: ' + status.state);
}, },
/**
* Returns 'action-button' only for the very first check.
* @return {string}
* @private
*/
getButtonTypeClass_() {
return this.waitsForFirstCheck_() ? 'action-button' : ' ';
},
/** /**
* Returns true iff the check/stop button should be visible for a given state. * Returns true iff the check/stop button should be visible for a given state.
* @param {!PasswordManagerProxy.PasswordCheckStatus} status * @param {!PasswordManagerProxy.PasswordCheckStatus} status
...@@ -382,7 +395,10 @@ Polymer({ ...@@ -382,7 +395,10 @@ Polymer({
* @private * @private
*/ */
bannerImageSrc_(isDarkMode) { bannerImageSrc_(isDarkMode) {
const type = this.status_.state == CheckState.IDLE ? 'positive' : 'neutral'; const type =
(this.status_.state == CheckState.IDLE && !this.waitsForFirstCheck_()) ?
'positive' :
'neutral';
const suffix = isDarkMode ? '_dark' : ''; const suffix = isDarkMode ? '_dark' : '';
return `chrome://settings/images/password_check_${type}${suffix}.svg`; return `chrome://settings/images/password_check_${type}${suffix}.svg`;
}, },
...@@ -442,7 +458,7 @@ Polymer({ ...@@ -442,7 +458,7 @@ Polymer({
} }
switch (this.status_.state) { switch (this.status_.state) {
case CheckState.IDLE: case CheckState.IDLE:
return true; return !this.waitsForFirstCheck_();
case CheckState.CANCELED: case CheckState.CANCELED:
case CheckState.RUNNING: case CheckState.RUNNING:
case CheckState.OFFLINE: case CheckState.OFFLINE:
...@@ -460,6 +476,15 @@ Polymer({ ...@@ -460,6 +476,15 @@ Polymer({
this.status_.state); this.status_.state);
}, },
/**
* Returns true iff the leak check was performed at least once before.
* @return {boolean}
* @private
*/
waitsForFirstCheck_() {
return !this.status_.elapsedTimeSinceLastCheck;
},
/** /**
* Returns true iff the user cannot retry the password check in their account. * Returns true iff the user cannot retry the password check in their account.
* Either because they are syncing or because they use a custom passphrase. * Either because they are syncing or because they use a custom passphrase.
......
...@@ -130,7 +130,28 @@ cr.define('settings_passwords_check', function() { ...@@ -130,7 +130,28 @@ cr.define('settings_passwords_check', function() {
// Test verifies that clicking 'Check again' make proper function call to // Test verifies that clicking 'Check again' make proper function call to
// password manager // password manager
test('testCheckAgainButtonWhenIdle', function() { test('testCheckAgainButtonWhenIdleAfterFirstRun', function() {
const data = passwordManager.data;
data.checkStatus = autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.IDLE,
/*checked=*/ undefined,
/*remaining=*/ undefined,
/*lastCheck=*/ 'Just now');
const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus')
.then(() => {
assertTrue(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals(
section.i18n('checkPasswordsAgain'),
section.$.controlPasswordCheckButton.innerText);
section.$.controlPasswordCheckButton.click();
})
.then(() => passwordManager.whenCalled('startBulkPasswordCheck'));
});
// Test verifies that clicking 'Start Check' make proper function call to
// password manager
test('testStartCheckButtonWhenIdle', function() {
assertEquals( assertEquals(
PasswordCheckState.IDLE, passwordManager.data.checkStatus.state); PasswordCheckState.IDLE, passwordManager.data.checkStatus.state);
const section = createCheckPasswordSection(); const section = createCheckPasswordSection();
...@@ -138,7 +159,7 @@ cr.define('settings_passwords_check', function() { ...@@ -138,7 +159,7 @@ cr.define('settings_passwords_check', function() {
.then(() => { .then(() => {
assertTrue(isElementVisible(section.$.controlPasswordCheckButton)); assertTrue(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals( expectEquals(
section.i18n('checkPasswordsAgain'), section.i18n('checkPasswords'),
section.$.controlPasswordCheckButton.innerText); section.$.controlPasswordCheckButton.innerText);
section.$.controlPasswordCheckButton.click(); section.$.controlPasswordCheckButton.click();
}) })
...@@ -540,8 +561,12 @@ cr.define('settings_passwords_check', function() { ...@@ -540,8 +561,12 @@ cr.define('settings_passwords_check', function() {
// Tests that the spinner is replaced with a checkmark on successful runs. // Tests that the spinner is replaced with a checkmark on successful runs.
test('testShowsCheckmarkIconWhenFinishedWithoutLeaks', function() { test('testShowsCheckmarkIconWhenFinishedWithoutLeaks', function() {
const data = passwordManager.data; const data = passwordManager.data;
assertEquals(PasswordCheckState.IDLE, data.checkStatus.state);
assertEquals(0, data.leakedCredentials.length); assertEquals(0, data.leakedCredentials.length);
data.checkStatus = autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.IDLE,
/*checked=*/ undefined,
/*remaining=*/ undefined,
/*lastCheck=*/ 'Just now');
const checkPasswordSection = createCheckPasswordSection(); const checkPasswordSection = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => { return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
...@@ -555,6 +580,23 @@ cr.define('settings_passwords_check', function() { ...@@ -555,6 +580,23 @@ cr.define('settings_passwords_check', function() {
}); });
}); });
// Tests that there is neither spinner nor icon if the check hasn't run yet.
test('testIconWhenFirstRunIsPending', function() {
const data = passwordManager.data;
assertEquals(0, data.leakedCredentials.length);
data.checkStatus =
autofill_test_util.makePasswordCheckStatus(PasswordCheckState.IDLE);
const checkPasswordSection = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
Polymer.dom.flush();
const icon = checkPasswordSection.$$('iron-icon');
const spinner = checkPasswordSection.$$('paper-spinner-lite');
expectFalse(isElementVisible(spinner));
expectFalse(isElementVisible(icon));
});
});
// Tests that the spinner is replaced with a triangle if leaks were found. // Tests that the spinner is replaced with a triangle if leaks were found.
test('testShowsTriangleIconWhenFinishedWithLeaks', function() { test('testShowsTriangleIconWhenFinishedWithLeaks', function() {
const data = passwordManager.data; const data = passwordManager.data;
...@@ -681,16 +723,17 @@ cr.define('settings_passwords_check', function() { ...@@ -681,16 +723,17 @@ cr.define('settings_passwords_check', function() {
}); });
}); });
// After running, show confirmation, timestamp and number of leaks. // Before the first run, show only a description of what the check does.
test('testDontShowTimeStampIfNotRun', function() { test('testShowOnlyDescriptionIfNotRun', function() {
const section = createCheckPasswordSection(); const section = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => { return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
Polymer.dom.flush(); Polymer.dom.flush();
const title = section.$.title; const title = section.$.title;
const subtitle = section.$.subtitle; const subtitle = section.$.subtitle;
assertTrue(isElementVisible(title)); assertTrue(isElementVisible(title));
assertTrue(isElementVisible(subtitle)); assertFalse(isElementVisible(subtitle));
expectEquals(section.i18n('checkPasswords'), title.innerText); expectEquals(section.i18n('checkPasswordsDescription'),
title.innerText);
}); });
}); });
...@@ -715,7 +758,7 @@ cr.define('settings_passwords_check', function() { ...@@ -715,7 +758,7 @@ cr.define('settings_passwords_check', function() {
assertTrue(isElementVisible(title)); assertTrue(isElementVisible(title));
assertTrue(isElementVisible(subtitle)); assertTrue(isElementVisible(subtitle));
expectEquals( expectEquals(
section.i18n('checkPasswords') + ' • Just now', title.innerText); section.i18n('checkedPasswords') + ' • Just now', title.innerText);
}); });
}); });
...@@ -847,7 +890,7 @@ cr.define('settings_passwords_check', function() { ...@@ -847,7 +890,7 @@ cr.define('settings_passwords_check', function() {
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => { return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
assertTrue(isElementVisible(section.$.controlPasswordCheckButton)); assertTrue(isElementVisible(section.$.controlPasswordCheckButton));
expectEquals( expectEquals(
section.i18n('checkPasswordsAgain'), section.i18n('checkPasswords'),
section.$.controlPasswordCheckButton.innerText); section.$.controlPasswordCheckButton.innerText);
// Change status from running to IDLE. // Change status from running to IDLE.
...@@ -869,8 +912,12 @@ cr.define('settings_passwords_check', function() { ...@@ -869,8 +912,12 @@ cr.define('settings_passwords_check', function() {
// after a leak check finished. // after a leak check finished.
test('testShowsPositiveBannerWhenIdle', function() { test('testShowsPositiveBannerWhenIdle', function() {
const data = passwordManager.data; const data = passwordManager.data;
assertEquals(PasswordCheckState.IDLE, data.checkStatus.state);
assertEquals(0, data.leakedCredentials.length); assertEquals(0, data.leakedCredentials.length);
data.checkStatus = autofill_test_util.makePasswordCheckStatus(
/*state=*/ PasswordCheckState.IDLE,
/*checked=*/ undefined,
/*remaining=*/ undefined,
/*lastCheck=*/ 'Just now');
const checkPasswordSection = createCheckPasswordSection(); const checkPasswordSection = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => { return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
...@@ -882,6 +929,22 @@ cr.define('settings_passwords_check', function() { ...@@ -882,6 +929,22 @@ cr.define('settings_passwords_check', function() {
}); });
}); });
// Test that the banner indicates a neutral state if no check was run yet.
test('testShowsNeutralBannerBeforeFirstRun', function() {
const data = passwordManager.data;
assertEquals(PasswordCheckState.IDLE, data.checkStatus.state);
assertEquals(0, data.leakedCredentials.length);
const checkPasswordSection = createCheckPasswordSection();
return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
Polymer.dom.flush();
assertTrue(isElementVisible(checkPasswordSection.$$('#bannerImage')));
expectEquals(
'chrome://settings/images/password_check_neutral.svg',
checkPasswordSection.$$('#bannerImage').src);
});
});
// Test that the banner is in a state that shows that the leak check is // Test that the banner is in a state that shows that the leak check is
// in progress but hasn't found anything yet. // in progress but hasn't found anything yet.
test('testShowsNeutralBannerWhenRunning', function() { test('testShowsNeutralBannerWhenRunning', function() {
......
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