Commit f9a1bb01 authored by Jinho Bang's avatar Jinho Bang Committed by Commit Bot

PaymentRequest: Fix a bug that retry errors are not consistently shown

This patch includes the following things:
  - Add retry error labels for dropdown fields
  - Move the reset logic from afterTextChanged() to onTextChanged()
    because the afterTextChanged() event happens when setText() is
    called from formatters (e.g. phone number formatter)
  - Re-enable testRetryWithShippingAddressErrors() test

Render Tests:
  PaymentRequestRetryTest#testRetryWithShippingAddressErrors
  PaymentRequestRetryTest#testRetryWithPayerErrors

Manual Tests:
  wpt/payment-request/PaymentValidationErrors/retry-shows-shippingAddress-member-manual.https.html
  wpt/payment-request/PaymentValidationErrors/retry-shows-payer-member-manual.https.html

Bug: 861704, 926252, 942216
Change-Id: I1917e5c6cea05bcb49221c4eee21afa676f50885
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1610258Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Jinho Bang <jinho.bang@samsung.com>
Cr-Commit-Position: refs/heads/master@{#660733}
parent 6b9bf768
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
<!-- PaymentRequestUI editor dialog. --> <!-- PaymentRequestUI editor dialog. -->
<RelativeLayout <RelativeLayout
xmlns:android="http://schemas.android.com/apk/res/android" xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/editor_container"
android:background="@color/modern_primary_color"> android:background="@color/modern_primary_color">
<!-- Toolbar --> <!-- Toolbar -->
......
...@@ -27,6 +27,14 @@ ...@@ -27,6 +27,14 @@
android:padding="0dp" android:padding="0dp"
android:dropDownWidth="match_parent" /> android:dropDownWidth="match_parent" />
<View style="@style/PreferenceSpinnerUnderlineView" /> <View android:id="@+id/spinner_underline" style="@style/PreferenceSpinnerUnderlineView" />
</LinearLayout> <TextView
\ No newline at end of file android:id="@+id/spinner_error"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/pref_autofill_field_top_margin"
android:visibility="gone"
android:textAppearance="@style/TextAppearance.ErrorCaption" />
</LinearLayout>
...@@ -21,6 +21,7 @@ import android.widget.ArrayAdapter; ...@@ -21,6 +21,7 @@ import android.widget.ArrayAdapter;
import android.widget.Spinner; import android.widget.Spinner;
import android.widget.TextView; import android.widget.TextView;
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.preferences.autofill.AutofillProfileBridge.DropdownKeyValue; import org.chromium.chrome.browser.preferences.autofill.AutofillProfileBridge.DropdownKeyValue;
import org.chromium.ui.KeyboardVisibilityDelegate; import org.chromium.ui.KeyboardVisibilityDelegate;
...@@ -37,6 +38,8 @@ class EditorDropdownField implements EditorFieldView { ...@@ -37,6 +38,8 @@ class EditorDropdownField implements EditorFieldView {
private final View mLayout; private final View mLayout;
private final TextView mLabel; private final TextView mLabel;
private final Spinner mDropdown; private final Spinner mDropdown;
private final View mUnderline;
private final TextView mErrorLabel;
private int mSelectedIndex; private int mSelectedIndex;
private ArrayAdapter<CharSequence> mAdapter; private ArrayAdapter<CharSequence> mAdapter;
@Nullable @Nullable
...@@ -66,6 +69,10 @@ class EditorDropdownField implements EditorFieldView { ...@@ -66,6 +69,10 @@ class EditorDropdownField implements EditorFieldView {
? mFieldModel.getLabel() + EditorDialog.REQUIRED_FIELD_INDICATOR ? mFieldModel.getLabel() + EditorDialog.REQUIRED_FIELD_INDICATOR
: mFieldModel.getLabel()); : mFieldModel.getLabel());
mUnderline = mLayout.findViewById(R.id.spinner_underline);
mErrorLabel = mLayout.findViewById(R.id.spinner_error);
final List<DropdownKeyValue> dropdownKeyValues = mFieldModel.getDropdownKeyValues(); final List<DropdownKeyValue> dropdownKeyValues = mFieldModel.getDropdownKeyValues();
final List<CharSequence> dropdownValues = getDropdownValues(dropdownKeyValues); final List<CharSequence> dropdownValues = getDropdownValues(dropdownKeyValues);
if (mFieldModel.getHint() != null) { if (mFieldModel.getHint() != null) {
...@@ -121,6 +128,8 @@ class EditorDropdownField implements EditorFieldView { ...@@ -121,6 +128,8 @@ class EditorDropdownField implements EditorFieldView {
} }
mSelectedIndex = position; mSelectedIndex = position;
mFieldModel.setDropdownKey(key, changedCallback); mFieldModel.setDropdownKey(key, changedCallback);
mFieldModel.setCustomErrorMessage(null);
updateDisplayedError(false);
} }
if (mObserverForTest != null) { if (mObserverForTest != null) {
mObserverForTest.onEditorTextUpdate(); mObserverForTest.onEditorTextUpdate();
...@@ -176,8 +185,16 @@ class EditorDropdownField implements EditorFieldView { ...@@ -176,8 +185,16 @@ class EditorDropdownField implements EditorFieldView {
drawable.setBounds( drawable.setBounds(
0, 0, drawable.getIntrinsicWidth(), drawable.getIntrinsicHeight()); 0, 0, drawable.getIntrinsicWidth(), drawable.getIntrinsicHeight());
((TextView) view).setError(mFieldModel.getErrorMessage(), drawable); ((TextView) view).setError(mFieldModel.getErrorMessage(), drawable);
mUnderline.setBackgroundColor(ApiCompatibilityUtils.getColor(
mContext.getResources(), R.color.error_text_color));
mErrorLabel.setText(mFieldModel.getErrorMessage());
mErrorLabel.setVisibility(View.VISIBLE);
} else { } else {
((TextView) view).setError(null); ((TextView) view).setError(null);
mUnderline.setBackgroundColor(ApiCompatibilityUtils.getColor(
mContext.getResources(), R.color.modern_grey_600));
mErrorLabel.setText(null);
mErrorLabel.setVisibility(View.GONE);
} }
} }
} }
......
...@@ -124,7 +124,6 @@ public class EditorTextField extends FrameLayout implements EditorFieldView, Vie ...@@ -124,7 +124,6 @@ public class EditorTextField extends FrameLayout implements EditorFieldView, Vie
@Override @Override
public void afterTextChanged(Editable s) { public void afterTextChanged(Editable s) {
fieldModel.setValue(s.toString()); fieldModel.setValue(s.toString());
fieldModel.setCustomErrorMessage(null);
updateDisplayedError(false); updateDisplayedError(false);
updateFieldValueIcon(false); updateFieldValueIcon(false);
if (mObserverForTest != null) { if (mObserverForTest != null) {
...@@ -143,7 +142,11 @@ public class EditorTextField extends FrameLayout implements EditorFieldView, Vie ...@@ -143,7 +142,11 @@ public class EditorTextField extends FrameLayout implements EditorFieldView, Vie
public void beforeTextChanged(CharSequence s, int start, int count, int after) {} public void beforeTextChanged(CharSequence s, int start, int count, int after) {}
@Override @Override
public void onTextChanged(CharSequence s, int start, int before, int count) {} public void onTextChanged(CharSequence s, int start, int before, int count) {
if (mInput.hasFocus()) {
fieldModel.setCustomErrorMessage(null);
}
}
}); });
// Display any autofill suggestions. // Display any autofill suggestions.
......
...@@ -16,7 +16,6 @@ import org.junit.Test; ...@@ -16,7 +16,6 @@ import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.chromium.base.test.util.CommandLineFlags; import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeSwitches; import org.chromium.chrome.browser.ChromeSwitches;
...@@ -27,6 +26,7 @@ import org.chromium.chrome.browser.autofill.PersonalDataManager.CreditCard; ...@@ -27,6 +26,7 @@ import org.chromium.chrome.browser.autofill.PersonalDataManager.CreditCard;
import org.chromium.chrome.browser.payments.PaymentRequestTestRule.MainActivityStartCallback; import org.chromium.chrome.browser.payments.PaymentRequestTestRule.MainActivityStartCallback;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ui.DisableAnimationsTestRule; import org.chromium.chrome.test.ui.DisableAnimationsTestRule;
import org.chromium.chrome.test.util.RenderTestRule;
import org.chromium.ui.modaldialog.ModalDialogProperties; import org.chromium.ui.modaldialog.ModalDialogProperties;
import java.util.concurrent.TimeoutException; import java.util.concurrent.TimeoutException;
...@@ -46,6 +46,10 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback { ...@@ -46,6 +46,10 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback {
public PaymentRequestTestRule mPaymentRequestTestRule = public PaymentRequestTestRule mPaymentRequestTestRule =
new PaymentRequestTestRule("payment_request_retry.html", this); new PaymentRequestTestRule("payment_request_retry.html", this);
@Rule
public RenderTestRule mRenderTestRule =
new RenderTestRule("components/test/data/payments/render_tests");
@Override @Override
public void onMainActivityStarted() throws InterruptedException, TimeoutException { public void onMainActivityStarted() throws InterruptedException, TimeoutException {
AutofillTestHelper helper = new AutofillTestHelper(); AutofillTestHelper helper = new AutofillTestHelper();
...@@ -111,11 +115,10 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback { ...@@ -111,11 +115,10 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback {
/** /**
* Test for retry() with shipping address errors. * Test for retry() with shipping address errors.
*/ */
@DisabledTest(message = "crbug.com/926252")
@Test @Test
@MediumTest @MediumTest
@Feature({"Payments"}) @Feature({"Payments", "RenderTest"})
public void testRetryWithShippingAddressErrors() throws InterruptedException, TimeoutException { public void testRetryWithShippingAddressErrors() throws Throwable {
mPaymentRequestTestRule.triggerUIAndWait(mPaymentRequestTestRule.getReadyForInput()); mPaymentRequestTestRule.triggerUIAndWait(mPaymentRequestTestRule.getReadyForInput());
mPaymentRequestTestRule.clickAndWait( mPaymentRequestTestRule.clickAndWait(
R.id.button_primary, mPaymentRequestTestRule.getReadyForUnmaskInput()); R.id.button_primary, mPaymentRequestTestRule.getReadyForUnmaskInput());
...@@ -127,14 +130,33 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback { ...@@ -127,14 +130,33 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback {
mPaymentRequestTestRule.retryPaymentRequest("{" mPaymentRequestTestRule.retryPaymentRequest("{"
+ " shippingAddress: {" + " shippingAddress: {"
+ " country: 'COUNTRY ERROR',"
+ " recipient: 'RECIPIENT ERROR',"
+ " organization: 'ORGANIZATION ERROR',"
+ " addressLine: 'ADDRESS LINE ERROR'," + " addressLine: 'ADDRESS LINE ERROR',"
+ " city: 'CITY ERROR'" + " city: 'CITY ERROR',"
+ " region: 'REGION ERROR',"
+ " postalCode: 'POSTAL CODE ERROR',"
+ " phone: 'PHONE ERROR'"
+ " }" + " }"
+ "}", + "}",
mPaymentRequestTestRule.getEditorValidationError()); mPaymentRequestTestRule.getEditorValidationError());
mPaymentRequestTestRule.clickInEditorAndWait(
R.id.editor_dialog_done_button, mPaymentRequestTestRule.getReadyToEdit());
mPaymentRequestTestRule.getKeyboardDelegate().hideKeyboard(
mPaymentRequestTestRule.getEditorDialogView());
RenderTestRule.sanitize(mPaymentRequestTestRule.getEditorDialogView());
mRenderTestRule.render(mPaymentRequestTestRule.getEditorDialogView(),
"retry_with_shipping_address_errors");
mPaymentRequestTestRule.setSpinnerSelectionInEditorAndWait(
0 /* Afghanistan */, mPaymentRequestTestRule.getReadyToEdit());
mPaymentRequestTestRule.setTextInEditorAndWait( mPaymentRequestTestRule.setTextInEditorAndWait(
new String[] {"Jane Doe", "Edge Corp.", "111 Wall St.", "New York", "NY"}, new String[] {
"Alice", "Supreme Court", "Airport Road", "Kabul", "1043", "020-253-0000"},
mPaymentRequestTestRule.getEditorTextUpdate()); mPaymentRequestTestRule.getEditorTextUpdate());
mPaymentRequestTestRule.clickInEditorAndWait( mPaymentRequestTestRule.clickInEditorAndWait(
R.id.editor_dialog_done_button, mPaymentRequestTestRule.getReadyToPay()); R.id.editor_dialog_done_button, mPaymentRequestTestRule.getReadyToPay());
...@@ -145,8 +167,8 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback { ...@@ -145,8 +167,8 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback {
*/ */
@Test @Test
@MediumTest @MediumTest
@Feature({"Payments"}) @Feature({"Payments", "RenderTest"})
public void testRetryWithPayerErrors() throws InterruptedException, TimeoutException { public void testRetryWithPayerErrors() throws Throwable {
mPaymentRequestTestRule.triggerUIAndWait(mPaymentRequestTestRule.getReadyForInput()); mPaymentRequestTestRule.triggerUIAndWait(mPaymentRequestTestRule.getReadyForInput());
mPaymentRequestTestRule.clickAndWait( mPaymentRequestTestRule.clickAndWait(
R.id.button_primary, mPaymentRequestTestRule.getReadyForUnmaskInput()); R.id.button_primary, mPaymentRequestTestRule.getReadyForUnmaskInput());
...@@ -165,9 +187,19 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback { ...@@ -165,9 +187,19 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback {
+ "}", + "}",
mPaymentRequestTestRule.getEditorValidationError()); mPaymentRequestTestRule.getEditorValidationError());
mPaymentRequestTestRule.clickInEditorAndWait(
R.id.editor_dialog_done_button, mPaymentRequestTestRule.getReadyToEdit());
mPaymentRequestTestRule.getKeyboardDelegate().hideKeyboard(
mPaymentRequestTestRule.getEditorDialogView());
RenderTestRule.sanitize(mPaymentRequestTestRule.getEditorDialogView());
mRenderTestRule.render(
mPaymentRequestTestRule.getEditorDialogView(), "retry_with_payer_errors");
mPaymentRequestTestRule.setTextInEditorAndWait( mPaymentRequestTestRule.setTextInEditorAndWait(
new String[] {"Jane Doe", "650-253-0000", "jon.doe@gmail.com"}, new String[] {"Jane Doe", "650-253-0000", "jane.doe@gmail.com"},
mPaymentRequestTestRule.getReadyToEdit()); mPaymentRequestTestRule.getEditorTextUpdate());
mPaymentRequestTestRule.clickInEditorAndWait( mPaymentRequestTestRule.clickInEditorAndWait(
R.id.editor_dialog_done_button, mPaymentRequestTestRule.getReadyToPay()); R.id.editor_dialog_done_button, mPaymentRequestTestRule.getReadyToPay());
} }
......
...@@ -751,6 +751,7 @@ public class PaymentRequestTestRule extends ChromeTabbedActivityTestRule ...@@ -751,6 +751,7 @@ public class PaymentRequestTestRule extends ChromeTabbedActivityTestRule
ThreadUtils.runOnUiThreadBlocking(() -> { ThreadUtils.runOnUiThreadBlocking(() -> {
List<EditText> fields = mUI.getEditorDialog().getEditableTextFieldsForTest(); List<EditText> fields = mUI.getEditorDialog().getEditableTextFieldsForTest();
for (int i = 0; i < values.length; i++) { for (int i = 0; i < values.length; i++) {
fields.get(i).requestFocus();
fields.get(i).setText(values[i]); fields.get(i).setText(values[i]);
} }
}); });
...@@ -920,6 +921,11 @@ public class PaymentRequestTestRule extends ChromeTabbedActivityTestRule ...@@ -920,6 +921,11 @@ public class PaymentRequestTestRule extends ChromeTabbedActivityTestRule
.findViewById(R.id.autofill_card_unmask_prompt)); .findViewById(R.id.autofill_card_unmask_prompt));
} }
/* package */ View getEditorDialogView() throws Throwable {
return ThreadUtils.runOnUiThreadBlocking(
() -> mUI.getEditorDialog().findViewById(R.id.editor_container));
}
/** Allows to skip UI into paymenthandler for"basic-card". */ /** Allows to skip UI into paymenthandler for"basic-card". */
protected void enableSkipUIForBasicCard() { protected void enableSkipUIForBasicCard() {
ThreadUtils.runOnUiThreadBlocking( ThreadUtils.runOnUiThreadBlocking(
......
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