Skip to content

Commit

Permalink
Dismiss mixed form warning on typing
Browse files Browse the repository at this point in the history
Currently a new warning is shown every time a character is typed, which
results in a flashing warning when typing. This CL makes it so we
consider typing as dismissing the warning and no longer show it again.

Bug: 1106542
Change-Id: Iac0aa33bd8e063f27b0aca7ff36cd5bdeef7400d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352215
Commit-Queue: Carlos IL <carlosil@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797856}
  • Loading branch information
carlosjoan91 authored and Commit Bot committed Aug 13, 2020
1 parent 8c1305f commit 675407d
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 7 deletions.
23 changes: 16 additions & 7 deletions components/autofill/core/browser/autofill_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,7 @@ void AutofillManager::OnQueryFormFieldAutofillImpl(
autoselect_first_suggestion);
return;

case SuppressReason::kInsecureForm:
case SuppressReason::kAutocompleteOff:
return;
}
Expand All @@ -964,8 +965,9 @@ void AutofillManager::OnQueryFormFieldAutofillImpl(
// If there are no Autofill suggestions, consider showing Autocomplete
// suggestions. We will not show Autocomplete suggestions for a field that
// specifies autocomplete=off (or an unrecognized type), a field for which we
// will show the credit card signin promo, or a field that we think is a
// credit card expiration, cvc or number.
// will show the credit card signin promo, a field that we think is a
// credit card expiration, cvc or number, or on forms displayed on secure
// (i.e. HTTPS) sites that submit insecurely (over HTTP).
if (suggestions.empty() && !ShouldShowCreditCardSigninPromo(form, field) &&
field.should_autocomplete &&
!(context.focused_field &&
Expand All @@ -975,7 +977,8 @@ void AutofillManager::OnQueryFormFieldAutofillImpl(
context.focused_field->Type().GetStorableType() ==
CREDIT_CARD_NUMBER ||
context.focused_field->Type().GetStorableType() ==
CREDIT_CARD_VERIFICATION_CODE))) {
CREDIT_CARD_VERIFICATION_CODE)) &&
context.suppress_reason != SuppressReason::kInsecureForm) {
// Suggestions come back asynchronously, so the Autocomplete manager will
// handle sending the results back to the renderer.
autocomplete_history_manager_->OnGetAutocompleteSuggestions(
Expand Down Expand Up @@ -2610,10 +2613,16 @@ void AutofillManager::GetAvailableSuggestions(
features::kAutofillPreventMixedFormsFilling) &&
client_->GetPrefs()->GetBoolean(::prefs::kMixedFormsWarningsEnabled)) {
suggestions->clear();
Suggestion warning_suggestion(
l10n_util::GetStringUTF16(IDS_AUTOFILL_WARNING_MIXED_FORM));
warning_suggestion.frontend_id = POPUP_ITEM_ID_MIXED_FORM_MESSAGE;
suggestions->emplace_back(warning_suggestion);
// If the user begins typing, we interpret that as dismissing the warning.
// No suggestions are allowed, but the warning is no longer shown.
if (field.DidUserType()) {
context->suppress_reason = SuppressReason::kInsecureForm;
} else {
Suggestion warning_suggestion(
l10n_util::GetStringUTF16(IDS_AUTOFILL_WARNING_MIXED_FORM));
warning_suggestion.frontend_id = POPUP_ITEM_ID_MIXED_FORM_MESSAGE;
suggestions->emplace_back(warning_suggestion);
}
return;
}

Expand Down
4 changes: 4 additions & 0 deletions components/autofill/core/browser/autofill_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,10 @@ class AutofillManager : public AutofillHandler,
// Address suggestions are not shown because the field is annotated with
// autocomplete=off and the directive is being observed by the browser.
kAutocompleteOff,
// Suggestions are not shown because this form is on a secure site, but
// submits insecurely. This is only used when the user has started typing,
// otherwise a warning is shown.
kInsecureForm,
};

// The context for the list of suggestions available for a given field to be
Expand Down
26 changes: 26 additions & 0 deletions components/autofill/core/browser/autofill_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8679,6 +8679,32 @@ TEST_F(AutofillManagerTestWithMixedForms,
EXPECT_FALSE(external_delegate_->on_suggestions_returned_seen());
}

// Test that we dismiss the mixed form warning if user starts typing.
TEST_F(AutofillManagerTestWithMixedForms, GetSuggestions_MixedFormUserTyped) {
// Set up our form data.
FormData form;
form.name = ASCIIToUTF16("MyForm");
form.url = GURL("https://myform.com/form.html");
form.action = GURL("http://myform.com/submit.html");
FormFieldData field;
test::CreateTestFormField("Name on Card", "nameoncard", "", "text", &field);
form.fields.push_back(field);

GetAutofillSuggestions(form, form.fields[0]);

// Test that we sent the right values to the external delegate.
CheckSuggestions(
kDefaultPageID,
Suggestion(l10n_util::GetStringUTF8(IDS_AUTOFILL_WARNING_MIXED_FORM), "",
"", POPUP_ITEM_ID_MIXED_FORM_MESSAGE));

// Pretend user started typing and make sure we no longer set suggestions.
form.fields[0].value = base::ASCIIToUTF16("Michael");
form.fields[0].properties_mask |= kUserTyped;
GetAutofillSuggestions(form, form.fields[0]);
external_delegate_->CheckNoSuggestions(kDefaultPageID);
}

// Desktop only tests.
#if !defined(OS_ANDROID) && !defined(OS_IOS)
class AutofillManagerTestForVirtualCardOption : public AutofillManagerTest {
Expand Down

0 comments on commit 675407d

Please sign in to comment.