Commit 42f315f4 authored by gcasto@chromium.org's avatar gcasto@chromium.org

[Password Generation] Wait longer to dismiss suggestion UI

UX feedback is that users generally look at the keyboard when starting to
type passwords and thus miss the prompt. The UI will now show until the user
types 5 characters instead of being dismissed after they type anything.

BUG=318977

Review URL: https://codereview.chromium.org/447873004

Cr-Commit-Position: refs/heads/master@{#288935}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288935 0039d316-1c4b-4281-b951-d872f2087c98
parent db4461b9
...@@ -172,7 +172,7 @@ void PasswordGenerationPopupControllerImpl::CalculateBounds() { ...@@ -172,7 +172,7 @@ void PasswordGenerationPopupControllerImpl::CalculateBounds() {
void PasswordGenerationPopupControllerImpl::Show(bool display_password) { void PasswordGenerationPopupControllerImpl::Show(bool display_password) {
display_password_ = display_password; display_password_ = display_password;
if (display_password_) if (display_password_ && current_password_.empty())
current_password_ = base::ASCIIToUTF16(generator_->Generate()); current_password_ = base::ASCIIToUTF16(generator_->Generate());
if (!view_) { if (!view_) {
......
...@@ -60,7 +60,7 @@ class PasswordGenerationPopupControllerImpl ...@@ -60,7 +60,7 @@ class PasswordGenerationPopupControllerImpl
// Create a PasswordGenerationPopupView if one doesn't already exist. // Create a PasswordGenerationPopupView if one doesn't already exist.
// If |display_password| is true, a generated password is shown that can be // If |display_password| is true, a generated password is shown that can be
// selected by the user. Otherwise just the text explaining generated // selected by the user. Otherwise just the text explaining generated
// passwords is shown. // passwords is shown. Idempotent.
void Show(bool display_password); void Show(bool display_password);
// Hides the popup and destroys |this|. // Hides the popup and destroys |this|.
......
...@@ -58,7 +58,6 @@ class PasswordGenerationAgentTest : public ChromeRenderViewTest { ...@@ -58,7 +58,6 @@ class PasswordGenerationAgentTest : public ChromeRenderViewTest {
WebElement element = WebElement element =
document.getElementById(WebString::fromUTF8(element_id)); document.getElementById(WebString::fromUTF8(element_id));
ASSERT_FALSE(element.isNull()); ASSERT_FALSE(element.isNull());
WebInputElement target_element = element.to<WebInputElement>();
ExecuteJavaScript( ExecuteJavaScript(
base::StringPrintf("document.getElementById('%s').focus();", base::StringPrintf("document.getElementById('%s').focus();",
element_id).c_str()); element_id).c_str());
...@@ -256,4 +255,79 @@ TEST_F(PasswordGenerationAgentTest, AccountCreationFormsDetectedTest) { ...@@ -256,4 +255,79 @@ TEST_F(PasswordGenerationAgentTest, AccountCreationFormsDetectedTest) {
ExpectPasswordGenerationAvailable("first_password", true); ExpectPasswordGenerationAvailable("first_password", true);
} }
TEST_F(PasswordGenerationAgentTest, MaximumOfferSize) {
LoadHTML(kAccountCreationFormHTML);
SetNotBlacklistedMessage(kAccountCreationFormHTML);
SetAccountCreationFormsDetectedMessage(kAccountCreationFormHTML);
ExpectPasswordGenerationAvailable("first_password", true);
WebDocument document = GetMainFrame()->document();
WebElement element =
document.getElementById(WebString::fromUTF8("first_password"));
ASSERT_FALSE(element.isNull());
WebInputElement first_password_element = element.to<WebInputElement>();
// Make a password just under maximum offer size.
first_password_element.setValue(
base::ASCIIToUTF16(
std::string(password_generation_->kMaximumOfferSize - 1, 'a')));
// Cast to WebAutofillClient where textFieldDidChange() is public.
static_cast<blink::WebAutofillClient*>(autofill_agent_)->textFieldDidChange(
first_password_element);
// textFieldDidChange posts a task, so we need to wait until it's been
// processed.
base::MessageLoop::current()->RunUntilIdle();
// There should now be a message to show the UI.
ASSERT_EQ(1u, password_generation_->messages().size());
EXPECT_EQ(AutofillHostMsg_ShowPasswordGenerationPopup::ID,
password_generation_->messages()[0]->type());
password_generation_->clear_messages();
// Simulate a user typing a password just over maximum offer size.
first_password_element.setValue(
base::ASCIIToUTF16(
std::string(password_generation_->kMaximumOfferSize + 1, 'a')));
// Cast to WebAutofillClient where textFieldDidChange() is public.
static_cast<blink::WebAutofillClient*>(autofill_agent_)->textFieldDidChange(
first_password_element);
// textFieldDidChange posts a task, so we need to wait until it's been
// processed.
base::MessageLoop::current()->RunUntilIdle();
// There should now be a message to hide the UI.
ASSERT_EQ(1u, password_generation_->messages().size());
EXPECT_EQ(AutofillHostMsg_HidePasswordGenerationPopup::ID,
password_generation_->messages()[0]->type());
password_generation_->clear_messages();
// Simulate the user deleting characters. The generation popup should be shown
// again.
first_password_element.setValue(
base::ASCIIToUTF16(
std::string(password_generation_->kMaximumOfferSize, 'a')));
// Cast to WebAutofillClient where textFieldDidChange() is public.
static_cast<blink::WebAutofillClient*>(autofill_agent_)->textFieldDidChange(
first_password_element);
// textFieldDidChange posts a task, so we need to wait until it's been
// processed.
base::MessageLoop::current()->RunUntilIdle();
// There should now be a message to show the UI.
ASSERT_EQ(1u, password_generation_->messages().size());
EXPECT_EQ(AutofillHostMsg_ShowPasswordGenerationPopup::ID,
password_generation_->messages()[0]->type());
password_generation_->clear_messages();
// Change focus. Bubble should be hidden, but that is handled by AutofilAgent,
// so no messages are sent.
ExecuteJavaScript("document.getElementById('username').focus();");
EXPECT_EQ(0u, password_generation_->messages().size());
password_generation_->clear_messages();
// Focusing the password field will bring up the generation UI again.
ExecuteJavaScript("document.getElementById('first_password').focus();");
EXPECT_EQ(1u, password_generation_->messages().size());
EXPECT_EQ(AutofillHostMsg_ShowPasswordGenerationPopup::ID,
password_generation_->messages()[0]->type());
password_generation_->clear_messages();
}
} // namespace autofill } // namespace autofill
...@@ -288,10 +288,12 @@ bool PasswordGenerationAgent::FocusedNodeHasChanged( ...@@ -288,10 +288,12 @@ bool PasswordGenerationAgent::FocusedNodeHasChanged(
return true; return true;
} }
// Only trigger if the password field is empty. // Assume that if the password field has less than kMaximumOfferSize
// characters then the user is not finished typing their password and display
// the password suggestion.
if (!element->isReadOnly() && if (!element->isReadOnly() &&
element->isEnabled() && element->isEnabled() &&
element->value().isEmpty()) { element->value().length() <= kMaximumOfferSize) {
ShowGenerationPopup(); ShowGenerationPopup();
return true; return true;
} }
...@@ -318,10 +320,7 @@ bool PasswordGenerationAgent::TextDidChangeInTextField( ...@@ -318,10 +320,7 @@ bool PasswordGenerationAgent::TextDidChangeInTextField(
// Offer generation again. // Offer generation again.
ShowGenerationPopup(); ShowGenerationPopup();
} else if (!password_is_generated_) { } else if (password_is_generated_) {
// User has rejected the feature and has started typing a password.
HidePopup();
} else {
password_edited_ = true; password_edited_ = true;
// Mirror edits to any confirmation password fields. // Mirror edits to any confirmation password fields.
for (std::vector<blink::WebInputElement>::iterator it = for (std::vector<blink::WebInputElement>::iterator it =
...@@ -329,6 +328,14 @@ bool PasswordGenerationAgent::TextDidChangeInTextField( ...@@ -329,6 +328,14 @@ bool PasswordGenerationAgent::TextDidChangeInTextField(
it != password_elements_.end(); ++it) { it != password_elements_.end(); ++it) {
it->setValue(element.value()); it->setValue(element.value());
} }
} else if (element.value().length() > kMaximumOfferSize) {
// User has rejected the feature and has started typing a password.
HidePopup();
} else {
// Password isn't generated and there are fewer than kMaximumOfferSize
// characters typed, so keep offering the password. Note this function
// will just keep the previous popup if one is already showing.
ShowGenerationPopup();
} }
return true; return true;
......
...@@ -39,6 +39,9 @@ class PasswordGenerationAgent : public content::RenderViewObserver { ...@@ -39,6 +39,9 @@ class PasswordGenerationAgent : public content::RenderViewObserver {
// Returns true if the newly focused node caused the generation UI to show. // Returns true if the newly focused node caused the generation UI to show.
bool FocusedNodeHasChanged(const blink::WebNode& node); bool FocusedNodeHasChanged(const blink::WebNode& node);
// The length that a password can be before the UI is hidden.
static const size_t kMaximumOfferSize = 5;
protected: protected:
// Returns true if this document is one that we should consider analyzing. // Returns true if this document is one that we should consider analyzing.
// Virtual so that it can be overriden during testing. // Virtual so that it can be overriden during testing.
......
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