Commit 66faa2f0 authored by Victor Hugo Vianna Silva's avatar Victor Hugo Vianna Silva Committed by Commit Bot

[b4p/Settings] Remove exceptions from 'passwords on this device' page

This CL is essentially a revert of cc694e8d, the reason being that UX
has decided not to have exceptions on this page anymore. The only two
differences regarding the original commit are: hasDeviceOnlyPasswords_
and hasDeviceAndAccountPasswords_ are still replaced with a shared use
of isNonEmpty_(); and the file documentation is updated to remove any
mention to exceptions.

Bug: 1049141
Change-Id: Ic95bf0042d6f4214ec40dd9dd0fedc492de56421
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2248683
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779319}
parent 980a9c7d
......@@ -275,10 +275,7 @@ js_library("passwords_section") {
js_library("passwords_device_section") {
deps = [
":merge_exceptions_store_copies_behavior",
":merge_passwords_store_copies_behavior",
":multi_store_exception_entry",
":multi_store_id_handler",
":multi_store_password_ui_entry",
":password_list_item",
":password_manager_proxy",
......
......@@ -48,54 +48,3 @@
</div>
</div>
</passwords-list-handler>
<div class="cr-row first">
<h2 class="flex">$i18n{passwordExceptionsHeading}</h2>
</div>
<div class="list-frame">
<div class="list-item column-header" aria-hidden="true">
$i18n{deviceOnlyPasswordsHeading}
</div>
<div class="vertical-list" id="deviceOnlyExceptionsList">
<template is="dom-repeat"
items="[[getFilteredExceptions_(deviceOnlyExceptions_, filter)]]">
<div class="list-item">
<div class="start website-column">
<site-favicon url="[[item.urls.link]]"></site-favicon>
<a id="exception" href="[[item.urls.link]]" target="_blank">
[[item.urls.shown]]
</a>
</div>
<cr-icon-button class="icon-clear" id="removeExceptionButton"
on-click="onRemoveExceptionButtonTap_" tabindex$="[[tabIndex]]"
title="$i18n{deletePasswordException}"></cr-icon-button>
</div>
</template>
</div>
<div id="noDeviceOnlyExceptionsLabel" class="list-item"
hidden$="[[isNonEmpty_(deviceOnlyExceptions_)]]">
$i18n{noExceptionsFound}
</div>
<div class="list-item column-header" aria-hidden="true">
$i18n{deviceAndAccountPasswordsHeading}
</div>
<div class="vertical-list" id="deviceAndAccountExceptionsList">
<template is="dom-repeat"
items="[[getFilteredExceptions_(deviceAndAccountExceptions_, filter)]]">
<div class="list-item">
<div class="start website-column">
<site-favicon url="[[item.urls.link]]"></site-favicon>
<a id="exception" href="[[item.urls.link]]" target="_blank">
[[item.urls.shown]]
</a>
</div>
<cr-icon-button class="icon-clear" id="removeExceptionButton"
on-click="onRemoveExceptionButtonTap_" tabindex$="[[tabIndex]]"
title="$i18n{deletePasswordException}"></cr-icon-button>
</div>
</template>
</div>
<div id="noDeviceAndAccountExceptionsLabel" class="list-item"
hidden$="[[isNonEmpty_(deviceAndAccountExceptions_)]]">
$i18n{noExceptionsFound}
</div>
</div>
......@@ -4,8 +4,7 @@
/**
* @fileoverview 'passwords-device-section' represents the page containing
* the list of passwords and exceptions (websites where passwords are never
* saved) which have at least one copy on the user device.
* the list of passwords which have at least one copy on the user device.
*
* This page is *not* displayed on ChromeOS.
*/
......@@ -26,10 +25,7 @@ import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bun
import {GlobalScrollTargetBehavior} from '../global_scroll_target_behavior.m.js';
import {routes} from '../route.js';
import {MergeExceptionsStoreCopiesBehavior} from './merge_exceptions_store_copies_behavior.js';
import {MergePasswordsStoreCopiesBehavior} from './merge_passwords_store_copies_behavior.js';
import {MultiStoreExceptionEntry} from './multi_store_exception_entry.js';
import {MultiStoreIdHandler} from './multi_store_id_handler.js';
import {MultiStorePasswordUiEntry} from './multi_store_password_ui_entry.js';
import {PasswordManagerImpl} from './password_manager_proxy.js';
......@@ -53,7 +49,6 @@ Polymer({
_template: html`{__html_template__}`,
behaviors: [
MergeExceptionsStoreCopiesBehavior,
MergePasswordsStoreCopiesBehavior,
I18nBehavior,
IronA11yKeysBehavior,
......@@ -67,7 +62,7 @@ Polymer({
value: routes.DEVICE_PASSWORDS,
},
/** Filter on the saved passwords and exceptions. */
/** Search filter on the saved passwords. */
filter: {
type: String,
value: '',
......@@ -90,7 +85,8 @@ Polymer({
deviceOnlyPasswords_: {
type: Array,
value: () => [],
computed: 'getDeviceOnlyEntries_(savedPasswords, savedPasswords.splices)',
computed:
'computeDeviceOnlyPasswords_(savedPasswords, savedPasswords.splices)',
},
/**
......@@ -101,34 +97,10 @@ Polymer({
deviceAndAccountPasswords_: {
type: Array,
value: () => [],
computed: 'getDeviceAndAccountEntries_(savedPasswords, ' +
computed: 'computeDeviceAndAccountPasswords_(savedPasswords, ' +
'savedPasswords.splices)',
},
/**
* Exceptions displayed in the device-only subsection.
* @type {!Array<!MultiStoreExceptionEntry>}
* @private
*/
deviceOnlyExceptions_: {
type: Array,
value: () => [],
computed: 'getDeviceOnlyEntries_(passwordExceptions, ' +
'passwordExceptions.splices)',
},
/**
* Exceptions displayed in the device-and-account subsection.
* @type {!Array<!MultiStoreExceptionEntry>}
* @private
*/
deviceAndAccountExceptions_: {
type: Array,
value: () => [],
computed: 'getDeviceAndAccountEntries_(passwordExceptions, ' +
'passwordExceptions.splices)',
},
/** @private {!MultiStorePasswordUiEntry} */
lastFocused_: Object,
......@@ -138,12 +110,12 @@ Polymer({
},
/**
* @param {!Array<!MultiStoreIdHandler>} entries
* @param {!Array<!MultiStorePasswordUiEntry>} passwords
* @return {boolean}
* @private
*/
isNonEmpty_(entries) {
return entries.length > 0;
isNonEmpty_(passwords) {
return passwords.length > 0;
},
keyBindings: {
......@@ -156,23 +128,21 @@ Polymer({
},
/**
* @param {!Array<!MultiStoreIdHandler>} entries
* @return {!Array<!MultiStoreIdHandler>}
* @return {!Array<!MultiStorePasswordUiEntry>}
* @private
*/
getDeviceOnlyEntries_(entries) {
return entries.filter(
entry => entry.isPresentOnDevice() && !entry.isPresentInAccount());
computeDeviceOnlyPasswords_() {
return this.savedPasswords.filter(
p => p.isPresentOnDevice() && !p.isPresentInAccount());
},
/**
* @param {!Array<!MultiStoreIdHandler>} entries
* @return {!Array<!MultiStoreIdHandler>}
* @return {!Array<!MultiStorePasswordUiEntry>}
* @private
*/
getDeviceAndAccountEntries_(entries) {
return entries.filter(
entry => entry.isPresentOnDevice() && entry.isPresentInAccount());
computeDeviceAndAccountPasswords_() {
return this.savedPasswords.filter(
p => p.isPresentOnDevice() && p.isPresentInAccount());
},
/**
......@@ -191,37 +161,6 @@ Polymer({
term => term.toLowerCase().includes(filter.toLowerCase())));
},
/**
* @param {!Array<!MultiStoreExceptionEntry>} exceptions
* @param {string} filter
* @return {!Array<!MultiStoreExceptionEntry>}
* @private
*/
getFilteredExceptions_(exceptions, filter) {
return exceptions.filter(
e => e.urls.shown.toLowerCase().includes(filter.toLowerCase()));
},
/**
* Handler for removing an exception.
* @param {!{model: !{item: !chrome.passwordsPrivate.ExceptionEntry}}} event
* @private
*/
// TODO(crbug.com/1049141): Consider introducing <exception-list-item> to
// avoid duplicating this handler.
onRemoveExceptionButtonTap_(event) {
const exception = event.model.item;
/** @type {!Array<number>} */
const allExceptionIds = [];
if (exception.isPresentInAccount()) {
allExceptionIds.push(exception.accountId);
}
if (exception.isPresentOnDevice()) {
allExceptionIds.push(exception.deviceId);
}
PasswordManagerImpl.getInstance().removeExceptions(allExceptionIds);
},
/**
* Handle the undo shortcut.
* @param {!Event} event
......
......@@ -6,8 +6,8 @@
import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {MultiStoreExceptionEntry, MultiStorePasswordUiEntry, PasswordManagerImpl, PasswordManagerProxy} from 'chrome://settings/settings.js';
import {createExceptionEntry, createMultiStoreExceptionEntry, createMultiStorePasswordEntry, createPasswordEntry} from 'chrome://test/settings/passwords_and_autofill_fake_data.js';
import {MultiStorePasswordUiEntry, PasswordManagerImpl, PasswordManagerProxy} from 'chrome://settings/settings.js';
import {createMultiStorePasswordEntry, createPasswordEntry} from 'chrome://test/settings/passwords_and_autofill_fake_data.js';
import {simulateStoredAccounts, simulateSyncStatus} from 'chrome://test/settings/sync_test_util.m.js';
import {TestPasswordManagerProxy} from 'chrome://test/settings/test_password_manager_proxy.js';
......@@ -15,13 +15,10 @@ import {TestPasswordManagerProxy} from 'chrome://test/settings/test_password_man
* Sets the fake data and creates the element for testing.
* @param {!TestPasswordManagerProxy} passwordManager
* @param {!Array<!chrome.passwordsPrivate.PasswordUiEntry>} passwordList
* @param {!Array<!chrome.passwordsPrivate.ExceptionEntry>} exceptionList
* @return {!Object}
*/
function createPasswordsDeviceSection(
passwordManager, passwordList, exceptionList) {
function createPasswordsDeviceSection(passwordManager, passwordList) {
passwordManager.data.passwords = passwordList;
passwordManager.data.exceptions = exceptionList;
const passwordsDeviceSection =
document.createElement('passwords-device-section');
document.body.appendChild(passwordsDeviceSection);
......@@ -51,27 +48,6 @@ function validatePasswordsSubsection(subsection, expectedPasswords) {
}
}
/**
* @param {!Element} subsection
* @param {!Array<!MultiStoreExceptionEntry} expectedExceptions
*/
function validateExceptionsSubsection(subsection, expectedExceptions) {
assertTrue(!!subsection);
const listItemElements = subsection.querySelectorAll('.list-item');
assertEquals(expectedExceptions.length, listItemElements.length);
for (let index = 0; index < expectedExceptions.length; ++index) {
const expectedException = expectedExceptions[index];
const listItemElement = listItemElements[index];
assertTrue(!!listItemElement);
assertEquals(
expectedException.urls.shown,
listItemElement.querySelector('#exception').innerText);
assertEquals(
expectedException.urls.link,
listItemElement.querySelector('#exception').href);
}
}
suite('PasswordsDeviceSection', function() {
/** @type {TestPasswordManagerProxy} */
let passwordManager = null;
......@@ -97,23 +73,17 @@ suite('PasswordsDeviceSection', function() {
passwordManager.setIsOptedInForAccountStorageAndNotify(false);
});
// Test verifies that the fallback text is displayed when passwords/exceptions
// are not present.
// Test verifies that the fallback text is displayed when passwords are not
// present.
test('verifyPasswordsEmptySubsections', function() {
const passwordsDeviceSection =
createPasswordsDeviceSection(passwordManager, [], []);
createPasswordsDeviceSection(passwordManager, []);
assertFalse(passwordsDeviceSection.shadowRoot
.querySelector('#noDeviceOnlyPasswordsLabel')
.hidden);
assertFalse(passwordsDeviceSection.shadowRoot
.querySelector('#noDeviceAndAccountPasswordsLabel')
.hidden);
assertFalse(passwordsDeviceSection.shadowRoot
.querySelector('#noDeviceOnlyExceptionsLabel')
.hidden);
assertFalse(passwordsDeviceSection.shadowRoot
.querySelector('#noDeviceAndAccountExceptionsLabel')
.hidden);
});
// Test verifies that account passwords are not displayed, whereas
......@@ -130,15 +100,13 @@ suite('PasswordsDeviceSection', function() {
{username: 'both', frontendId: 42, id: 3, fromAccountStore: true});
// Shuffle entries a little.
const passwordsDeviceSection = createPasswordsDeviceSection(
passwordManager,
[
const passwordsDeviceSection =
createPasswordsDeviceSection(passwordManager, [
devicePassword,
deviceCopyPassword,
accountPassword,
accountCopyPassword,
],
[]);
]);
validatePasswordsSubsection(
passwordsDeviceSection.$.deviceOnlyPasswordList, [
......@@ -155,12 +123,6 @@ suite('PasswordsDeviceSection', function() {
assertTrue(passwordsDeviceSection.shadowRoot
.querySelector('#noDeviceAndAccountPasswordsLabel')
.hidden);
assertFalse(passwordsDeviceSection.shadowRoot
.querySelector('#noDeviceOnlyExceptionsLabel')
.hidden);
assertFalse(passwordsDeviceSection.shadowRoot
.querySelector('#noDeviceAndAccountExceptionsLabel')
.hidden);
});
// Test verifies that removing the device copy of a duplicated password
......@@ -172,7 +134,7 @@ suite('PasswordsDeviceSection', function() {
];
const passwordsDeviceSection =
createPasswordsDeviceSection(passwordManager, passwordList, []);
createPasswordsDeviceSection(passwordManager, passwordList);
validatePasswordsSubsection(
passwordsDeviceSection.$.deviceOnlyPasswordList, []);
validatePasswordsSubsection(
......@@ -200,7 +162,7 @@ suite('PasswordsDeviceSection', function() {
];
const passwordsDeviceSection =
createPasswordsDeviceSection(passwordManager, passwordList, []);
createPasswordsDeviceSection(passwordManager, passwordList);
validatePasswordsSubsection(
passwordsDeviceSection.$.deviceOnlyPasswordList, []);
validatePasswordsSubsection(
......@@ -220,49 +182,6 @@ suite('PasswordsDeviceSection', function() {
passwordsDeviceSection.$.deviceAndAccountPasswordList, []);
});
// Test verifies that account exceptions are not displayed, whereas
// device-only and device-and-account ones end up in the correct subsection.
test('verifyExceptionsFilledSubsections', function() {
const deviceException = createExceptionEntry(
{url: 'device.com', id: 0, fromAccountStore: false});
const accountException = createExceptionEntry(
{url: 'account.com', id: 1, fromAccountStore: true});
// Create duplicate that gets merged.
const deviceCopyException = createExceptionEntry(
{url: 'both.com', frontendId: 42, id: 2, fromAccountStore: false});
const accountCopyException = createExceptionEntry(
{url: 'both.com', frontendId: 42, id: 3, fromAccountStore: true});
// Shuffle entries a little.
const passwordsDeviceSection =
createPasswordsDeviceSection(passwordManager, [], [
deviceException,
deviceCopyException,
accountException,
accountCopyException,
]);
validateExceptionsSubsection(
passwordsDeviceSection.$.deviceOnlyExceptionsList,
[createMultiStoreExceptionEntry({url: 'device.com', deviceId: 0})]);
validateExceptionsSubsection(
passwordsDeviceSection.$.deviceAndAccountExceptionsList,
[createMultiStoreExceptionEntry(
{url: 'both.com', deviceId: 2, accountId: 3})]);
assertFalse(passwordsDeviceSection.shadowRoot
.querySelector('#noDeviceOnlyPasswordsLabel')
.hidden);
assertFalse(passwordsDeviceSection.shadowRoot
.querySelector('#noDeviceAndAccountPasswordsLabel')
.hidden);
assertTrue(passwordsDeviceSection.shadowRoot
.querySelector('#noDeviceOnlyExceptionsLabel')
.hidden);
assertTrue(passwordsDeviceSection.shadowRoot
.querySelector('#noDeviceAndAccountExceptionsLabel')
.hidden);
});
// Test verifies that the overflow menu offers an option to move a password
// to the account and that it has the right text.
test('hasMoveToAccountOption', 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