Bug 1243722 - Introduce notUsernameSelector in passwordmgr recipes, r=MattN

Additionally, add a recipe for Facebook. MozReview-Commit-ID: 2l60v7R0TPP --HG-- extra : rebase_source : ac46dd5d759613d540cf4145c24610b279015c0c
2016-04-01 14:25:14 +08:00 · 2016-04-01 14:25:14 +08:00 · 30229f6ecd
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@ -663,10 +663,17 @@ var LoginManagerContent = {
      // already logged in to the site.
      for (var i = pwFields[0].index - 1; i >= 0; i--) {
        var element = form.elements[i];
-        if (LoginHelper.isUsernameFieldType(element)) {
-          usernameField = element;
-          break;
+        if (!LoginHelper.isUsernameFieldType(element)) {
+          continue;
        }
+
+        if (fieldOverrideRecipe && fieldOverrideRecipe.notUsernameSelector &&
+            element.matches(fieldOverrideRecipe.notUsernameSelector)) {
+          continue;
+        }
+
+        usernameField = element;
+        break;
      }
    }

--- a/toolkit/components/passwordmgr/LoginRecipes.jsm
+++ b/toolkit/components/passwordmgr/LoginRecipes.jsm
@ -8,7 +8,7 @@ this.EXPORTED_SYMBOLS = ["LoginRecipesContent", "LoginRecipesParent"];

 const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
 const REQUIRED_KEYS = ["hosts"];
-const OPTIONAL_KEYS = ["description", "passwordSelector", "pathRegex", "usernameSelector"];
+const OPTIONAL_KEYS = ["description", "notUsernameSelector", "passwordSelector", "pathRegex", "usernameSelector"];
 const SUPPORTED_KEYS = REQUIRED_KEYS.concat(OPTIONAL_KEYS);

 Cu.importGlobalProperties(["URL"]);
@ -227,7 +227,8 @@ var LoginRecipesContent = {
    let chosenRecipe = null;
    // Find the first (most-specific recipe that involves field overrides).
    for (let recipe of recipes) {
-      if (!recipe.usernameSelector && !recipe.passwordSelector) {
+      if (!recipe.usernameSelector && !recipe.passwordSelector &&
+          !recipe.notUsernameSelector) {
        continue;
      }

--- a/toolkit/components/passwordmgr/content/recipes.json
+++ b/toolkit/components/passwordmgr/content/recipes.json
@ -21,6 +21,11 @@
      "usernameSelector": "#accountname, input[name='loginname']",
      "passwordSelector": "#password1, input[name='loginpassword']",
      "pathRegex": "^\/account\/"
+    },
+    {
+      "description": "Username field will be incorrectly captured in the change password form (bug 1243722)",
+      "hosts": ["www.facebook.com"],
+      "notUsernameSelector": "#password_strength"
    }
  ]
 }
--- a/toolkit/components/passwordmgr/test/mochitest/test_recipe_login_fields.html
+++ b/toolkit/components/passwordmgr/test/mochitest/test_recipe_login_fields.html
@ -4,21 +4,41 @@
  <meta charset="utf-8">
  <title>Test for recipes overriding login fields</title>
  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="/tests/SimpleTest/SpawnTask.js"></script>
+  <script src="pwmgr_common.js"></script>
  <link rel="stylesheet" href="/tests/SimpleTest/test.css" />
 </head>
 <body>
-<script type="application/javascript;version=1.8">
-SimpleTest.waitForExplicitFinish();
+<script>
+var chromeScript = runChecksAfterCommonInit();

-const PWMGR_COMMON_URL = SimpleTest.getTestFileURL("pwmgr_common.js");
-let pwmgrCommonScript = SpecialPowers.loadChromeScript(PWMGR_COMMON_URL);
+let fillPromiseResolvers = [];

-// Tell the parent to setup test logins.
-pwmgrCommonScript.sendAsyncMessage("setupParent", { selfFilling: true });
+function waitForFills(fillCount) {
+  let promises = [];
+  while (fillCount--) {
+    let promise = new Promise(resolve => fillPromiseResolvers.push(resolve));
+    promises.push(promise);
+  }

-// When the setup is done, load a recipe for this test.
-pwmgrCommonScript.addMessageListener("doneSetup", function doneSetup() {
-  pwmgrCommonScript.sendAsyncMessage("loadRecipes", {
+  return Promise.all(promises);
+}
+
+add_task(function* setup() {
+  document.getElementById("content")
+  .addEventListener("input", function handleInputEvent(evt) {
+    let resolve = fillPromiseResolvers.shift();
+    if (!resolve) {
+      ok(false, "Too many fills");
+      return;
+    }
+
+    resolve(evt.target);
+  });
+});
+
+add_task(function* loadUsernamePasswordSelectorRecipes() {
+  yield loadRecipes({
    siteRecipes: [{
      hosts: ["mochi.test:8888"],
      usernameSelector: "input[name='uname1']",
@ -27,39 +47,80 @@ pwmgrCommonScript.addMessageListener("doneSetup", function doneSetup() {
  });
 });

-pwmgrCommonScript.addMessageListener("loadedRecipes", function loadedRecipes() {
-  // Append the form dynamically so autofill is triggered after setup above.
-  document.getElementById("content").innerHTML += `
-<!-- form with recipe for the username and password -->
-  <form id="form1">
-    <input type="text"     name="uname1" oninput="reportFill(this, true)">
-    <input type="text"     name="uname2" oninput="reportFill(this, false)">
-    <input type="password" name="pword1" oninput="reportFill(this, false)">
-    <input type="password" name="pword2" oninput="reportFill(this, true)">
-  </form>`;
+add_task(function* testOverriddingFields() {
+  // Insert the form dynamically so autofill is triggered after setup above.
+  document.getElementById("content").innerHTML = `
+    <!-- form with recipe for the username and password -->
+      <form id="form1">
+        <input type="text"     name="uname1" data-expected="true">
+        <input type="text"     name="uname2" data-expected="false">
+        <input type="password" name="pword1" data-expected="false">
+        <input type="password" name="pword2" data-expected="true">
+      </form>`;
+
+  let elements = yield waitForFills(2);
+  for (let element of elements) {
+    is(element.dataset.expected, "true", `${element.name} was filled`);
+  }
 });

-const EXPECTED_FILLS = 4;
-let fillCount = 0;
+add_task(function* testDefaultHeuristics() {
+  // Insert the form dynamically so autofill is triggered after setup above.
+  document.getElementById("content").innerHTML = `
+    <!-- Fallback to the default heuristics since the selectors don't match -->
+    <form id="form2">
+      <input type="text"     name="uname3" data-expected="false">
+      <input type="text"     name="uname4" data-expected="true">
+      <input type="password" name="pword3" data-expected="true">
+      <input type="password" name="pword4" data-expected="false">
+    </form>`;

-function reportFill(element, expected) {
-  ok(expected, `${element.name} was filled`);
-  if (++fillCount == EXPECTED_FILLS) {
-    pwmgrCommonScript.destroy();
-    SimpleTest.finish();
-  } else if (fillCount == 2) {
-    document.getElementById("content").innerHTML = `
-  <!-- Fallback to the default heuristics since the selectors don't match -->
-  <form id="form2">
-    <input type="text"     name="uname3" oninput="reportFill(this, false)">
-    <input type="text"     name="uname4" oninput="reportFill(this, true)">
-    <input type="password" name="pword3" oninput="reportFill(this, true)">
-    <input type="password" name="pword4" oninput="reportFill(this, false)">
-  </form>`;
-  } else if (fillCount > EXPECTED_FILLS) {
-    ok(false, "Too many fills");
+  let elements = yield waitForFills(2);
+  for (let element of elements) {
+    is(element.dataset.expected, "true", `${element.name} was filled`);
  }
-}
+});
+
+add_task(function* loadNotUsernameSelectorRecipes() {
+  yield resetRecipes();
+  yield loadRecipes({
+    siteRecipes: [{
+      hosts: ["mochi.test:8888"],
+      notUsernameSelector: "input[name='not_uname1']"
+    }],
+  });
+});
+
+add_task(function* testNotUsernameField() {
+  document.getElementById("content").innerHTML = `
+    <!-- The field matching notUsernameSelector should be skipped -->
+    <form id="form3">
+      <input type="text"     name="uname5" data-expected="true">
+      <input type="text"     name="not_uname1" data-expected="false">
+      <input type="password" name="pword5" data-expected="true">
+    </form>`;
+
+  let elements = yield waitForFills(2);
+  for (let element of elements) {
+    is(element.dataset.expected, "true", `${element.name} was filled`);
+  }
+});
+
+add_task(function* testNotUsernameFieldNoUsername() {
+  document.getElementById("content").innerHTML = `
+    <!-- The field matching notUsernameSelector should be skipped.
+         No username field should be found and filled in this case -->
+    <form id="form4">
+      <input type="text"     name="not_uname1" data-expected="false">
+      <input type="password" name="pword6" data-expected="true">
+    </form>`;
+
+  let elements = yield waitForFills(1);
+  for (let element of elements) {
+    is(element.dataset.expected, "true", `${element.name} was filled`);
+  }
+});
+
 </script>

 <p id="display"></p>
--- a/toolkit/components/passwordmgr/test/pwmgr_common.js
+++ b/toolkit/components/passwordmgr/test/pwmgr_common.js
@ -282,6 +282,26 @@ function promiseFormsProcessed(expectedCount = 1) {
  });
 }

+function loadRecipes(recipes) {
+  return new Promise(resolve => {
+    chromeScript.addMessageListener("loadedRecipes", function loaded() {
+      chromeScript.removeMessageListener("loadedRecipes", loaded);
+      resolve(recipes);
+    });
+    chromeScript.sendAsyncMessage("loadRecipes", recipes);
+  });
+}
+
+function resetRecipes() {
+  return new Promise(resolve => {
+    chromeScript.addMessageListener("recipesReset", function reset() {
+      chromeScript.removeMessageListener("recipesReset", reset);
+      resolve();
+    });
+    chromeScript.sendAsyncMessage("resetRecipes");
+  });
+}
+
 /**
 * Run a function synchronously in the parent process and destroy it in the test cleanup function.
 * @param {Function|String} aFunctionOrURL - either a function that will be stringified and run
@ -310,6 +330,7 @@ function runChecksAfterCommonInit(aFunction = null) {
    pwmgrCommonScript.addMessageListener("registerRunTests", () => registerRunTests());
  }
  pwmgrCommonScript.sendSyncMessage("setupParent");
+  return pwmgrCommonScript;
 }

 // Code to run when loaded as a chrome script in tests via loadChromeScript
@ -334,6 +355,13 @@ if (this.addMessageListener) {
    sendAsyncMessage("loadedRecipes", recipes);
  }));

+  addMessageListener("resetRecipes", Task.async(function* resetRecipes() {
+    let { LoginManagerParent } = Cu.import("resource://gre/modules/LoginManagerParent.jsm", {});
+    let recipeParent = yield LoginManagerParent.recipeParentPromise;
+    yield recipeParent.reset();
+    sendAsyncMessage("recipesReset");
+  }));
+
  var globalMM = Cc["@mozilla.org/globalmessagemanager;1"].getService(Ci.nsIMessageListenerManager);
  globalMM.addMessageListener("RemoteLogins:onFormSubmit", function onFormSubmit(message) {
    sendAsyncMessage("formSubmissionProcessed", message.data, message.objects);