From f81c6783749f9ee0425892d3f42086f82de4e813 Mon Sep 17 00:00:00 2001 From: Jared Wein Date: Fri, 14 Jun 2019 13:52:44 +0000 Subject: [PATCH] Bug 1556934 - Don't show invalid form field styling on inputs before the user has modified them or submitted the form. r=MattN Differential Revision: https://phabricator.services.mozilla.com/D34409 --HG-- extra : moz-landing-system : lando --- .../content/components/login-item.js | 21 +++++++- .../content/components/modal-input.js | 54 +++++++++++-------- .../tests/browser/browser_createLogin.js | 12 +++-- .../tests/chrome/test_modal_input.html | 15 ++++-- 4 files changed, 70 insertions(+), 32 deletions(-) diff --git a/browser/components/aboutlogins/content/components/login-item.js b/browser/components/aboutlogins/content/components/login-item.js index db25f94002d5..1b83107444b4 100644 --- a/browser/components/aboutlogins/content/components/login-item.js +++ b/browser/components/aboutlogins/content/components/login-item.js @@ -137,7 +137,7 @@ export default class LoginItem extends ReflectedFluentElement { let title = this.shadowRoot.querySelector(".title"); title.textContent = this._login.title || title.getAttribute("new-login-title"); this.shadowRoot.querySelector(".origin-saved-value").textContent = this._login.origin || ""; - this.shadowRoot.querySelector("input[name='origin']").value = this._login.origin || ""; + this.shadowRoot.querySelector("input[name='origin']").defaultValue = this._login.origin || ""; this.shadowRoot.querySelector("modal-input[name='username']").setAttribute("value", this._login.username || ""); this.shadowRoot.querySelector("modal-input[name='password']").setAttribute("value", this._login.password || ""); } @@ -224,6 +224,11 @@ export default class LoginItem extends ReflectedFluentElement { setLogin(login) { this._login = login; + + let originInput = + this.resetValidation(this.shadowRoot.querySelector("input[name='origin']"), login.origin); + originInput.addEventListener("blur", this); + this.toggleAttribute("isNewLogin", !login.guid); this.toggleEditing(!login.guid); this.render(); @@ -274,6 +279,20 @@ export default class LoginItem extends ReflectedFluentElement { this.toggleAttribute("editing", shouldEdit); } + resetValidation(formElement, value) { + let wasRequired = formElement.hasAttribute("required"); + let newFormElement = document.createElement(formElement.localName); + newFormElement.defaultValue = value || ""; + newFormElement.className = formElement.className; + newFormElement.setAttribute("name", formElement.getAttribute("name")); + newFormElement.setAttribute("type", formElement.getAttribute("type")); + if (wasRequired) { + newFormElement.setAttribute("required", ""); + } + formElement.replaceWith(newFormElement); + return newFormElement; + } + /** * Checks that the edit/new-login form has valid values present for their * respective required fields. diff --git a/browser/components/aboutlogins/content/components/modal-input.js b/browser/components/aboutlogins/content/components/modal-input.js index d3258b2b6855..6aab890600d7 100644 --- a/browser/components/aboutlogins/content/components/modal-input.js +++ b/browser/components/aboutlogins/content/components/modal-input.js @@ -77,7 +77,7 @@ export default class ModalInput extends ReflectedFluentElement { case "editing": { let isEditing = newValue !== null; if (!isEditing) { - this.setAttribute("value", unlockedValue.value); + this.setAttribute("value", unlockedValue.defaultValue); } break; } @@ -107,26 +107,26 @@ export default class ModalInput extends ReflectedFluentElement { } handleEvent(event) { - if (event.type != "click" || - !event.target.classList.contains("reveal-checkbox")) { - return; + switch (event.type) { + case "click": { + let revealCheckbox = event.target; + let lockedValue = this.shadowRoot.querySelector(".locked-value"); + let unlockedValue = this.shadowRoot.querySelector(".unlocked-value"); + if (revealCheckbox.checked) { + lockedValue.textContent = this.value; + unlockedValue.setAttribute("type", "text"); + + recordTelemetryEvent({object: "password", method: "show"}); + } else { + lockedValue.textContent = this.constructor.LOCKED_PASSWORD_DISPLAY; + unlockedValue.setAttribute("type", "password"); + + recordTelemetryEvent({object: "password", method: "hide"}); + } + this.updateRevealCheckboxTitle(); + break; + } } - - let revealCheckbox = event.target; - let lockedValue = this.shadowRoot.querySelector(".locked-value"); - let unlockedValue = this.shadowRoot.querySelector(".unlocked-value"); - if (revealCheckbox.checked) { - lockedValue.textContent = this.value; - unlockedValue.setAttribute("type", "text"); - - recordTelemetryEvent({object: "password", method: "show"}); - } else { - lockedValue.textContent = this.constructor.LOCKED_PASSWORD_DISPLAY; - unlockedValue.setAttribute("type", "password"); - - recordTelemetryEvent({object: "password", method: "hide"}); - } - this.updateRevealCheckboxTitle(); } get value() { @@ -139,12 +139,22 @@ export default class ModalInput extends ReflectedFluentElement { this.setAttribute("value", val); return; } - this.shadowRoot.querySelector(".unlocked-value").value = val; + let unlockedValue = this.shadowRoot.querySelector(".unlocked-value"); + let wasRequired = unlockedValue.hasAttribute("required"); + let newUnlockedValue = document.createElement(unlockedValue.localName); + newUnlockedValue.defaultValue = val || ""; + newUnlockedValue.className = unlockedValue.className; + newUnlockedValue.setAttribute("type", unlockedValue.getAttribute("type")); + if (wasRequired) { + newUnlockedValue.setAttribute("required", ""); + } + unlockedValue.replaceWith(newUnlockedValue); + unlockedValue = newUnlockedValue; let lockedValue = this.shadowRoot.querySelector(".locked-value"); if (this.getAttribute("type") == "password" && val && val.length) { lockedValue.textContent = this.constructor.LOCKED_PASSWORD_DISPLAY; } else { - lockedValue.textContent = val; + lockedValue.textContent = val || ""; } let revealCheckbox = this.shadowRoot.querySelector(".reveal-checkbox"); revealCheckbox.checked = false; diff --git a/browser/components/aboutlogins/tests/browser/browser_createLogin.js b/browser/components/aboutlogins/tests/browser/browser_createLogin.js index 0637dbfae9a5..63a6481bb325 100644 --- a/browser/components/aboutlogins/tests/browser/browser_createLogin.js +++ b/browser/components/aboutlogins/tests/browser/browser_createLogin.js @@ -33,8 +33,10 @@ add_task(async function test_create_login() { let loginItem = Cu.waiveXrays(content.document.querySelector("login-item")); let originInput = loginItem.shadowRoot.querySelector("input[name='origin']"); - let usernameInput = loginItem.shadowRoot.querySelector("modal-input[name='username']"); - let passwordInput = loginItem.shadowRoot.querySelector("modal-input[name='password']"); + let usernameInput = loginItem.shadowRoot.querySelector("modal-input[name='username']") + .shadowRoot.querySelector(".unlocked-value"); + let passwordInput = loginItem.shadowRoot.querySelector("modal-input[name='password']") + .shadowRoot.querySelector(".unlocked-value"); originInput.value = aOriginTuple[0]; usernameInput.value = "testuser1"; @@ -72,8 +74,10 @@ add_task(async function test_create_login() { let editButton = loginItem.shadowRoot.querySelector(".edit-button"); editButton.click(); - let usernameInput = loginItem.shadowRoot.querySelector("modal-input[name='username']"); - let passwordInput = loginItem.shadowRoot.querySelector("modal-input[name='password']"); + let usernameInput = loginItem.shadowRoot.querySelector("modal-input[name='username']") + .shadowRoot.querySelector(".unlocked-value"); + let passwordInput = loginItem.shadowRoot.querySelector("modal-input[name='password']") + .shadowRoot.querySelector(".unlocked-value"); usernameInput.value = "testuser2"; passwordInput.value = "testpass2"; diff --git a/browser/components/aboutlogins/tests/chrome/test_modal_input.html b/browser/components/aboutlogins/tests/chrome/test_modal_input.html index ba440133fdc1..ae8ebf6896b7 100644 --- a/browser/components/aboutlogins/tests/chrome/test_modal_input.html +++ b/browser/components/aboutlogins/tests/chrome/test_modal_input.html @@ -39,7 +39,7 @@ add_task(async function setup() { add_task(async function test_initial_state() { ok(gModalInput, "modalInput exists"); is(gModalInput.shadowRoot.querySelector(".locked-value").textContent, TEST_INPUT_VALUE, "Values are set initially"); - is(gModalInput.shadowRoot.querySelector(".unlocked-value").value, TEST_INPUT_VALUE, "Values are set initially"); + is(gModalInput.shadowRoot.querySelector(".unlocked-value").defaultValue, TEST_INPUT_VALUE, "Values are set initially"); is(getComputedStyle(gModalInput.shadowRoot.querySelector(".locked-value")).display, "block", ".locked-value is visible by default"); is(getComputedStyle(gModalInput.shadowRoot.querySelector(".unlocked-value")).display, "none", ".unlocked-value is hidden by default"); }); @@ -54,11 +54,14 @@ add_task(async function test_editing_set_unset() { const NEW_VALUE = "editedValue"; SpecialPowers.wrap(unlockedValue).setUserInput(NEW_VALUE); gModalInput.removeAttribute("editing"); + unlockedValue = gModalInput.shadowRoot.querySelector(".unlocked-value"); - is(lockedValue.textContent, NEW_VALUE, "Values are updated from edit"); - is(unlockedValue.value, NEW_VALUE, "Values are updated from edit"); - is(gModalInput.getAttribute("value"), NEW_VALUE, "The value attribute on the host element is updated from edit"); + is(lockedValue.textContent, TEST_INPUT_VALUE, "Values are restored to value prior to edit"); + is(unlockedValue.value, TEST_INPUT_VALUE, "Edited value is cleared after 'edit' mode is left"); + is(unlockedValue.defaultValue, TEST_INPUT_VALUE, "Default value is maintained"); + is(gModalInput.getAttribute("value"), TEST_INPUT_VALUE, "The value attribute on the host element is restored"); is(getComputedStyle(lockedValue).display, "block", ".locked-value is visible when not editing"); + unlockedValue = gModalInput.shadowRoot.querySelector(".unlocked-value"); is(getComputedStyle(unlockedValue).display, "none", ".unlocked-value is hidden when not editing"); }); @@ -69,12 +72,13 @@ add_task(async function test_password() { is(lockedValue.textContent, gModalInput.constructor.LOCKED_PASSWORD_DISPLAY, "type=password should display masked characters when locked"); - is(unlockedValue.value, gModalInput.getAttribute("value"), "type=password should have actual value in .unlocked-value"); + is(unlockedValue.defaultValue, gModalInput.getAttribute("value"), "type=password should have actual value in .unlocked-value"); is(unlockedValue.getAttribute("type"), "password", "input[type=password] should be used for .unlocked-value with type=password"); gModalInput.removeAttribute("value"); is(lockedValue.textContent, "", "type=password should display nothing when locked without a value (.locked-value)"); + unlockedValue = gModalInput.shadowRoot.querySelector(".unlocked-value"); is(unlockedValue.value, "", "type=password should display nothing when locked without a value (.unlocked-value)"); }); @@ -86,6 +90,7 @@ add_task(async function test_required() { gModalInput.setAttribute("required", ""); ok(!unlockedValue.checkValidity(), "Setting 'required' on the modal-input should make the unlocked-value required"); + unlockedValue = gModalInput.shadowRoot.querySelector(".unlocked-value"); unlockedValue.value = "foo"; ok(unlockedValue.checkValidity(), "Setting a value on the required modal-input should pass validation"); });