Backed out changeset cb874e871110 (bug 1334026) on request because need to wait for a different bug/set of patches
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Mon, 06 Feb 2017 11:01:11 +0100
changeset 480356 06eda6a9a51fa6a6d2d2e38b178d4bf009891a77
parent 480355 e5373fbb022e1a227116a989d22929040bfabc3c
child 480357 afe0d94be5473f11a871efad31bb638c17ca3e8f
push id44524
push usermartin.thomson@gmail.com
push dateWed, 08 Feb 2017 05:10:11 +0000
bugs1334026
milestone52.0
backs outcb874e871110d67e78f2c8bc37b618ebb9ba0e4c
Backed out changeset cb874e871110 (bug 1334026) on request because need to wait for a different bug/set of patches
toolkit/components/satchel/nsFormFillController.cpp
toolkit/components/satchel/test/test_password_autocomplete.html
--- a/toolkit/components/satchel/nsFormFillController.cpp
+++ b/toolkit/components/satchel/nsFormFillController.cpp
@@ -662,29 +662,20 @@ nsFormFillController::GetUserContextId(u
 ////////////////////////////////////////////////////////////////////////
 //// nsIAutoCompleteSearch
 
 NS_IMETHODIMP
 nsFormFillController::StartSearch(const nsAString &aSearchString, const nsAString &aSearchParam,
                                   nsIAutoCompleteResult *aPreviousResult, nsIAutoCompleteObserver *aListener)
 {
   nsresult rv;
-  nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(mFocusedInputNode);
 
   // If the login manager has indicated it's responsible for this field, let it
   // handle the autocomplete. Otherwise, handle with form history.
-  if (mFocusedInputNode && (mPwmgrInputs.Get(mFocusedInputNode) ||
-                            formControl->GetType() == NS_FORM_INPUT_PASSWORD)) {
-
-    // Handle the case where a password field is focused but
-    // MarkAsLoginManagerField wasn't called because password manager is disabled.
-    if (!mLoginManager) {
-      mLoginManager = do_GetService("@mozilla.org/login-manager;1");
-    }
-
+  if (mPwmgrInputs.Get(mFocusedInputNode)) {
     // XXX aPreviousResult shouldn't ever be a historyResult type, since we're not letting
     // satchel manage the field?
     mLastListener = aListener;
     rv = mLoginManager->AutoCompleteSearchAsync(aSearchString,
                                                 aPreviousResult,
                                                 mFocusedInput,
                                                 this);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -943,45 +934,39 @@ 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)
 {
   nsCOMPtr<nsIDOMHTMLInputElement> input = do_QueryInterface(
     aEvent->InternalDOMEvent()->GetTarget());
   MaybeStartControllingInput(input);
 
-  // Bail if we didn't start controlling the input.
-  if (!mFocusedInputNode) {
-    mContextMenuFiredBeforeFocus = false;
-    return NS_OK;
-  }
-
-  nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(mFocusedInputNode);
-  MOZ_ASSERT(formControl);
-
   // If this focus doesn't immediately follow a contextmenu event then show
   // the autocomplete popup
-  if (!mContextMenuFiredBeforeFocus &&
-      (mPwmgrInputs.Get(mFocusedInputNode)
-#ifndef ANDROID
-       || formControl->GetType() == NS_FORM_INPUT_PASSWORD
-#endif
-       )) {
+  if (!mContextMenuFiredBeforeFocus && mPwmgrInputs.Get(mFocusedInputNode)) {
     ShowPopup();
   }
 
   mContextMenuFiredBeforeFocus = false;
   return NS_OK;
 }
 
 nsresult
--- a/toolkit/components/satchel/test/test_password_autocomplete.html
+++ b/toolkit/components/satchel/test/test_password_autocomplete.html
@@ -63,45 +63,58 @@ 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)),
   ]);
 }
 
+let input = $_(1, "field1");
+
 add_task(function* test_initialize() {
+  info("remember: " + SpecialPowers.getBoolPref("signon.rememberSignons"));
   yield SpecialPowers.pushPrefEnv({"set": [["signon.rememberSignons", false]]});
+  info("remember: " + SpecialPowers.getBoolPref("signon.rememberSignons"));
 
   // Now that rememberSignons is false, create the password field
-  $_(1, "field1").type = "password";
+  input.type = "password";
 
   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_insecure_focusWarning() {
-  // The form is insecure so should show the warning even if password manager is disabled.
-  let input = $_(1, "field1");
-  let shownPromise = waitForNextPopup();
-  input.focus();
-  yield shownPromise;
+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(),
+    () => 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);
+  }
 
-  ok(getMenuEntries()[0].includes("Logins entered here could be compromised"),
-     "Check warning is first")
-
-  // Close the popup
+  // Close the popup.
   input.blur();
 });
 </script>
 </pre>
 </body>
 </html>