Commit bb999330 authored by Thomas Tellier's avatar Thomas Tellier Committed by Commit Bot

Reland "Change hiding timer logic for password textfield on login/lock screen"

This reverts commit 59d5a5a3.

Reason for revert: Reland the CL by fixing the bug

Original change's description:
> Revert "Change hiding timer logic for password textfield on login/lock screen"
> 
> This reverts commit 4af05312.
> 
> Reason for revert: Breaking ash_unittests and browser_tests
> on chrome os builders, see
> https://bugs.chromium.org/p/chromium/issues/detail?id=1112924
> 
> Original change's description:
> > Change hiding timer logic for password textfield on login/lock screen
> > 
> > 5-second instead of 3-second timer for password/PIN going back to masked format.
> > 5-second timer will reset upon character being entered to keep the password/PIN visible during revision.
> > 
> > Bug: 1101326
> > Change-Id: I962139aef26bd9fb68e5110d5169d0f486a5bce4
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2314176
> > Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> > Commit-Queue: Thomas Tellier <tellier@google.com>
> > Cr-Commit-Position: refs/heads/master@{#794602}
> 
> TBR=rsorokin@chromium.org,tellier@google.com
> 
> Change-Id: I2cb94561e291ccadf623af2bd4529cd4c9bd23de
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1101326
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2337388
> Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
> Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#794721}

TBR=rsorokin@chromium.org,rbpotter@chromium.org,tellier@google.com

# Not skipping CQ checks because this is a reland.

Bug: 1101326
Change-Id: Ib1aca0d5dfc9f8912b39c7d27de4053b80d0126e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2337474Reviewed-by: default avatarRoman Sorokin [CET] <rsorokin@chromium.org>
Reviewed-by: default avatarThomas Tellier <tellier@google.com>
Commit-Queue: Thomas Tellier <tellier@google.com>
Cr-Commit-Position: refs/heads/master@{#794965}
parent 82bcf366
...@@ -108,7 +108,7 @@ constexpr base::TimeDelta kClearPasswordAfterDelay = ...@@ -108,7 +108,7 @@ constexpr base::TimeDelta kClearPasswordAfterDelay =
// Hides the password after a short delay for security reasons. // Hides the password after a short delay for security reasons.
constexpr base::TimeDelta kHidePasswordAfterDelay = constexpr base::TimeDelta kHidePasswordAfterDelay =
base::TimeDelta::FromSeconds(3); base::TimeDelta::FromSeconds(5);
constexpr const char kLoginPasswordViewName[] = "LoginPasswordView"; constexpr const char kLoginPasswordViewName[] = "LoginPasswordView";
...@@ -742,11 +742,12 @@ void LoginPasswordView::ContentsChanged(views::Textfield* sender, ...@@ -742,11 +742,12 @@ void LoginPasswordView::ContentsChanged(views::Textfield* sender,
if (!is_display_password_feature_enabled_) if (!is_display_password_feature_enabled_)
return; return;
// Only reset the timer if the display password feature is enabled. // If the password is currently revealed.
if (textfield_->GetTextInputType() == ui::TEXT_INPUT_TYPE_NULL)
hide_password_timer_->Reset();
// The feature could be enabled on the device but disabled for this user by policy.
if (display_password_button_->GetVisible()) if (display_password_button_->GetVisible())
clear_password_timer_->Reset(); clear_password_timer_->Reset();
// For UX purposes, hide back the password when the user is typing.
HidePassword(false /*chromevox_exception*/);
display_password_button_->SetEnabled(!new_contents.empty()); display_password_button_->SetEnabled(!new_contents.empty());
} }
......
...@@ -326,12 +326,15 @@ TEST_F(LoginPasswordViewTestFeatureEnabled, PasswordAutoClearsAndHides) { ...@@ -326,12 +326,15 @@ TEST_F(LoginPasswordViewTestFeatureEnabled, PasswordAutoClearsAndHides) {
ui::TEXT_INPUT_TYPE_PASSWORD); ui::TEXT_INPUT_TYPE_PASSWORD);
} }
// Verifies that the password textfield hides back when the content changes. // Verifies that the password textfield remains in the same visibility state
TEST_F(LoginPasswordViewTestFeatureEnabled, PasswordHidesAfterTyping) { // when the content changes.
TEST_F(LoginPasswordViewTestFeatureEnabled,
ContentChangesDoNotImpactPasswordVisibility) {
LoginPasswordView::TestApi test_api(view_); LoginPasswordView::TestApi test_api(view_);
ui::test::EventGenerator* generator = GetEventGenerator(); ui::test::EventGenerator* generator = GetEventGenerator();
// Show the password. // Type to enable the display password button and click on it to display the
// password.
EXPECT_EQ(test_api.textfield()->GetTextInputType(), EXPECT_EQ(test_api.textfield()->GetTextInputType(),
ui::TEXT_INPUT_TYPE_PASSWORD); ui::TEXT_INPUT_TYPE_PASSWORD);
generator->PressKey(ui::KeyboardCode::VKEY_A, ui::EF_NONE); generator->PressKey(ui::KeyboardCode::VKEY_A, ui::EF_NONE);
...@@ -340,18 +343,22 @@ TEST_F(LoginPasswordViewTestFeatureEnabled, PasswordHidesAfterTyping) { ...@@ -340,18 +343,22 @@ TEST_F(LoginPasswordViewTestFeatureEnabled, PasswordHidesAfterTyping) {
generator->ClickLeftButton(); generator->ClickLeftButton();
EXPECT_EQ(test_api.textfield()->GetTextInputType(), ui::TEXT_INPUT_TYPE_NULL); EXPECT_EQ(test_api.textfield()->GetTextInputType(), ui::TEXT_INPUT_TYPE_NULL);
// Type and check if the password textfield hides back. // Type manually and programmatically, and check if the password textfield
// remains visible.
generator->PressKey(ui::KeyboardCode::VKEY_A, ui::EF_NONE); generator->PressKey(ui::KeyboardCode::VKEY_A, ui::EF_NONE);
EXPECT_EQ(test_api.textfield()->GetTextInputType(), EXPECT_EQ(test_api.textfield()->GetTextInputType(), ui::TEXT_INPUT_TYPE_NULL);
ui::TEXT_INPUT_TYPE_PASSWORD); test_api.textfield()->InsertText(base::ASCIIToUTF16("test"));
EXPECT_EQ(test_api.textfield()->GetTextInputType(), ui::TEXT_INPUT_TYPE_NULL);
// Click again to show the password. // Click again on the display password button to hide the password.
generator->MoveMouseTo( generator->MoveMouseTo(
test_api.display_password_button()->GetBoundsInScreen().CenterPoint()); test_api.display_password_button()->GetBoundsInScreen().CenterPoint());
generator->ClickLeftButton(); generator->ClickLeftButton();
EXPECT_EQ(test_api.textfield()->GetTextInputType(), ui::TEXT_INPUT_TYPE_NULL); EXPECT_EQ(test_api.textfield()->GetTextInputType(),
ui::TEXT_INPUT_TYPE_PASSWORD);
// Modifies the content programmatically and check it is still triggered. // Type manually and programmatically, and check if the password textfield
// remains invisible.
test_api.textfield()->InsertText(base::ASCIIToUTF16("test")); test_api.textfield()->InsertText(base::ASCIIToUTF16("test"));
EXPECT_EQ(test_api.textfield()->GetTextInputType(), EXPECT_EQ(test_api.textfield()->GetTextInputType(),
ui::TEXT_INPUT_TYPE_PASSWORD); ui::TEXT_INPUT_TYPE_PASSWORD);
......
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