Bug 1334026 - Show the the insecure field warning on insecure password fields even if they're not marked. r=mconley
MozReview-Commit-ID: LxvPzD3SOnc
--- a/toolkit/components/satchel/nsFormFillController.cpp
+++ b/toolkit/components/satchel/nsFormFillController.cpp
@@ -662,20 +662,29 @@ 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 (mPwmgrInputs.Get(mFocusedInputNode)) {
+ 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");
+ }
+
// 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);
@@ -934,39 +943,45 @@ 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)) {
+ if (!mContextMenuFiredBeforeFocus &&
+ (mPwmgrInputs.Get(mFocusedInputNode)
+#ifndef ANDROID
+ || formControl->GetType() == NS_FORM_INPUT_PASSWORD
+#endif
+ )) {
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,58 +63,45 @@ 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
- input.type = "password";
+ $_(1, "field1").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_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);
- }
+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;
- // Close the popup.
+ ok(getMenuEntries()[0].includes("Logins entered here could be compromised"),
+ "Check warning is first")
+
+ // Close the popup
input.blur();
});
</script>
</pre>
</body>
</html>