Commit 5dd9a9b4 authored by Severin's avatar Severin
Browse files

Bug 1630191 - enable 'show password' toggle after deleting autofilled...

Bug 1630191 - enable 'show password' toggle after deleting autofilled password. r=sfoster,MattN, a=pascalc

Differential Revision: https://phabricator.services.mozilla.com/D72091
parent fedda084
Loading
Loading
Loading
Loading
+37 −17
Original line number Diff line number Diff line
@@ -283,15 +283,19 @@ const observer = {

      case "input": {
        let field = aEvent.target;
        let { hasBeenTypePassword } = field;
        let isPasswordType = LoginHelper.isPasswordFieldType(field);
        // React to input into fields filled with generated passwords.
        if (docState.generatedPasswordFields.has(field)) {
          LoginManagerChild.forWindow(
            window
          )._maybeStopTreatingAsGeneratedPasswordField(aEvent);
        let loginManagerChild = LoginManagerChild.forWindow(window);
        if (
          docState.generatedPasswordFields.has(field) &&
          loginManagerChild._doesEventClearPrevFieldValue(aEvent)
        ) {
          loginManagerChild._stopTreatingAsGeneratedPasswordField(
            aEvent.target
          );
        }

        if (!hasBeenTypePassword && !LoginHelper.isUsernameFieldType(field)) {
        if (!isPasswordType && !LoginHelper.isUsernameFieldType(field)) {
          break;
        }

@@ -311,14 +315,14 @@ const observer = {
        // don't flag as user-modified if the form was autofilled and doesn't appear to have changed
        let isAutofillInput = filledLogin && !fillWasUserTriggered;
        if (!alreadyModified && isAutofillInput) {
          if (hasBeenTypePassword && filledLogin.password == field.value) {
          if (isPasswordType && filledLogin.password == field.value) {
            log(
              "Ignoring password input event that doesn't change autofilled values"
            );
            break;
          }
          if (
            !hasBeenTypePassword &&
            !isPasswordType &&
            filledLogin.usernameField &&
            filledLogin.username == field.value
          ) {
@@ -330,6 +334,20 @@ const observer = {
        }
        docState.fieldModificationsByRootElement.set(formLikeRoot, true);

        if (
          // When the password field value is cleared or entirely replaced we don't treat it as
          // an autofilled form any more. We don't do the same for username edits to avoid snooping
          // on the autofilled password in the resulting doorhanger
          isPasswordType &&
          loginManagerChild._doesEventClearPrevFieldValue(aEvent) &&
          // Don't clear last recorded autofill if THIS is an autofilled value. This will be true
          // when filling from the context menu.
          filledLogin &&
          filledLogin.password !== field.value
        ) {
          docState.fillsByRootElement.delete(formLikeRoot);
        }

        break;
      }

@@ -1684,15 +1702,17 @@ this.LoginManagerChild = class LoginManagerChild extends JSWindowActorChild {
    }
  }

  _maybeStopTreatingAsGeneratedPasswordField(event) {
    let passwordField = event.target;
    let { value } = passwordField;

    // If the field is now empty or the inserted text replaced the whole value
    // then stop treating it as a generated password field.
    if (!value || (event.data && event.data == value)) {
      this._stopTreatingAsGeneratedPasswordField(passwordField);
    }
  /**
   * Heuristic for whether or not we should consider [field]s value to be 'new' (as opposed to
   * 'changed') after applying [event].
   *
   * @param {HTMLInputElement} event.target input element being changed.
   * @param {string?} event.data new value being input into the field.
   *
   * @returns {boolean}
   */
  _doesEventClearPrevFieldValue({ target, data }) {
    return !target.value || (data && data == target.value);
  }

  _stopTreatingAsGeneratedPasswordField(passwordField) {
+1 −0
Original line number Diff line number Diff line
@@ -96,6 +96,7 @@ skip-if = verify
support-files =
  subtst_privbrowsing_1.html
  form_password_change.html
[browser_test_changeContentInputValue.js]
[browser_username_select_dialog.js]
support-files =
  subtst_notifications_change_p.html
+1 −9
Original line number Diff line number Diff line
@@ -214,15 +214,7 @@ let testCases = [
        username: "user",
        password: "pass",
      },
      // Eventually we'll want to remove the doorhanger in this case
      doorhanger: {
        type: "password-change",
        dismissed: true,
        anchorExtraAttr: "",
        username: "user1",
        password: "pass1",
        toggle: "visible",
      },
      doorhanger: null,
    },
  },
];
+55 −1
Original line number Diff line number Diff line
/* eslint no-shadow:"off" */

const passwordInputSelector = "#form-basic-password";
const usernameInputSelector = "#form-basic-username";
const FORM_URL =
@@ -119,6 +121,50 @@ let testCases = [
      },
    },
  },
  {
    /* Test that the reveal password checkbox is shown when editing the
     * password of a login that has been autofilled and then deleted
     */
    name: "test_autofilled_cleared_then_updated_password",
    logins: [{ username: "username1", password: "password" }],
    formDefaults: {},
    formChanges: [
      {
        [passwordInputSelector]: "",
      },
      {
        [passwordInputSelector]: "password!",
      },
    ],
    expected: {
      initialForm: {
        username: "username1",
        password: "password",
      },
      passwordChangedDoorhanger: {
        type: "password-change",
        dismissed: true,
        username: "username1",
        password: "password!",
        toggleVisible: true,
        initialToggleState: {
          inputType: "password",
          toggleChecked: false,
        },
      },
      submitDoorhanger: {
        type: "password-change",
        dismissed: false,
        username: "username1",
        password: "password!",
        toggleVisible: true,
        initialToggleState: {
          inputType: "password",
          toggleChecked: false,
        },
      },
    },
  },
  {
    /* Test that the reveal password checkbox is hidden when editing the
     * username of an autofilled login
@@ -185,6 +231,8 @@ async function testDoorhangerToggles({
  expected,
  enabledMasterPassword,
}) {
  formChanges = Array.isArray(formChanges) ? formChanges : [formChanges];

  for (let login of logins) {
    await LoginTestUtils.addLogin(login);
  }
@@ -210,7 +258,12 @@ async function testDoorhangerToggles({
      let formChanged = expected.passwordChangedDoorhanger
        ? listenForTestNotification("PasswordEditedOrGenerated")
        : Promise.resolve();
      await changeContentFormValues(browser, formChanges);
      for (let change of formChanges) {
        await changeContentFormValues(browser, change, {
          method: "paste_text",
        });
      }

      await formChanged;

      if (expected.passwordChangedDoorhanger) {
@@ -313,6 +366,7 @@ async function verifyDoorhangerToggles(browser, notif, expected) {
      "The visibility checkbox is hidden"
    );
  }

  if (initialToggleState) {
    is(
      toggleCheckbox.checked,
+132 −0
Original line number Diff line number Diff line
/**
 * Tests head.js#changeContentInputValue.
 */

"use strict";

// The origin for the test URIs.
const TEST_ORIGIN = "https://example.com";
const BASIC_FORM_PAGE_PATH = DIRECTORY_PATH + "form_basic.html";
const USERNAME_INPUT_SELECTOR = "#form-basic-username";

let testCases = [
  {
    name: "blank string should clear input value",
    originalValue: "start text",
    inputEvent: "",
    expectedKeypresses: ["Backspace"],
  },
  {
    name:
      "input value that adds to original string should only add the difference",
    originalValue: "start text",
    inputEvent: "start text!!!",
    expectedKeypresses: ["!", "!", "!"],
  },
  {
    name:
      "input value that is a subset of original string should only delete the difference",
    originalValue: "start text",
    inputEvent: "start",
    expectedKeypresses: ["Backspace"],
  },
  {
    name:
      "input value that is unrelated to the original string should replace it",
    originalValue: "start text",
    inputEvent: "wut?",
    expectedKeypresses: ["w", "u", "t", "?"],
  },
];

for (let testData of testCases) {
  let tmp = {
    async [testData.name]() {
      await testStringChange(testData);
    },
  };
  add_task(tmp[testData.name]);
}

async function testStringChange({
  name,
  originalValue,
  inputEvent,
  expectedKeypresses,
}) {
  info("Starting test " + name);
  await LoginTestUtils.clearData();

  await LoginTestUtils.addLogin({
    username: originalValue,
    password: "password",
  });

  let formProcessedPromise = listenForTestNotification("FormProcessed");
  let url = TEST_ORIGIN + BASIC_FORM_PAGE_PATH;
  info("Opening tab with url: " + url);

  await BrowserTestUtils.withNewTab(
    {
      gBrowser,
      url,
    },
    async function(browser) {
      info(`Opened tab with url: ${url}, waiting for focus`);
      await SimpleTest.promiseFocus(browser.ownerGlobal);
      info("Waiting for form-processed message");
      await formProcessedPromise;
      await checkForm(browser, originalValue);
      info("form checked");

      await ContentTask.spawn(
        browser,
        { USERNAME_INPUT_SELECTOR, expectedKeypresses },
        async function({ USERNAME_INPUT_SELECTOR, expectedKeypresses }) {
          let input = content.document.querySelector(USERNAME_INPUT_SELECTOR);

          let verifyKeyListener = event => {
            is(
              expectedKeypresses[0],
              event.key,
              "Key press matches expected value"
            );
            expectedKeypresses.shift();

            if (!expectedKeypresses.length) {
              input.removeEventListner("keydown", verifyKeyListener);
              input.addEventListener("keydown", () => {
                throw new Error("Unexpected keypress encountered");
              });
            }
          };

          input.addEventListener("keydown", verifyKeyListener);
        }
      );

      changeContentInputValue(browser, USERNAME_INPUT_SELECTOR, inputEvent);
    }
  );
}

async function checkForm(browser, expectedUsername) {
  await ContentTask.spawn(
    browser,
    {
      expectedUsername,
      USERNAME_INPUT_SELECTOR,
    },
    async function contentCheckForm({
      expectedUsername,
      USERNAME_INPUT_SELECTOR,
    }) {
      let field = content.document.querySelector(USERNAME_INPUT_SELECTOR);
      is(
        field.value,
        expectedUsername,
        `Username field has teh expected initial value '${expectedUsername}'`
      );
    }
  );
}
Loading