Implement the PaymentRequest payment card selector with the security code input

RESOLVED FIXED in Firefox 60

Status

()

enhancement
P1
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: MattN, Assigned: MattN)

Tracking

Trunk
Firefox 60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

(Whiteboard: [webpayments])

Attachments

(4 attachments)

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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
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+
Assignee

Comment 14

a year ago
mozreview-review-reply
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.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 19

a year ago
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]
Component: WebPayments UI → WebPayments UI
Product: Toolkit → Firefox
Target Milestone: mozilla60 → Firefox 60
You need to log in before you can comment on or make changes to this bug.