Commit b149d8bf authored by Maria Kazinova's avatar Maria Kazinova Committed by Commit Bot

Fill On Account Select improvements for empty username credentials.

Now Fill On Account Select on password field displays all credentials,
including the ones with empty usernames.
If the credential with non-empty username is chosen first, and the
empty username credential is chosen on password field afterwards, the
username is not refilled.

Bug: 926844
Change-Id: Iacd62b9c17680fbebbee8199be7c4e2b0b41c83c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1982550
Commit-Queue: Maria Kazinova <kazinova@google.com>
Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727568}
parent 822d2210
...@@ -88,6 +88,8 @@ const char kBobPassword[] = "secret"; ...@@ -88,6 +88,8 @@ const char kBobPassword[] = "secret";
const char kCarolUsername[] = "Carol"; const char kCarolUsername[] = "Carol";
const char kCarolPassword[] = "test"; const char kCarolPassword[] = "test";
const char kCarolAlternateUsername[] = "RealCarolUsername"; const char kCarolAlternateUsername[] = "RealCarolUsername";
const char kEmptyUsername[] = "";
const char kEmptyUsernamePassword[] = "empty";
const char kFormHTML[] = const char kFormHTML[] =
"<FORM id='LoginTestForm' action='http://www.bidule.com'>" "<FORM id='LoginTestForm' action='http://www.bidule.com'>"
...@@ -348,6 +350,8 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest { ...@@ -348,6 +350,8 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
username3_ = ASCIIToUTF16(kCarolUsername); username3_ = ASCIIToUTF16(kCarolUsername);
password3_ = ASCIIToUTF16(kCarolPassword); password3_ = ASCIIToUTF16(kCarolPassword);
alternate_username3_ = ASCIIToUTF16(kCarolAlternateUsername); alternate_username3_ = ASCIIToUTF16(kCarolAlternateUsername);
username4_ = ASCIIToUTF16(kEmptyUsername);
password4_ = ASCIIToUTF16(kEmptyUsernamePassword);
FormFieldData username_field; FormFieldData username_field;
username_field.name = ASCIIToUTF16(kUsernameName); username_field.name = ASCIIToUTF16(kUsernameName);
...@@ -366,6 +370,9 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest { ...@@ -366,6 +370,9 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
PasswordAndMetadata password3; PasswordAndMetadata password3;
password3.password = password3_; password3.password = password3_;
fill_data_.additional_logins[username3_] = password3; fill_data_.additional_logins[username3_] = password3;
PasswordAndMetadata password4;
password3.password = password4_;
fill_data_.additional_logins[username4_] = password4;
// We need to set the origin so it matches the frame URL and the action so // We need to set the origin so it matches the frame URL and the action so
// it matches the form action, otherwise we won't autocomplete. // it matches the form action, otherwise we won't autocomplete.
...@@ -615,7 +622,8 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest { ...@@ -615,7 +622,8 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
// suggestions, or only those starting with |username|. // suggestions, or only those starting with |username|.
void CheckSuggestions(const std::string& username, bool show_all) { void CheckSuggestions(const std::string& username, bool show_all) {
auto show_all_matches = [show_all](int options) { auto show_all_matches = [show_all](int options) {
return show_all == !!(options & autofill::SHOW_ALL); return (show_all == ((options & autofill::SHOW_ALL) != 0)) ||
(show_all == ((options & autofill::IS_PASSWORD_FIELD) != 0));
}; };
EXPECT_CALL(fake_driver_, EXPECT_CALL(fake_driver_,
...@@ -775,10 +783,12 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest { ...@@ -775,10 +783,12 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
base::string16 username1_; base::string16 username1_;
base::string16 username2_; base::string16 username2_;
base::string16 username3_; base::string16 username3_;
base::string16 username4_;
base::string16 password1_; base::string16 password1_;
base::string16 password2_; base::string16 password2_;
base::string16 password3_; base::string16 password3_;
base::string16 alternate_username3_; base::string16 alternate_username3_;
base::string16 password4_;
PasswordFormFillData fill_data_; PasswordFormFillData fill_data_;
WebInputElement username_element_; WebInputElement username_element_;
...@@ -2364,7 +2374,7 @@ TEST_F(PasswordAutofillAgentTest, ShowPopupOnEmptyPasswordField) { ...@@ -2364,7 +2374,7 @@ TEST_F(PasswordAutofillAgentTest, ShowPopupOnEmptyPasswordField) {
SimulateSuggestionChoiceOfUsernameAndPassword( SimulateSuggestionChoiceOfUsernameAndPassword(
password_element_, base::string16(), ASCIIToUTF16(kAlicePassword)); password_element_, base::string16(), ASCIIToUTF16(kAlicePassword));
CheckSuggestions(std::string(), false); CheckSuggestions(std::string(), true);
EXPECT_EQ(ASCIIToUTF16(kAlicePassword), password_element_.Value().Utf16()); EXPECT_EQ(ASCIIToUTF16(kAlicePassword), password_element_.Value().Utf16());
EXPECT_TRUE(password_element_.IsAutofilled()); EXPECT_TRUE(password_element_.IsAutofilled());
} }
...@@ -2389,7 +2399,7 @@ TEST_F(PasswordAutofillAgentTest, ShowPopupOnAutofilledPasswordField) { ...@@ -2389,7 +2399,7 @@ TEST_F(PasswordAutofillAgentTest, ShowPopupOnAutofilledPasswordField) {
SimulateSuggestionChoiceOfUsernameAndPassword( SimulateSuggestionChoiceOfUsernameAndPassword(
password_element_, base::string16(), ASCIIToUTF16(kAlicePassword)); password_element_, base::string16(), ASCIIToUTF16(kAlicePassword));
CheckSuggestions(std::string(), false); CheckSuggestions(std::string(), true);
EXPECT_EQ(ASCIIToUTF16(kAlicePassword), password_element_.Value().Utf16()); EXPECT_EQ(ASCIIToUTF16(kAlicePassword), password_element_.Value().Utf16());
EXPECT_TRUE(password_element_.IsAutofilled()); EXPECT_TRUE(password_element_.IsAutofilled());
} }
...@@ -2647,7 +2657,7 @@ TEST_F(PasswordAutofillAgentTest, ...@@ -2647,7 +2657,7 @@ TEST_F(PasswordAutofillAgentTest,
// Simulate a user clicking on the password element. This should produce a // Simulate a user clicking on the password element. This should produce a
// message. // message.
autofill_agent_->FormControlElementClicked(password_element_, true); autofill_agent_->FormControlElementClicked(password_element_, true);
CheckSuggestions("", false); CheckSuggestions("", true);
} }
// Tests that only the password field is autocompleted when the browser sends // Tests that only the password field is autocompleted when the browser sends
...@@ -3152,9 +3162,9 @@ TEST_F(PasswordAutofillAgentTest, ShowSuggestionForNonUsernameFieldForms) { ...@@ -3152,9 +3162,9 @@ TEST_F(PasswordAutofillAgentTest, ShowSuggestionForNonUsernameFieldForms) {
SimulateOnFillPasswordForm(fill_data_); SimulateOnFillPasswordForm(fill_data_);
SimulateElementClick("password1"); SimulateElementClick("password1");
CheckSuggestions(std::string(), false); CheckSuggestions(std::string(), true);
SimulateElementClick("password2"); SimulateElementClick("password2");
CheckSuggestions(std::string(), false); CheckSuggestions(std::string(), true);
} }
// Tests that password manager sees both autofill assisted and user entered // Tests that password manager sees both autofill assisted and user entered
...@@ -3255,7 +3265,7 @@ TEST_F(PasswordAutofillAgentTest, SuggestPasswordFieldSignInForm) { ...@@ -3255,7 +3265,7 @@ TEST_F(PasswordAutofillAgentTest, SuggestPasswordFieldSignInForm) {
// Simulate a user clicking on the password element. This should produce a // Simulate a user clicking on the password element. This should produce a
// dropdown with suggestion of all available usernames. // dropdown with suggestion of all available usernames.
autofill_agent_->FormControlElementClicked(password_element_, false); autofill_agent_->FormControlElementClicked(password_element_, false);
CheckSuggestions("", false); CheckSuggestions("", true);
} }
// Tests that a suggestion dropdown is shown on each password field. But when a // Tests that a suggestion dropdown is shown on each password field. But when a
...@@ -3278,13 +3288,13 @@ TEST_F(PasswordAutofillAgentTest, SuggestMultiplePasswordFields) { ...@@ -3278,13 +3288,13 @@ TEST_F(PasswordAutofillAgentTest, SuggestMultiplePasswordFields) {
// Simulate a user clicking on the password elements. This should produce // Simulate a user clicking on the password elements. This should produce
// dropdowns with suggestion of all available usernames. // dropdowns with suggestion of all available usernames.
SimulateElementClick("password"); SimulateElementClick("password");
CheckSuggestions("", false); CheckSuggestions("", true);
SimulateElementClick("newpassword"); SimulateElementClick("newpassword");
CheckSuggestions("", false); CheckSuggestions("", true);
SimulateElementClick("confirmpassword"); SimulateElementClick("confirmpassword");
CheckSuggestions("", false); CheckSuggestions("", true);
// The user chooses to autofill the current password field. // The user chooses to autofill the current password field.
EXPECT_TRUE(password_autofill_agent_->FillSuggestion( EXPECT_TRUE(password_autofill_agent_->FillSuggestion(
...@@ -3301,7 +3311,7 @@ TEST_F(PasswordAutofillAgentTest, SuggestMultiplePasswordFields) { ...@@ -3301,7 +3311,7 @@ TEST_F(PasswordAutofillAgentTest, SuggestMultiplePasswordFields) {
// But when the user clicks on the autofilled password field again it should // But when the user clicks on the autofilled password field again it should
// still produce a suggestion dropdown. // still produce a suggestion dropdown.
SimulateElementClick("password"); SimulateElementClick("password");
CheckSuggestions("", false); CheckSuggestions("", true);
} }
TEST_F(PasswordAutofillAgentTest, ShowAutofillSignaturesFlag) { TEST_F(PasswordAutofillAgentTest, ShowAutofillSignaturesFlag) {
...@@ -3345,7 +3355,7 @@ TEST_F(PasswordAutofillAgentTest, SuggestWhenJavaScriptUpdatesFieldNames) { ...@@ -3345,7 +3355,7 @@ TEST_F(PasswordAutofillAgentTest, SuggestWhenJavaScriptUpdatesFieldNames) {
// Simulate a user clicking on the password element. This should produce a // Simulate a user clicking on the password element. This should produce a
// dropdown with suggestion of all available usernames. // dropdown with suggestion of all available usernames.
autofill_agent_->FormControlElementClicked(password_element_, false); autofill_agent_->FormControlElementClicked(password_element_, false);
CheckSuggestions("", false); CheckSuggestions("", true);
} }
// Checks that a same-document navigation form submission could have an empty // Checks that a same-document navigation form submission could have an empty
...@@ -3577,7 +3587,7 @@ TEST_F(PasswordAutofillAgentTest, SuggestLatestCredentials) { ...@@ -3577,7 +3587,7 @@ TEST_F(PasswordAutofillAgentTest, SuggestLatestCredentials) {
password_autofill_agent_->FillPasswordForm(fill_data_); password_autofill_agent_->FillPasswordForm(fill_data_);
SimulateElementClick(kPasswordName); SimulateElementClick(kPasswordName);
// Empty value because nothing was typed into the field. // Empty value because nothing was typed into the field.
CheckSuggestions("", false); CheckSuggestions("", true);
} }
// Tests that PSL matched password is not autofilled even when there is // Tests that PSL matched password is not autofilled even when there is
...@@ -3823,6 +3833,26 @@ TEST_F(PasswordAutofillAgentTest, SingleUsernameClearPreview) { ...@@ -3823,6 +3833,26 @@ TEST_F(PasswordAutofillAgentTest, SingleUsernameClearPreview) {
CheckUsernameSelection(3, 3); CheckUsernameSelection(3, 3);
} }
// Fill on account select for credentials with empty usernames:
// Do not refill usernames if non-empty username is already selected.
TEST_F(PasswordAutofillAgentTest, NoUsernameCredential) {
SimulateOnFillPasswordForm(fill_data_);
ClearUsernameAndPasswordFields();
EXPECT_CALL(fake_driver_, ShowPasswordSuggestions);
SimulateSuggestionChoiceOfUsernameAndPassword(password_element_,
ASCIIToUTF16(kAliceUsername),
ASCIIToUTF16(kAlicePassword));
CheckTextFieldsDOMState(kAliceUsername, true, kAlicePassword, true);
base::RunLoop().RunUntilIdle();
EXPECT_CALL(fake_driver_, ShowPasswordSuggestions);
SimulateSuggestionChoiceOfUsernameAndPassword(
password_element_, ASCIIToUTF16(kEmptyUsername),
ASCIIToUTF16(kEmptyUsernamePassword));
CheckTextFieldsDOMState(kAliceUsername, true, kEmptyUsernamePassword, true);
}
// Tests that any fields that have user input are not refilled on the next // Tests that any fields that have user input are not refilled on the next
// call of FillPasswordForm. // call of FillPasswordForm.
TEST_F(PasswordAutofillAgentTest, NoRefillOfUserInput) { TEST_F(PasswordAutofillAgentTest, NoRefillOfUserInput) {
......
...@@ -142,16 +142,17 @@ void LogHTMLForm(SavePasswordProgressLogger* logger, ...@@ -142,16 +142,17 @@ void LogHTMLForm(SavePasswordProgressLogger* logger,
// |current_username| as a prefix. // |current_username| as a prefix.
bool CanShowSuggestion(const PasswordFormFillData& fill_data, bool CanShowSuggestion(const PasswordFormFillData& fill_data,
const base::string16& current_username, const base::string16& current_username,
bool show_all) { bool show_all,
bool is_password_field) {
base::string16 current_username_lower = base::i18n::ToLower(current_username); base::string16 current_username_lower = base::i18n::ToLower(current_username);
if (show_all || if (show_all || is_password_field ||
base::StartsWith(base::i18n::ToLower(fill_data.username_field.value), base::StartsWith(base::i18n::ToLower(fill_data.username_field.value),
current_username_lower, base::CompareCase::SENSITIVE)) { current_username_lower, base::CompareCase::SENSITIVE)) {
return true; return true;
} }
for (const auto& login : fill_data.additional_logins) { for (const auto& login : fill_data.additional_logins) {
if (show_all || if (show_all || is_password_field ||
base::StartsWith(base::i18n::ToLower(login.first), base::StartsWith(base::i18n::ToLower(login.first),
current_username_lower, current_username_lower,
base::CompareCase::SENSITIVE)) { base::CompareCase::SENSITIVE)) {
...@@ -602,6 +603,7 @@ bool PasswordAutofillAgent::FillSuggestion( ...@@ -602,6 +603,7 @@ bool PasswordAutofillAgent::FillSuggestion(
if (IsUsernameAmendable(username_element, if (IsUsernameAmendable(username_element,
element->IsPasswordFieldForAutofill()) && element->IsPasswordFieldForAutofill()) &&
!(username.empty() && element->IsPasswordFieldForAutofill()) &&
username_element.Value().Utf16() != username) { username_element.Value().Utf16() != username) {
FillField(&username_element, username); FillField(&username_element, username);
} }
...@@ -880,12 +882,7 @@ bool PasswordAutofillAgent::ShowSuggestions(const WebInputElement& element, ...@@ -880,12 +882,7 @@ bool PasswordAutofillAgent::ShowSuggestions(const WebInputElement& element,
if (touch_to_fill_state_ == TouchToFillState::kIsShowing) if (touch_to_fill_state_ == TouchToFillState::kIsShowing)
return true; return true;
// Chrome should never show more than one account for a password element since return ShowSuggestionPopup(*password_info, element, show_all,
// this implies that the username element cannot be modified. Thus even if
// |show_all| is true, check if the element in question is a password element
// for the call to ShowSuggestionPopup.
return ShowSuggestionPopup(*password_info, element,
show_all && !element.IsPasswordFieldForAutofill(),
element.IsPasswordFieldForAutofill()); element.IsPasswordFieldForAutofill());
} }
...@@ -1443,11 +1440,15 @@ bool PasswordAutofillAgent::ShowSuggestionPopup( ...@@ -1443,11 +1440,15 @@ bool PasswordAutofillAgent::ShowSuggestionPopup(
? base::string16() ? base::string16()
: user_input.Value().Utf16()); : user_input.Value().Utf16());
username_query_prefix_ = username_string;
if (!CanShowSuggestion(password_info.fill_data, username_string, show_all,
show_on_password_field)) {
return false;
}
GetPasswordManagerDriver()->ShowPasswordSuggestions( GetPasswordManagerDriver()->ShowPasswordSuggestions(
field.text_direction, username_string, options, field.text_direction, username_string, options,
render_frame()->ElementBoundsInWindow(user_input)); render_frame()->ElementBoundsInWindow(user_input));
username_query_prefix_ = username_string; return true;
return CanShowSuggestion(password_info.fill_data, username_string, show_all);
} }
void PasswordAutofillAgent::CleanupOnDocumentShutdown() { void PasswordAutofillAgent::CleanupOnDocumentShutdown() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment