Commit 01186b68 authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

Reland "[Mfill Android] Render credit card warning in TextView"

This is a reland of 3cd4817b

The original CL was based on https://crrev.com/c/1729592 which
was reverted. Once the reland https://crrev.com/c/1741916 is merged,
this CL should work as intended.

TBR=jdoerrie@chromium.org,ioanap@chromium.org

Original change's description:
> [Mfill Android] Render credit card warning in TextView
>
> This CL ensures that the unsafe origin warning is rendered as TextView
> and that fields which are marked as disabled are actually disabled.
>
> Bug: 965500
> Change-Id: I2c0c66e86075b3152431dcd18b4922c1167786e9
> Reviewed-on:
https://chromium-review.googlesource.com/c/chromium/src/+/1736355
> Reviewed-by: Ioana Pandele <ioanap@chromium.org>
> Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#685112}

Bug: 965500
Change-Id: I0cbe46c4d7e0a6d2fb936a1b0dcb99c657af2a9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1742236Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685537}
parent 1c4cac07
......@@ -40,6 +40,7 @@
style="@style/InputChip" />
<LinearLayout
android:id="@+id/exp_group"
android:gravity="center_vertical|start"
android:fillViewport="true"
android:layout_height="@dimen/keyboard_accessory_suggestion_height"
......
......@@ -119,8 +119,9 @@ class ManualFillingComponentBridge {
}
@CalledByNative
private static Object createAccessorySheetData(@AccessoryTabType int type, String title) {
return new AccessorySheetData(type, title);
private static Object createAccessorySheetData(
@AccessoryTabType int type, String title, String warning) {
return new AccessorySheetData(type, title, warning);
}
@CalledByNative
......
......@@ -125,6 +125,8 @@ class KeyboardAccessoryMediator
*/
private boolean shouldShowSuggestion(AutofillSuggestion suggestion) {
switch (suggestion.getSuggestionId()) {
case PopupItemId.ITEM_ID_INSECURE_CONTEXT_PAYMENT_DISABLED_MESSAGE:
// The insecure context warning has a replacement in the fallback sheet.
case PopupItemId.ITEM_ID_SEPARATOR:
case PopupItemId.ITEM_ID_CLEAR_FORM:
case PopupItemId.ITEM_ID_CREDIT_CARD_SIGNIN_PROMO:
......@@ -134,9 +136,6 @@ class KeyboardAccessoryMediator
case PopupItemId.ITEM_ID_SHOW_ACCOUNT_CARDS:
case PopupItemId.ITEM_ID_AUTOFILL_OPTIONS:
return false;
case PopupItemId.ITEM_ID_INSECURE_CONTEXT_PAYMENT_DISABLED_MESSAGE:
return ChromeFeatureList.isEnabled(
ChromeFeatureList.AUTOFILL_MANUAL_FALLBACK_ANDROID);
case PopupItemId.ITEM_ID_AUTOCOMPLETE_ENTRY:
case PopupItemId.ITEM_ID_PASSWORD_ENTRY:
case PopupItemId.ITEM_ID_DATALIST_ENTRY:
......
......@@ -64,6 +64,9 @@ class AccessorySheetTabMediator implements Provider.Observer<AccessorySheetData>
if (shouldShowTitle(accessorySheetData.getUserInfoList())) {
items.add(new AccessorySheetDataPiece(accessorySheetData.getTitle(), Type.TITLE));
}
if (!accessorySheetData.getWarning().isEmpty()) {
items.add(new AccessorySheetDataPiece(accessorySheetData.getWarning(), Type.WARNING));
}
for (UserInfo userInfo : accessorySheetData.getUserInfoList()) {
items.add(new AccessorySheetDataPiece(userInfo, mUserInfoType));
}
......
......@@ -25,7 +25,7 @@ class AccessorySheetTabModel extends ListModel<AccessorySheetTabModel.AccessoryS
*/
static class AccessorySheetDataPiece {
@IntDef({Type.TITLE, Type.PASSWORD_INFO, Type.ADDRESS_INFO, Type.CREDIT_CARD_INFO,
Type.TOUCH_TO_FILL_INFO, Type.FOOTER_COMMAND})
Type.TOUCH_TO_FILL_INFO, Type.FOOTER_COMMAND, Type.WARNING})
@Retention(RetentionPolicy.SOURCE)
@interface Type {
/**
......@@ -52,6 +52,10 @@ class AccessorySheetTabModel extends ListModel<AccessorySheetTabModel.AccessoryS
* A command at the end of the accessory sheet tab.
*/
int FOOTER_COMMAND = 6;
/**
* An optional warning to be displayed at the beginning of a sheet.
*/
int WARNING = 7;
}
private Object mDataPiece;
......
......@@ -23,6 +23,7 @@ import org.chromium.ui.widget.ChipView;
class CreditCardAccessoryInfoView extends LinearLayout {
private ImageView mIcon;
private ChipView mCCNumber;
private LinearLayout mExpiryGroup;
private ChipView mExpMonth;
private ChipView mExpYear;
private ChipView mCardholder;
......@@ -40,6 +41,7 @@ class CreditCardAccessoryInfoView extends LinearLayout {
mIcon = findViewById(R.id.icon);
mCCNumber = findViewById(R.id.cc_number);
mExpiryGroup = findViewById(R.id.exp_group);
mExpMonth = findViewById(R.id.exp_month);
mExpYear = findViewById(R.id.exp_year);
mCardholder = findViewById(R.id.cardholder);
......@@ -69,4 +71,8 @@ class CreditCardAccessoryInfoView extends LinearLayout {
public ChipView getCardholder() {
return mCardholder;
}
public LinearLayout getExpiryGroup() {
return mExpiryGroup;
}
}
......@@ -22,6 +22,7 @@ import org.chromium.ui.widget.ChipView;
class CreditCardAccessorySheetViewBinder {
static ElementViewHolder create(ViewGroup parent, @AccessorySheetDataPiece.Type int viewType) {
switch (viewType) {
case AccessorySheetDataPiece.Type.WARNING: // Fallthrough to reuse title container.
case AccessorySheetDataPiece.Type.TITLE:
return new AccessorySheetTabViewBinder.TitleViewHolder(
parent, R.layout.keyboard_accessory_sheet_tab_title);
......@@ -50,6 +51,11 @@ class CreditCardAccessorySheetViewBinder {
bindChipView(view.getExpYear(), info.getFields().get(2));
bindChipView(view.getCardholder(), info.getFields().get(3));
view.getExpiryGroup().setVisibility(view.getExpYear().getVisibility() == View.VISIBLE
|| view.getExpMonth().getVisibility() == View.VISIBLE
? View.VISIBLE
: View.GONE);
view.setIcon(AppCompatResources.getDrawable(
view.getContext(), getDrawableForOrigin(info.getOrigin())));
}
......@@ -57,11 +63,11 @@ class CreditCardAccessorySheetViewBinder {
private static void bindChipView(ChipView chip, UserInfoField field) {
chip.getPrimaryTextView().setText(field.getDisplayText());
chip.getPrimaryTextView().setContentDescription(field.getA11yDescription());
if (!field.isSelectable() || field.getDisplayText().isEmpty()) {
chip.setVisibility(View.GONE);
chip.setVisibility(field.getDisplayText().isEmpty() ? View.GONE : View.VISIBLE);
if (!field.isSelectable()) {
chip.setEnabled(false);
return;
}
chip.setVisibility(View.VISIBLE);
chip.setOnClickListener(src -> field.triggerSelection());
chip.setClickable(true);
chip.setEnabled(true);
......
......@@ -139,7 +139,33 @@ public class CreditCardAccessorySheetViewTest {
@Test
@MediumTest
public void testEmptyChipsAreNotVisible() throws ExecutionException {
public void testAddingUnselectableFieldsRendersUnclickabeChips() throws ExecutionException {
assertThat(mView.get().getChildCount(), is(0));
TestThreadUtils.runOnUiThreadBlocking(() -> {
UserInfo infoWithUnclickableField = new UserInfo("", null);
infoWithUnclickableField.addField(
new UserInfoField("4111111111111111", "4111111111111111", "", false, null));
infoWithUnclickableField.addField(new UserInfoField("", "", "month", false, null));
infoWithUnclickableField.addField(new UserInfoField("", "", "year", false, null));
infoWithUnclickableField.addField(new UserInfoField("", "", "name", false, null));
mModel.add(new AccessorySheetDataPiece(
infoWithUnclickableField, AccessorySheetDataPiece.Type.CREDIT_CARD_INFO));
mModel.add(new AccessorySheetDataPiece(
new KeyboardAccessoryData.FooterCommand("Manage credit cards", null),
AccessorySheetDataPiece.Type.FOOTER_COMMAND));
});
CriteriaHelper.pollUiThread(Criteria.equals(2, () -> mView.get().getChildCount()));
assertThat(getChipText(R.id.cc_number), is("4111111111111111"));
assertThat(findChipView(R.id.cc_number).isShown(), is(true));
assertThat(findChipView(R.id.cc_number).isEnabled(), is(false));
}
@Test
@MediumTest
public void testEmptyChipsAreNotVisible() {
final AtomicBoolean clicked = new AtomicBoolean();
assertThat(mView.get().getChildCount(), is(0));
......@@ -158,6 +184,29 @@ public class CreditCardAccessorySheetViewTest {
assertThat(findChipView(R.id.cardholder).isShown(), is(false));
}
@Test
@MediumTest
public void testRendersWarning() {
final String kWarning = "Insecure, so filling is no.";
assertThat(mView.get().getChildCount(), is(0));
TestThreadUtils.runOnUiThreadBlocking(() -> {
mModel.add(new AccessorySheetDataPiece(kWarning, AccessorySheetDataPiece.Type.WARNING));
mModel.add(new AccessorySheetDataPiece(
new KeyboardAccessoryData.FooterCommand("Manage credit cards", null),
AccessorySheetDataPiece.Type.FOOTER_COMMAND));
});
CriteriaHelper.pollUiThread(Criteria.equals(2, () -> mView.get().getChildCount()));
assertThat(mView.get().getChildAt(0), instanceOf(LinearLayout.class));
LinearLayout warning = (LinearLayout) mView.get().getChildAt(0);
assertThat(warning.findViewById(R.id.tab_title), instanceOf(TextView.class));
TextView warningText = warning.findViewById(R.id.tab_title);
assertThat(warningText.isShown(), is(true));
assertThat(warningText.getText(), is(kWarning));
}
private UserInfo createInfo(
String number, String month, String year, String name, AtomicBoolean clickRecorder) {
UserInfo info = new UserInfo("", null);
......
......@@ -200,7 +200,7 @@ public class ManualFillingControllerTest {
*/
void providePasswordSheet(String passwordString) {
AccessorySheetData sheetData =
new AccessorySheetData(AccessoryTabType.PASSWORDS, "Passwords");
new AccessorySheetData(AccessoryTabType.PASSWORDS, "Passwords", "");
UserInfo userInfo = new UserInfo("", null);
userInfo.addField(
new UserInfoField("(No username)", "No username", /*id=*/"", false, null));
......
......@@ -107,13 +107,13 @@ public class AddressAccessorySheetControllerTest {
// If the coordinator receives a set of initial items, the model should report an insertion.
testProvider.notifyObservers(
new AccessorySheetData(AccessoryTabType.ADDRESSES, "Addresses"));
new AccessorySheetData(AccessoryTabType.ADDRESSES, "Addresses", ""));
verify(mMockItemListObserver).onItemRangeInserted(mSheetDataPieces, 0, 1);
assertThat(mSheetDataPieces.size(), is(1));
// If the coordinator receives a new set of items, the model should report a change.
testProvider.notifyObservers(
new AccessorySheetData(AccessoryTabType.ADDRESSES, "Other Addresses"));
new AccessorySheetData(AccessoryTabType.ADDRESSES, "Other Addresses", ""));
verify(mMockItemListObserver).onItemRangeChanged(mSheetDataPieces, 0, 1, null);
assertThat(mSheetDataPieces.size(), is(1));
......@@ -131,7 +131,7 @@ public class AddressAccessorySheetControllerTest {
public void testSplitsTabDataToList() {
final PropertyProvider<AccessorySheetData> testProvider = new PropertyProvider<>();
final AccessorySheetData testData =
new AccessorySheetData(AccessoryTabType.ADDRESSES, "Addresses for this site");
new AccessorySheetData(AccessoryTabType.ADDRESSES, "Addresses for this site", "");
testData.getUserInfoList().add(new UserInfo("", null));
testData.getUserInfoList().get(0).addField(
new UserInfoField("Name", "Name", "", false, null));
......@@ -150,7 +150,7 @@ public class AddressAccessorySheetControllerTest {
public void testUsesTitleElementForEmptyState() {
final PropertyProvider<AccessorySheetData> testProvider = new PropertyProvider<>();
final AccessorySheetData testData =
new AccessorySheetData(AccessoryTabType.ADDRESSES, "No addresses");
new AccessorySheetData(AccessoryTabType.ADDRESSES, "No addresses", "");
mCoordinator.registerDataProvider(testProvider);
testProvider.notifyObservers(testData);
......@@ -183,7 +183,7 @@ public class AddressAccessorySheetControllerTest {
// If the tab is shown without interactive item, log "0" samples.
AccessorySheetData accessorySheetData =
new AccessorySheetData(AccessoryTabType.ADDRESSES, "No addresses!");
new AccessorySheetData(AccessoryTabType.ADDRESSES, "No addresses!", "");
testProvider.notifyObservers(accessorySheetData);
mCoordinator.onTabShown();
......@@ -203,7 +203,7 @@ public class AddressAccessorySheetControllerTest {
// Add only two interactive items - the third one should not be recorded.
AccessorySheetData accessorySheetData =
new AccessorySheetData(AccessoryTabType.ADDRESSES, "Addresses");
new AccessorySheetData(AccessoryTabType.ADDRESSES, "Addresses", "");
accessorySheetData.getUserInfoList().add(new UserInfo("", null));
accessorySheetData.getUserInfoList().get(0).addField(
new UserInfoField("Todd Tester", "Todd Tester", "0", false, result -> {}));
......
......@@ -108,13 +108,13 @@ public class CreditCardAccessorySheetControllerTest {
// If the coordinator receives a set of initial items, the model should report an insertion.
testProvider.notifyObservers(
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Payments"));
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Payments", ""));
verify(mMockItemListObserver).onItemRangeInserted(mSheetDataPieces, 0, 1);
assertThat(mSheetDataPieces.size(), is(1));
// If the coordinator receives a new set of items, the model should report a change.
testProvider.notifyObservers(
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Other Payments"));
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Other Payments", ""));
verify(mMockItemListObserver).onItemRangeChanged(mSheetDataPieces, 0, 1, null);
assertThat(mSheetDataPieces.size(), is(1));
......@@ -132,7 +132,7 @@ public class CreditCardAccessorySheetControllerTest {
public void testSplitsTabDataToList() {
final PropertyProvider<AccessorySheetData> testProvider = new PropertyProvider<>();
final AccessorySheetData testData =
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Payments");
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Payments", "");
testData.getUserInfoList().add(new UserInfo("", null));
testData.getUserInfoList().get(0).addField(
new UserInfoField("Todd", "Todd", "", false, field -> {}));
......@@ -151,7 +151,7 @@ public class CreditCardAccessorySheetControllerTest {
public void testUsesTitleElementForEmptyState() {
final PropertyProvider<AccessorySheetData> testProvider = new PropertyProvider<>();
final AccessorySheetData testData =
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Payments");
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Payments", "");
mCoordinator.registerDataProvider(testProvider);
testProvider.notifyObservers(testData);
......@@ -184,7 +184,7 @@ public class CreditCardAccessorySheetControllerTest {
// If the tab is shown without interactive item, log "0" samples.
AccessorySheetData accessorySheetData =
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Payments");
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Payments", "");
testProvider.notifyObservers(accessorySheetData);
mCoordinator.onTabShown();
......@@ -204,7 +204,7 @@ public class CreditCardAccessorySheetControllerTest {
// Add only two interactive items - the third one should not be recorded.
AccessorySheetData accessorySheetData =
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Payments");
new AccessorySheetData(AccessoryTabType.CREDIT_CARDS, "Payments", "");
accessorySheetData.getUserInfoList().add(new UserInfo("", null));
accessorySheetData.getUserInfoList().get(0).addField(
new UserInfoField("Todd Tester", "Todd Tester", "0", false, result -> {}));
......
......@@ -110,13 +110,13 @@ public class PasswordAccessorySheetControllerTest {
// If the coordinator receives a set of initial items, the model should report an insertion.
testProvider.notifyObservers(
new AccessorySheetData(AccessoryTabType.PASSWORDS, "Passwords"));
new AccessorySheetData(AccessoryTabType.PASSWORDS, "Passwords", ""));
verify(mMockItemListObserver).onItemRangeInserted(mSheetDataPieces, 0, 1);
assertThat(mSheetDataPieces.size(), is(1));
// If the coordinator receives a new set of items, the model should report a change.
testProvider.notifyObservers(
new AccessorySheetData(AccessoryTabType.PASSWORDS, "Other Passwords"));
new AccessorySheetData(AccessoryTabType.PASSWORDS, "Other Passwords", ""));
verify(mMockItemListObserver).onItemRangeChanged(mSheetDataPieces, 0, 1, null);
assertThat(mSheetDataPieces.size(), is(1));
......@@ -135,7 +135,7 @@ public class PasswordAccessorySheetControllerTest {
setAutofillFeature(false);
final PropertyProvider<AccessorySheetData> testProvider = new PropertyProvider<>();
final AccessorySheetData testData =
new AccessorySheetData(AccessoryTabType.PASSWORDS, "Passwords for this site");
new AccessorySheetData(AccessoryTabType.PASSWORDS, "Passwords for this site", "");
testData.getUserInfoList().add(new UserInfo("", null));
testData.getUserInfoList().get(0).addField(
new UserInfoField("Name", "Name", "", false, null));
......@@ -160,7 +160,7 @@ public class PasswordAccessorySheetControllerTest {
setAutofillFeature(true);
final PropertyProvider<AccessorySheetData> testProvider = new PropertyProvider<>();
final AccessorySheetData testData =
new AccessorySheetData(AccessoryTabType.PASSWORDS, "No passwords for this");
new AccessorySheetData(AccessoryTabType.PASSWORDS, "No passwords for this", "");
mCoordinator.registerDataProvider(testProvider);
// Providing only FooterCommands and no User Info shows the title as empty state:
......@@ -207,7 +207,7 @@ public class PasswordAccessorySheetControllerTest {
// If the tab is shown without interactive item, log "0" samples.
AccessorySheetData accessorySheetData =
new AccessorySheetData(AccessoryTabType.PASSWORDS, "No passwords!");
new AccessorySheetData(AccessoryTabType.PASSWORDS, "No passwords!", "");
accessorySheetData.getFooterCommands().add(new FooterCommand("Manage all passwords", null));
accessorySheetData.getFooterCommands().add(new FooterCommand("Generate password", null));
testProvider.notifyObservers(accessorySheetData);
......
......@@ -267,6 +267,7 @@ public class KeyboardAccessoryData {
*/
public final static class AccessorySheetData {
private final String mTitle;
private final String mWarning;
private final @AccessoryTabType int mSheetType;
private final List<UserInfo> mUserInfoList = new ArrayList<>();
private final List<FooterCommand> mFooterCommands = new ArrayList<>();
......@@ -274,10 +275,12 @@ public class KeyboardAccessoryData {
/**
* Creates the AccessorySheetData object.
* @param title The title of accessory sheet tab.
* @param warning An optional warning to be displayed the beginning of the sheet.
*/
public AccessorySheetData(@AccessoryTabType int sheetType, String title) {
public AccessorySheetData(@AccessoryTabType int sheetType, String title, String warning) {
mSheetType = sheetType;
mTitle = title;
mWarning = warning;
}
public @AccessoryTabType int getSheetType() {
......@@ -291,6 +294,13 @@ public class KeyboardAccessoryData {
return mTitle;
}
/**
* Returns a warning to be displayed at the beginning of the sheet. Empty string otherwise.
*/
public String getWarning() {
return mWarning;
}
/**
* Returns the list of {@link UserInfo} to be shown on the accessory sheet.
*/
......
......@@ -152,7 +152,8 @@ ManualFillingViewAndroid::ConvertAccessorySheetDataToJavaObject(
ScopedJavaLocalRef<jobject> j_tab_data =
Java_ManualFillingComponentBridge_createAccessorySheetData(
env, static_cast<int>(tab_data.get_sheet_type()),
ConvertUTF16ToJavaString(env, tab_data.title()));
ConvertUTF16ToJavaString(env, tab_data.title()),
ConvertUTF16ToJavaString(env, tab_data.warning()));
for (const UserInfo& user_info : tab_data.user_info_list()) {
ScopedJavaLocalRef<jobject> j_user_info =
......
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