Closed Bug 1429195 Opened 6 years ago Closed 6 years ago

Implement the PaymentRequest payment card selector with the security code input

Categories

(Firefox :: WebPayments UI, enhancement, P1)

Product:
Firefox
Component:
WebPayments UI
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 60
Tracking Flags:
Tracking Status
firefox60 --- fixed

People

(Reporter: MattN, Assigned: MattN)

References

Details

(Whiteboard: [webpayments])

Attachments

(4 files)

Bug 1429195 - Add debugging buttons to modify credit card state for payment request.
59 bytes, text/x-review-board-request
jaws
: review+
Details
Bug 1429195 - Rename basic-card-option attribute names to match autofill storage.
59 bytes, text/x-review-board-request
jaws
: review+
Details
Bug 1429195 - Implement and use a <payment-method-picker> custom element.
59 bytes, text/x-review-board-request
jaws
: review+
Details
Bug 1429195 - Send the selected payment card to the wrapper and DOM.
59 bytes, text/x-review-board-request
jaws
: review+
Details 
Using the rich selector UI, display a list of payment cards including a CVV/CCV/security code <input>.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Priority: P3 → P1
Assignee: jaws → nobody
Status: ASSIGNED → NEW
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Comment on attachment 8948861 [details]
Bug 1429195 - Add debugging buttons to modify credit card state for payment request.

https://reviewboard.mozilla.org/r/218250/#review224176
Attachment #8948861 - Flags: review?(jaws) → review+
Comment on attachment 8948862 [details]
Bug 1429195 - Rename basic-card-option attribute names to match autofill storage.

https://reviewboard.mozilla.org/r/218252/#review224178
Attachment #8948862 - Flags: review?(jaws) → review+
Comment on attachment 8948863 [details]
Bug 1429195 - Implement and use a <payment-method-picker> custom element.

https://reviewboard.mozilla.org/r/218254/#review224180

::: toolkit/components/payments/res/containers/payment-method-picker.js:85
(Diff revision 3)
> +    switch (target) {
> +      case this.dropdown: {
> +        stateChange[selectedKey] = target.selectedOption && target.selectedOption.guid;

All of the code in onChange relies on selectedKey being valid, so we should have an early return if it's not instead of doing the check for it at line 102.

::: toolkit/components/payments/test/mochitest/payments_common.js:18
(Diff revision 3)
> +
> +function promiseStateChange(store) {
> +  return new Promise(resolve => {
> +    store.subscribe({
> +      stateChangeCallback(state) {
> +        resolve(state);

We should unsubscribe in the stateChangeCallback.

::: toolkit/components/payments/test/mochitest/test_payment_method_picker.html:123
(Diff revision 3)
> +  is(selectedPaymentCard, null, "store should have no option selected");
> +  is(selectedPaymentCardSecurityCode, null, "store should have no security code");
> +
> +  await SimpleTest.promiseFocus();
> +  let codeFocusPromise = new Promise(resolve => {
> +    picker1.securityCodeInput.addEventListener("focus", resolve);

Might as well add {once: true} here.
Attachment #8948863 - Flags: review?(jaws) → review+
Comment on attachment 8948864 [details]
Bug 1429195 - Send the selected payment card to the wrapper and DOM.

https://reviewboard.mozilla.org/r/218256/#review224528

::: toolkit/components/payments/content/paymentDialogWrapper.js:101
(Diff revision 3)
> +
> +    let methodData = this.createBasicCardResponseData({
> +      cardholderName: cardData["cc-name"],
> +      cardNumber,
> +      expiryMonth: cardData["cc-exp-month"].toString().padStart(2, "0"),
> +      expiryYear: cardData["cc-exp-year"].toString(),

This should have padStart(2, "0") also, right? In case some user enters in a credit card expiry year of 0-9 (2000-2009)? Not really useful since the card would be expired, but correct nonetheless.

::: toolkit/components/payments/test/PaymentTestUtils.jsm:77
(Diff revision 3)
>      completePayment: () => {
>        content.document.getElementById("pay").click();
>      },
> +
> +    setSecurityCode: ({securityCode}) => {
> +      let securityCodeField = content.document.querySelector("payment-method-picker input");

Could this selector point to the wrong element when we implement add/edit? We should put a class on the `input` to fix this.
Attachment #8948864 - Flags: review?(jaws) → review+
Comment on attachment 8948864 [details]
Bug 1429195 - Send the selected payment card to the wrapper and DOM.

https://reviewboard.mozilla.org/r/218256/#review224528

> This should have padStart(2, "0") also, right? In case some user enters in a credit card expiry year of 0-9 (2000-2009)? Not really useful since the card would be expired, but correct nonetheless.

Autofill storage already handles converting to four-digit years (assuming you meant `4`): https://dxr.mozilla.org/mozilla-central/rev/0ac953fcddf10132eaecdb753d72b2ba5a43c32a/browser/extensions/formautofill/FormAutofillStorage.jsm#64-65

> Could this selector point to the wrong element when we implement add/edit? We should put a class on the `input` to fix this.

I thought about this and think it's unlikely we'll use an input but also wrote this before I realize I can use the securityCodeInput property so I'll use that.
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/985cd81569ff
Add debugging buttons to modify credit card state for payment request. r=jaws
https://hg.mozilla.org/integration/autoland/rev/07bac14a767b
Rename basic-card-option attribute names to match autofill storage. r=jaws
https://hg.mozilla.org/integration/autoland/rev/5c40672bc6de
Implement and use a <payment-method-picker> custom element. r=jaws
https://hg.mozilla.org/integration/autoland/rev/61eeb80f413c
Send the selected payment card to the wrapper and DOM. r=jaws
Whiteboard: [webpayments]
Product: Toolkit → Firefox
Target Milestone: mozilla60 → Firefox 60
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: