Bug 1570475 - Saving a login should select the login. r=MattN

The AboutLoginsCreateLogin event was being used in two different contexts, one to signal to AboutLoginsChild.jsm that a new login was created and in a second context to signal within the page that the user wanted to start creating a new login. This overloading of the event has now been fixed. There was also another issue when comparing the typed origin to the saved origin which has been fixed. This patch also properly selects a different login in the list when the current one is deleted.

Differential Revision: https://phabricator.services.mozilla.com/D40130

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Jared Wein 2019-08-02 20:14:47 +00:00
Родитель 48baecedd5
Коммит bea5604f59
7 изменённых файлов: 85 добавлений и 49 удалений

Просмотреть файл

@ -52,6 +52,9 @@ class AboutLoginsChild extends ActorChild {
doLoginsMatch(loginA, loginB) {
return LoginHelper.doLoginsMatch(loginA, loginB, {});
},
getLoginOrigin(uriString) {
return LoginHelper.getLoginOrigin(uriString);
},
promptForMasterPassword(resolve) {
masterPasswordPromise = {
resolve,

Просмотреть файл

@ -72,9 +72,9 @@ export default class LoginItem extends HTMLElement {
this._openSiteButton.addEventListener("click", this);
this._originInput.addEventListener("click", this);
this._revealCheckbox.addEventListener("click", this);
window.addEventListener("AboutLoginsCreateLogin", this);
window.addEventListener("AboutLoginsInitialLoginSelected", this);
window.addEventListener("AboutLoginsLoginSelected", this);
window.addEventListener("AboutLoginsShowBlankLogin", this);
}
async render() {
@ -117,10 +117,6 @@ export default class LoginItem extends HTMLElement {
async handleEvent(event) {
switch (event.type) {
case "AboutLoginsCreateLogin": {
this.setLogin({});
break;
}
case "AboutLoginsInitialLoginSelected": {
this.setLogin(event.detail, { skipFocusChange: true });
break;
@ -145,6 +141,10 @@ export default class LoginItem extends HTMLElement {
}
break;
}
case "AboutLoginsShowBlankLogin": {
this.setLogin({});
break;
}
case "blur": {
// Add https:// prefix if one was not provided.
let originValue = this._originInput.value.trim();
@ -176,14 +176,19 @@ export default class LoginItem extends HTMLElement {
} else {
this.setLogin(this._login);
}
} else {
} else if (!this.hasPendingChanges()) {
window.dispatchEvent(new CustomEvent("AboutLoginsClearSelection"));
recordTelemetryEvent({
object: "new_login",
method: "cancel",
});
} else {
this.showConfirmationDialog("discard-changes", () => {
window.dispatchEvent(
new CustomEvent("AboutLoginsClearSelection")
);
});
}
recordTelemetryEvent({
object: wasExistingLogin ? "existing_login" : "new_login",
method: "cancel",
});
return;
}
if (
@ -223,7 +228,14 @@ export default class LoginItem extends HTMLElement {
return;
}
if (classList.contains("delete-button")) {
this.confirmDelete();
this.showConfirmationDialog("delete", () => {
document.dispatchEvent(
new CustomEvent("AboutLoginsDeleteLogin", {
bubbles: true,
detail: this._login,
})
);
});
return;
}
if (classList.contains("edit-button")) {
@ -307,27 +319,24 @@ export default class LoginItem extends HTMLElement {
break;
}
}
let wasExistingLogin = !!this._login.guid;
let method = type == "delete" ? "delete" : "cancel";
let dialogPromise = dialog.show(options);
dialogPromise.then(onConfirm, () => {});
dialogPromise.then(
() => {
try {
onConfirm();
} catch (ex) {}
recordTelemetryEvent({
object: wasExistingLogin ? "existing_login" : "new_login",
method,
});
},
() => {}
);
return dialogPromise;
}
/**
* Toggles the confirm delete dialog, completing the deletion if the user
* agrees.
*/
confirmDelete() {
this.showConfirmationDialog("delete", () => {
document.dispatchEvent(
new CustomEvent("AboutLoginsDeleteLogin", {
bubbles: true,
detail: this._login,
})
);
recordTelemetryEvent({ object: "existing_login", method: "delete" });
});
}
hasPendingChanges() {
let { origin = "", username = "", password = "" } = this._login || {};
@ -381,9 +390,14 @@ export default class LoginItem extends HTMLElement {
return;
}
this._toggleEditing(false);
this._login = login;
this.render();
this.dispatchEvent(
new CustomEvent("AboutLoginsLoginSelected", {
bubbles: true,
composed: true,
detail: login,
})
);
}
/**
@ -449,7 +463,8 @@ export default class LoginItem extends HTMLElement {
return {
username: this._usernameInput.value.trim(),
password: this._passwordInput.value.trim(),
origin: this._originInput.value.trim(),
origin:
window.AboutLoginsUtils.getLoginOrigin(this._originInput.value) || "",
};
}

Просмотреть файл

@ -45,10 +45,10 @@ export default class LoginList extends HTMLElement {
.getElementById("login-sort")
.addEventListener("change", this);
window.addEventListener("AboutLoginsClearSelection", this);
window.addEventListener("AboutLoginsCreateLogin", this);
window.addEventListener("AboutLoginsFilterLogins", this);
window.addEventListener("AboutLoginsInitialLoginSelected", this);
window.addEventListener("AboutLoginsLoginSelected", this);
window.addEventListener("AboutLoginsShowBlankLogin", this);
this._list.addEventListener("click", this);
this.addEventListener("keydown", this);
this._createLoginButton.addEventListener("click", this);
@ -105,7 +105,7 @@ export default class LoginList extends HTMLElement {
switch (event.type) {
case "click": {
if (event.originalTarget == this._createLoginButton) {
window.dispatchEvent(new CustomEvent("AboutLoginsCreateLogin"));
window.dispatchEvent(new CustomEvent("AboutLoginsShowBlankLogin"));
recordTelemetryEvent({ object: "new_login", method: "new" });
return;
}
@ -152,11 +152,6 @@ export default class LoginList extends HTMLElement {
}
break;
}
case "AboutLoginsCreateLogin": {
this._selectedGuid = null;
this._setListItemAsSelected(this._blankLoginListItem);
break;
}
case "AboutLoginsFilterLogins": {
this._filter = event.detail.toLocaleLowerCase();
this.render();
@ -178,6 +173,11 @@ export default class LoginList extends HTMLElement {
}
break;
}
case "AboutLoginsShowBlankLogin": {
this._selectedGuid = null;
this._setListItemAsSelected(this._blankLoginListItem);
break;
}
case "keydown": {
this._handleKeyboardNav(event);
break;
@ -271,10 +271,15 @@ export default class LoginList extends HTMLElement {
let index = this._loginGuidsSortedOrder.indexOf(login.guid);
if (this._loginGuidsSortedOrder.length > 1) {
let newlySelectedIndex = index > 0 ? index - 1 : index + 1;
let newlySelectedListItem = this._logins[
let newlySelectedLogin = this._logins[
this._loginGuidsSortedOrder[newlySelectedIndex]
].listItem;
this._setListItemAsSelected(newlySelectedListItem);
].login;
window.dispatchEvent(
new CustomEvent("AboutLoginsLoginSelected", {
detail: newlySelectedLogin,
cancelable: true,
})
);
}
}

Просмотреть файл

@ -12,6 +12,7 @@ function waitForTelemetryEventCount(count) {
Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS,
false
).content;
info("got " + (events && events.length) + " events");
return events && events.length == count;
}, "waiting for telemetry event count of: " + count);
}

Просмотреть файл

@ -13,6 +13,12 @@ add_task(async function setup() {
});
add_task(async function test_create_login() {
let browser = gBrowser.selectedBrowser;
await ContentTask.spawn(browser, null, async () => {
let loginList = Cu.waiveXrays(content.document.querySelector("login-list"));
ok(!loginList._selectedGuid, "should not be a selected guid by default");
});
let testCases = [
["ftp://ftp.example.com/", "ftp://ftp.example.com"],
["https://example.com/foo", "https://example.com"],
@ -32,7 +38,6 @@ add_task(async function test_create_login() {
(_, data) => data == "addLogin"
);
let browser = gBrowser.selectedBrowser;
await ContentTask.spawn(browser, originTuple, async aOriginTuple => {
let createButton = content.document
.querySelector("login-list")
@ -87,7 +92,16 @@ add_task(async function test_create_login() {
}, "Waiting for login to be displayed");
ok(loginGuid, "Expected login found in login-list");
let loginItem = Cu.waiveXrays(
content.document.querySelector("login-item")
);
is(loginItem._login.guid, loginGuid, "login-item should match");
let { login, listItem } = loginList._logins[loginGuid];
ok(
listItem.classList.contains("selected"),
"list item should be selected"
);
ok(
!!listItem,
`Stored login should only include the origin of the URL provided during creation (${
@ -104,12 +118,7 @@ add_task(async function test_create_login() {
"testpass1",
"Stored login should have password provided during creation"
);
listItem.click();
let loginItem = Cu.waiveXrays(
content.document.querySelector("login-item")
);
is(loginItem._login.guid, login.guid, "Login should be selected");
let editButton = loginItem.shadowRoot.querySelector(".edit-button");
editButton.click();

Просмотреть файл

@ -56,6 +56,9 @@ Object.defineProperty(window, "AboutLoginsUtils", {
configurable: true,
writable: true,
value: {
getLoginOrigin(uriString) {
return uriString;
},
promptForMasterPassword(resolve) {
resolve(true);
},

Просмотреть файл

@ -131,7 +131,7 @@ add_task(async function test_edit_login() {
is(document.l10n.getAttributes(gLoginItem.shadowRoot.querySelector(".time-changed")).args.timeChanged, TEST_LOGIN_1.timePasswordChanged, "time-changed should be populated");
is(document.l10n.getAttributes(gLoginItem.shadowRoot.querySelector(".time-used")).args.timeUsed, TEST_LOGIN_1.timeLastUsed, "time-used should be populated");
let copyButtons = [...gLoginItem.shadowRoot.querySelectorAll(".copy-button")];
ok(copyButtons.every(button => isHidden(button)), "The copy buttons should be visible when editing a login");
ok(copyButtons.every(button => isHidden(button)), "The copy buttons should be hidden when editing a login");
gLoginItem.shadowRoot.querySelector("input[name='username']").value = "newUsername";
gLoginItem.shadowRoot.querySelector("input[name='password']").value = "newPassword";