Bug 1334026 - Show the the insecure field warning on insecure password fields even if they're not marked. r=mconley
MozReview-Commit-ID: JwEYWQmexj
--- a/toolkit/components/satchel/nsFormFillController.cpp
+++ b/toolkit/components/satchel/nsFormFillController.cpp
@@ -689,20 +689,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);
@@ -973,39 +982,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/mochitest.ini
+++ b/toolkit/components/satchel/test/mochitest.ini
@@ -10,10 +10,11 @@ support-files =
[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]
+scheme = https
[test_popup_direction.html]
[test_popup_enter_event.html]
--- a/toolkit/components/satchel/test/test_password_autocomplete.html
+++ b/toolkit/components/satchel/test/test_password_autocomplete.html
@@ -16,17 +16,24 @@
<!-- we presumably can't hide the content for this test. -->
<div id="content">
<datalist id="datalist1">
<option>value10</option>
<option>value11</option>
<option>value12</option>
</datalist>
<form id="form1" onsubmit="return false;">
- <input type="password" name="field1" list="datalist1">
+ <!-- Don't set the type to password until rememberSignons is false since we
+ want to test when rememberSignons is false. -->
+ <input type="to-be-password" name="field1" list="datalist1">
+ <button type="submit">Submit</button>
+ </form>
+ <!-- Same as form1 but with an insecure HTTP action -->
+ <form id="form2" onsubmit="return false;" action="http://mochi.test/">
+ <input type="to-be-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 */
@@ -61,36 +68,42 @@ 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() {
+ yield SpecialPowers.pushPrefEnv({set: [["signon.rememberSignons", false]]});
-add_task(function* test_initialize() {
- yield SpecialPowers.pushPrefEnv("set", [["signon.rememberSignons", false]]);
+ is(window.location.protocol, "https:", "This test must run on HTTPS");
+
+ // Now that rememberSignons is false, create the password fields.
+ $_(1, "field1").type = "password";
+ $_(2, "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() {
+add_task(function* test_secure_noFormHistoryOrWarning() {
+ let input = $_(1, "field1");
+
// 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"),
@@ -102,12 +115,26 @@ add_task(function* test_no_form_history(
// 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);
}
// Close the popup.
input.blur();
});
+
+add_task(function* test_insecure_focusWarning() {
+ // Form 2 has an insecure action so should show the warning even if password manager is disabled.
+ let input = $_(2, "field1");
+ let shownPromise = waitForNextPopup();
+ input.focus();
+ yield shownPromise;
+
+ ok(getMenuEntries()[0].includes("Logins entered here could be compromised"),
+ "Check warning is first")
+
+ // Close the popup
+ input.blur();
+});
</script>
</pre>
</body>
</html>