Commit c8e4b08b authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

[PwdCheckAndroid] Toggle masking with Eye Icon

This CL implements the functionality of the eye icon. Clicking it
either masks or unmasks the password.

Screenshot in the bug.

Bug: 1114720, 1092444
Change-Id: I1005e61961171caae5b0d0e1ea3c79ef6c3273ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362850
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799193}
parent d46788e5
......@@ -38,6 +38,7 @@ android_library("public_java") {
":public_ui_java",
"internal:public_ui_factory_java",
"//base:base_java",
"//chrome/browser/password_manager/android:java",
"//components/browser_ui/settings/android:java",
"//components/browser_ui/widget/android:java",
"//third_party/android_deps:androidx_annotation_annotation_java",
......@@ -113,6 +114,7 @@ android_library("test_java") {
"//chrome/browser/flags:java",
"//chrome/browser/password_check/android:password_check_java_enums",
"//chrome/browser/password_check/android/internal:public_factory_java",
"//chrome/browser/password_manager/android:java",
"//chrome/browser/settings:test_support_java",
"//chrome/test/android:chrome_java_test_support",
"//components/browser_ui/widget/android:java",
......
......@@ -87,10 +87,21 @@
android:background="?attr/selectableItemBackground"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:contentDescription="@string/password_entry_viewer_view_stored_password"
app:srcCompat="@drawable/ic_visibility_black"
app:tint="@color/default_icon_color_tint_list"
style="?android:attr/buttonStyleSmall"/>
<org.chromium.ui.widget.ChromeImageButton
android:id="@+id/password_entry_editor_mask_password"
android:background="?attr/selectableItemBackground"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:contentDescription="@string/password_entry_viewer_hide_stored_password"
app:srcCompat="@drawable/ic_visibility_off_black"
app:tint="@color/default_icon_color_tint_list"
style="?android:attr/buttonStyleSmall"/>
</LinearLayout>
</LinearLayout>
\ No newline at end of file
......@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.password_check;
import android.os.Bundle;
import android.text.Editable;
import android.text.InputType;
import android.text.TextUtils;
import android.text.TextWatcher;
import android.view.LayoutInflater;
......@@ -14,7 +15,9 @@ import android.view.MenuInflater;
import android.view.MenuItem;
import android.view.View;
import android.view.ViewGroup;
import android.view.WindowManager;
import android.widget.EditText;
import android.widget.ImageButton;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
......@@ -24,6 +27,8 @@ import androidx.preference.PreferenceFragmentCompat;
import com.google.android.material.textfield.TextInputLayout;
import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.password_manager.settings.ReauthenticationManager;
import org.chromium.chrome.browser.password_manager.settings.ReauthenticationManager.ReauthScope;
/**
* This class is responsible for rendering the edit fragment where users can provide a new password
......@@ -34,14 +39,18 @@ public class PasswordCheckEditFragmentView extends PreferenceFragmentCompat {
public static final String EXTRA_COMPROMISED_CREDENTIAL = "extra_compromised_credential";
@VisibleForTesting
static final String EXTRA_NEW_PASSWORD = "extra_new_password";
static final String EXTRA_PASSWORD_MASKED = "extra_password_masked";
private Supplier<PasswordCheck> mPasswordCheckFactory;
private String mNewPassword;
private CompromisedCredential mCredential;
private boolean mPasswordVisible;
private EditText mPasswordText;
private MenuItem mSaveButton;
private TextInputLayout mPasswordLabel;
private ImageButton mViewPasswordButton;
private ImageButton mMaskPasswordButton;
/**
* Initializes the password check factory that allows to retrieve a {@link PasswordCheck}
......@@ -68,6 +77,7 @@ public class PasswordCheckEditFragmentView extends PreferenceFragmentCompat {
super.onViewCreated(view, savedInstanceState);
mCredential = getCredentialFromInstanceStateOrLaunchBundle(savedInstanceState);
mNewPassword = getNewPasswordFromInstanceStateOrLaunchBundle(savedInstanceState);
mPasswordVisible = getViewButtonPressedFromInstanceState(savedInstanceState);
EditText siteText = (EditText) view.findViewById(R.id.site_edit);
siteText.setText(mCredential.getDisplayOrigin());
......@@ -93,6 +103,16 @@ public class PasswordCheckEditFragmentView extends PreferenceFragmentCompat {
});
// Enforce that even the initial password (maybe from a saved instance) cannot be empty.
checkSavingConditions(TextUtils.isEmpty(mNewPassword));
mViewPasswordButton = view.findViewById(R.id.password_entry_editor_view_password);
mViewPasswordButton.setOnClickListener(unusedView -> this.unmaskPassword());
mMaskPasswordButton = view.findViewById(R.id.password_entry_editor_mask_password);
mMaskPasswordButton.setOnClickListener(unusedView -> this.maskPassword());
if (mPasswordVisible) {
maskPassword();
} else {
unmaskPassword();
}
}
@Override
......@@ -104,11 +124,23 @@ public class PasswordCheckEditFragmentView extends PreferenceFragmentCompat {
// TODO(crbug.com/1092444): Make the back arrow an 'X'.
}
@Override
public void onResume() {
super.onResume();
if (!ReauthenticationManager.authenticationStillValid(ReauthScope.ONE_AT_A_TIME)) {
// If the page was idle (e.g. screenlocked for a few minutes), go back to the previous
// page to ensure the user goes through reauth again.
// TODO(crbug.com/1114720): Trigger another reauth instead.
getActivity().finish();
}
}
@Override
public void onSaveInstanceState(@NonNull Bundle outState) {
super.onSaveInstanceState(outState);
outState.putParcelable(EXTRA_COMPROMISED_CREDENTIAL, mCredential);
outState.putString(EXTRA_NEW_PASSWORD, mNewPassword);
outState.putBoolean(EXTRA_PASSWORD_MASKED, mPasswordVisible);
}
@Override
......@@ -158,4 +190,29 @@ public class PasswordCheckEditFragmentView extends PreferenceFragmentCompat {
R.string.pref_edit_dialog_field_required_validation_message)
: "");
}
private boolean getViewButtonPressedFromInstanceState(Bundle savedInstanceState) {
return (savedInstanceState != null && savedInstanceState.containsKey(EXTRA_PASSWORD_MASKED))
&& savedInstanceState.getBoolean(EXTRA_PASSWORD_MASKED);
}
private void maskPassword() {
getActivity().getWindow().clearFlags(WindowManager.LayoutParams.FLAG_SECURE);
mPasswordText.setInputType(InputType.TYPE_CLASS_TEXT
| InputType.TYPE_TEXT_VARIATION_PASSWORD | InputType.TYPE_TEXT_FLAG_MULTI_LINE);
mViewPasswordButton.setVisibility(View.VISIBLE);
mMaskPasswordButton.setVisibility(View.GONE);
mPasswordVisible = false;
}
private void unmaskPassword() {
getActivity().getWindow().setFlags(
WindowManager.LayoutParams.FLAG_SECURE, WindowManager.LayoutParams.FLAG_SECURE);
mPasswordText.setInputType(InputType.TYPE_CLASS_TEXT
| InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD
| InputType.TYPE_TEXT_FLAG_MULTI_LINE);
mViewPasswordButton.setVisibility(View.GONE);
mMaskPasswordButton.setVisibility(View.VISIBLE);
mPasswordVisible = true;
}
}
......@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.password_check;
import static android.text.InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD;
import static androidx.test.espresso.Espresso.onView;
import static androidx.test.espresso.action.ViewActions.click;
import static androidx.test.espresso.assertion.ViewAssertions.matches;
......@@ -13,6 +15,7 @@ import static androidx.test.espresso.matcher.ViewMatchers.withId;
import static junit.framework.Assert.assertTrue;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
......@@ -25,14 +28,18 @@ import static org.chromium.content_public.browser.test.util.CriteriaHelper.pollU
import static org.chromium.content_public.browser.test.util.TestThreadUtils.runOnUiThreadBlocking;
import android.os.Bundle;
import android.text.InputType;
import android.view.View;
import android.widget.EditText;
import android.widget.ImageButton;
import androidx.annotation.StringRes;
import androidx.test.filters.MediumTest;
import com.google.android.material.textfield.TextInputLayout;
import org.hamcrest.BaseMatcher;
import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
......@@ -43,11 +50,14 @@ import org.mockito.MockitoAnnotations;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.password_manager.settings.ReauthenticationManager;
import org.chromium.chrome.browser.settings.SettingsActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import org.chromium.url.GURL;
import java.util.concurrent.ExecutionException;
/**
* View tests for the Password Check Edit screen only.
*/
......@@ -99,7 +109,7 @@ public class PasswordCheckEditViewTest {
assertNotNull(password.getText());
assertNotNull(password.getText().toString());
assertThat(password.getText().toString(), equalTo(ANA.getPassword()));
assertTrue((password.getInputType() & InputType.TYPE_TEXT_VARIATION_VISIBLE_PASSWORD) != 0);
assertTrue((password.getInputType() & TYPE_TEXT_VARIATION_VISIBLE_PASSWORD) != 0);
}
@Test
......@@ -151,7 +161,43 @@ public class PasswordCheckEditViewTest {
equalTo(getString(R.string.pref_edit_dialog_field_required_validation_message)));
}
@Test
@MediumTest
public void testMasksPasswordOnEyeIconClick() throws ExecutionException {
EditText password = mPasswordCheckEditView.getView().findViewById(R.id.password_edit);
ImageButton unmaskButton = mPasswordCheckEditView.getView().findViewById(
R.id.password_entry_editor_view_password);
ImageButton maskButton = mPasswordCheckEditView.getView().findViewById(
R.id.password_entry_editor_mask_password);
assertNotNull(password);
assertNotNull(unmaskButton);
assertNotNull(maskButton);
// Unmasked by default since the user just reauthenticated.
assertThat(maskButton.getVisibility(), is(View.VISIBLE));
assertThat(unmaskButton.getVisibility(), is(View.GONE));
assertThat(password, isVisiblePasswordInput(true));
// Clicking the mask button obfuscates the password.
runOnUiThreadBlocking(maskButton::callOnClick);
assertThat(maskButton.getVisibility(), is(View.GONE));
assertThat(unmaskButton.getVisibility(), is(View.VISIBLE));
assertThat(password, isVisiblePasswordInput(false));
// Clicking the unmask button shows the password again.
runOnUiThreadBlocking(unmaskButton::callOnClick);
assertThat(maskButton.getVisibility(), is(View.VISIBLE));
assertThat(unmaskButton.getVisibility(), is(View.GONE));
assertThat(password, isVisiblePasswordInput(true));
}
private void setUpUiLaunchedFromSettings() {
ReauthenticationManager.setApiOverride(ReauthenticationManager.OverrideState.AVAILABLE);
ReauthenticationManager.setScreenLockSetUpOverride(
ReauthenticationManager.OverrideState.AVAILABLE);
ReauthenticationManager.recordLastReauth(
System.currentTimeMillis(), ReauthenticationManager.ReauthScope.BULK);
Bundle fragmentArgs = new Bundle();
fragmentArgs.putParcelable(EXTRA_COMPROMISED_CREDENTIAL, ANA);
mTestRule.startSettingsActivity(fragmentArgs);
......@@ -161,4 +207,29 @@ public class PasswordCheckEditViewTest {
private String getString(@StringRes int stringId) {
return mPasswordCheckEditView.getContext().getString(stringId);
}
/**
* Matches any {@link EditText} which has the content visibility matching to |shouldBeVisible|.
* @return The matcher checking the input type.
*/
private Matcher<EditText> isVisiblePasswordInput(boolean shouldBeVisible) {
return new BaseMatcher<EditText>() {
@Override
public boolean matches(Object o) {
EditText editText = (EditText) o;
return ((editText.getInputType() & TYPE_TEXT_VARIATION_VISIBLE_PASSWORD)
== TYPE_TEXT_VARIATION_VISIBLE_PASSWORD)
== shouldBeVisible;
}
@Override
public void describeTo(Description description) {
if (shouldBeVisible) {
description.appendText("The content should be visible.");
} else {
description.appendText("The content should not be visible.");
}
}
};
}
}
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