Bug 1329351 - Only autocomplete on password fields which were marked. r=mconley draft
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Thu, 19 Jan 2017 01:06:47 -0800
changeset 463546 1f1560ab6c3735a9c4142a5c2127bf211ad0ba5a
parent 463545 b998d7e26c31103a0d7c23d4039573a9f99b5ad2
child 542704 7f5dce79c3db148bd95a3b5f9fa2f75308cf97d2
push id42095
push usermozilla@noorenberghe.ca
push dateThu, 19 Jan 2017 09:09:08 +0000
reviewersmconley
bugs1329351
milestone53.0a1
Bug 1329351 - Only autocomplete on password fields which were marked. r=mconley MozReview-Commit-ID: 3xNSPrlhOik
toolkit/components/satchel/nsFormFillController.cpp
toolkit/components/satchel/test/mochitest.ini
toolkit/components/satchel/test/test_password_autocomplete.html
--- a/toolkit/components/satchel/nsFormFillController.cpp
+++ b/toolkit/components/satchel/nsFormFillController.cpp
@@ -973,16 +973,24 @@ nsFormFillController::MaybeStartControll
   nsCOMPtr<nsIDOMHTMLElement> datalist;
   aInput->GetList(getter_AddRefs(datalist));
   bool hasList = datalist != nullptr;
 
   bool isPwmgrInput = false;
   if (mPwmgrInputs.Get(inputNode))
       isPwmgrInput = true;
 
+  // Don't show autocomplete on password fields regardless of datalist or
+  // autocomplete being enabled as we don't want to show form history on them.
+  // The GetType() check was more readable than !formControl->IsSingleLineTextControl(true)
+  // but this logic should be kept in-sync with that.
+  if (formControl->GetType() == NS_FORM_INPUT_PASSWORD && !isPwmgrInput) {
+    return;
+  }
+
   if (isPwmgrInput || hasList || autocomplete) {
     StartControllingInput(aInput);
   }
 }
 
 nsresult
 nsFormFillController::Focus(nsIDOMEvent* aEvent)
 {
--- a/toolkit/components/satchel/test/mochitest.ini
+++ b/toolkit/components/satchel/test/mochitest.ini
@@ -9,10 +9,11 @@ support-files =
 [test_bug_511615.html]
 [test_bug_787624.html]
 [test_datalist_with_caching.html]
 [test_form_autocomplete.html]
 [test_form_autocomplete_with_list.html]
 [test_form_submission.html]
 [test_form_submission_cap.html]
 [test_form_submission_cap2.html]
+[test_password_autocomplete.html]
 [test_popup_direction.html]
 [test_popup_enter_event.html]
copy from toolkit/components/satchel/test/test_bug_511615.html
copy to toolkit/components/satchel/test/test_password_autocomplete.html
--- a/toolkit/components/satchel/test/test_bug_511615.html
+++ b/toolkit/components/satchel/test/test_password_autocomplete.html
@@ -1,27 +1,32 @@
 <!DOCTYPE HTML>
 <html>
 <head>
-  <title>Test for Form History Autocomplete Untrusted Events: Bug 511615</title>
+  <title>Test for form history on type=password</title>
   <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
   <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
   <script type="text/javascript" src="satchel_common.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
 </head>
 <body>
-Test for Form History Autocomplete Untrusted Events: Bug 511615
+  Test for form history on type=password
+  (based on test_bug_511615.html)
 <p id="display"></p>
 
 <!-- we presumably can't hide the content for this test. -->
 <div id="content">
-  <!-- normal, basic form -->
+  <datalist id="datalist1">
+    <option>value10</option>
+    <option>value11</option>
+    <option>value12</option>
+  </datalist>
   <form id="form1" onsubmit="return false;">
-    <input  type="text" name="field1">
+    <input  type="password" name="field1" list="datalist1">
     <button type="submit">Submit</button>
   </form>
 </div>
 
 <pre id="test">
 <script class="testbody" type="text/javascript">
 /* import-globals-from satchel_common.js */
 
@@ -56,144 +61,53 @@ function expectPopupDoesNotOpen(triggerF
   let popupShown = waitForNextPopup();
   triggerFn();
   return Promise.race([
     popupShown.then(() => Promise.reject("Popup opened unexpectedly.")),
     new Promise(resolve => setTimeout(resolve, POPUP_RESPONSE_WAIT_TIME_MS)),
   ]);
 }
 
-/**
- * Checks that the selected index in the popup still matches the given value.
- */
-function checkSelectedIndexAfterResponseTime(expectedIndex) {
-  return new Promise(resolve => {
-    setTimeout(() => getPopupState(resolve), POPUP_RESPONSE_WAIT_TIME_MS);
-  }).then(popupState => {
-    is(popupState.open, true, "Popup should still be open.");
-    is(popupState.selectedIndex, expectedIndex, "Selected index should match.");
-  });
-}
-
-function doKeyUnprivileged(key) {
-  let keyName = "DOM_VK_" + key.toUpperCase();
-  let keycode, charcode, alwaysval;
-
-  if (key.length == 1) {
-      keycode = 0;
-      charcode = key.charCodeAt(0);
-      alwaysval = charcode;
-  } else {
-      keycode = KeyEvent[keyName];
-      if (!keycode)
-          throw "invalid keyname in test";
-      charcode = 0;
-      alwaysval = keycode;
-  }
-
-  let dnEvent = document.createEvent("KeyboardEvent");
-  let prEvent = document.createEvent("KeyboardEvent");
-  let upEvent = document.createEvent("KeyboardEvent");
-
-  /* eslint-disable no-multi-spaces */
-  dnEvent.initKeyEvent("keydown",  true, true, null, false, false, false, false, alwaysval, 0);
-  prEvent.initKeyEvent("keypress", true, true, null, false, false, false, false, keycode, charcode);
-  upEvent.initKeyEvent("keyup",    true, true, null, false, false, false, false, alwaysval, 0);
-  /* eslint-enable no-multi-spaces */
-
-  input.dispatchEvent(dnEvent);
-  input.dispatchEvent(prEvent);
-  input.dispatchEvent(upEvent);
-}
-
-function doClickWithMouseEventUnprivileged() {
-  let dnEvent = document.createEvent("MouseEvent");
-  let upEvent = document.createEvent("MouseEvent");
-  let ckEvent = document.createEvent("MouseEvent");
-
-  /* eslint-disable no-multi-spaces */
-  dnEvent.initMouseEvent("mousedown",  true, true, window, 1, 0, 0, 0, 0, false, false, false, false, 0, null);
-  upEvent.initMouseEvent("mouseup",    true, true, window, 1, 0, 0, 0, 0, false, false, false, false, 0, null);
-  ckEvent.initMouseEvent("mouseclick", true, true, window, 1, 0, 0, 0, 0, false, false, false, false, 0, null);
-  /* eslint-enable no-multi-spaces */
-
-  input.dispatchEvent(dnEvent);
-  input.dispatchEvent(upEvent);
-  input.dispatchEvent(ckEvent);
-}
-
 let input = $_(1, "field1");
 
 add_task(function* test_initialize() {
+  yield SpecialPowers.pushPrefEnv("set", [["signon.rememberSignons", false]]);
+
   yield new Promise(resolve => updateFormHistory([
     { op : "remove" },
     { op : "add", fieldname : "field1", value : "value1" },
     { op : "add", fieldname : "field1", value : "value2" },
     { op : "add", fieldname : "field1", value : "value3" },
     { op : "add", fieldname : "field1", value : "value4" },
     { op : "add", fieldname : "field1", value : "value5" },
     { op : "add", fieldname : "field1", value : "value6" },
     { op : "add", fieldname : "field1", value : "value7" },
     { op : "add", fieldname : "field1", value : "value8" },
     { op : "add", fieldname : "field1", value : "value9" },
   ], resolve));
 });
 
-add_task(function* test_untrusted_events_ignored() {
-  // The autocomplete popup should not open from untrusted events.
+add_task(function* test_no_form_history() {
+  // The autocomplete popup should not open under any circumstances on
+  // type=password with password manager disabled.
   for (let triggerFn of [
     () => input.focus(),
     () => input.click(),
-    () => doClickWithMouseEventUnprivileged(),
-    () => doKeyUnprivileged("down"),
-    () => doKeyUnprivileged("page_down"),
-    () => doKeyUnprivileged("return"),
-    () => doKeyUnprivileged("v"),
-    () => doKeyUnprivileged(" "),
-    () => doKeyUnprivileged("back_space"),
+    () => doKey("down"),
+    () => doKey("page_down"),
+    () => doKey("return"),
+    () => doKey("v"),
+    () => doKey(" "),
+    () => doKey("back_space"),
   ]) {
+    ok(true, "Testing: " + triggerFn.toString())
     // We must wait for the entire timeout for each individual test, because the
     // next event in the list might prevent the popup from opening.
     yield expectPopupDoesNotOpen(triggerFn);
   }
 
-  // A privileged key press will actually open the popup.
-  let popupShown = waitForNextPopup();
-  doKey("down");
-  yield popupShown;
-
-  // The selected autocomplete item should not change from untrusted events.
-  for (let triggerFn of [
-    () => doKeyUnprivileged("down"),
-    () => doKeyUnprivileged("page_down"),
-  ]) {
-    triggerFn();
-    yield checkSelectedIndexAfterResponseTime(-1);
-  }
-
-  // A privileged key press will actually change the selected index.
-  let indexChanged = new Promise(resolve => notifySelectedIndex(0, resolve));
-  doKey("down");
-  yield indexChanged;
-
-  // The selected autocomplete item should not change and it should not be
-  // possible to use it from untrusted events.
-  for (let triggerFn of [
-    () => doKeyUnprivileged("down"),
-    () => doKeyUnprivileged("page_down"),
-    () => doKeyUnprivileged("right"),
-    () => doKeyUnprivileged(" "),
-    () => doKeyUnprivileged("back_space"),
-    () => doKeyUnprivileged("back_space"),
-    () => doKeyUnprivileged("return"),
-  ]) {
-    triggerFn();
-    yield checkSelectedIndexAfterResponseTime(0);
-    is(input.value, "", "The selected item should not have been used.");
-  }
-
   // Close the popup.
   input.blur();
 });
 </script>
 </pre>
 </body>
 </html>