Bug 1329351 - Only autocomplete on password fields which were marked. r=mconley
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Thu, 19 Jan 2017 01:06:47 -0800
changeset 377393 2365700d798821589e32c8fbeceefced1ab3da7c
parent 377392 fabf199ceca6940003982d6b66e588460dad86aa
child 377394 52a8ae1e1a82e5f7166001b70d5ab5adb2f26336
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley
bugs1329351
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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>