1429195 - Implement the PaymentRequest payment card selector with the security code input

Status: RESOLVED FIXED

(Firefox :: WebPayments UI, enhancement, P1)

Description

[Matthew N. [:MattN]]

Using the rich selector UI, display a list of payment cards including a CVV/CCV/security code <input>.

Comment 1

[Matthew N. [:MattN]]

Attachment: Bug 1429195 - Add debugging buttons to modify credit card state for payment request.

Review commit: https://reviewboard.mozilla.org/r/218250/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218250/

Comment 2

[Matthew N. [:MattN]]

Attachment: Bug 1429195 - Rename basic-card-option attribute names to match autofill storage.

Review commit: https://reviewboard.mozilla.org/r/218252/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218252/

Comment 3

[Matthew N. [:MattN]]

Attachment: Bug 1429195 - Implement and use a <payment-method-picker> custom element.

Review commit: https://reviewboard.mozilla.org/r/218254/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218254/

Comment 4

[Matthew N. [:MattN]]

Attachment: Bug 1429195 - Send the selected payment card to the wrapper and DOM.

Review commit: https://reviewboard.mozilla.org/r/218256/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218256/

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8948862 [details]
Bug 1429195 - Rename basic-card-option attribute names to match autofill storage.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218252/diff/1-2/

Comment 6

[Matthew N. [:MattN]]

Comment on attachment 8948863 [details]
Bug 1429195 - Implement and use a <payment-method-picker> custom element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218254/diff/1-2/

Comment 7

[Matthew N. [:MattN]]

Comment on attachment 8948864 [details]
Bug 1429195 - Send the selected payment card to the wrapper and DOM.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218256/diff/1-2/

Comment 8

[Matthew N. [:MattN]]

Comment on attachment 8948863 [details]
Bug 1429195 - Implement and use a <payment-method-picker> custom element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218254/diff/2-3/

Comment 9

[Matthew N. [:MattN]]

Comment on attachment 8948864 [details]
Bug 1429195 - Send the selected payment card to the wrapper and DOM.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218256/diff/2-3/

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

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

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8948862 [details]
Bug 1429195 - Rename basic-card-option attribute names to match autofill storage.

https://reviewboard.mozilla.org/r/218252/#review224178

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 14

[Matthew N. [:MattN]]

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 15

[Matthew N. [:MattN]]

Comment on attachment 8948861 [details]
Bug 1429195 - Add debugging buttons to modify credit card state for payment request.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218250/diff/1-2/

Comment 16

[Matthew N. [:MattN]]

Comment on attachment 8948862 [details]
Bug 1429195 - Rename basic-card-option attribute names to match autofill storage.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218252/diff/2-3/

Comment 17

[Matthew N. [:MattN]]

Comment on attachment 8948863 [details]
Bug 1429195 - Implement and use a <payment-method-picker> custom element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218254/diff/3-4/

Comment 18

[Matthew N. [:MattN]]

Comment on attachment 8948864 [details]
Bug 1429195 - Send the selected payment card to the wrapper and DOM.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218256/diff/3-4/

Comment 19

[Pulsebot]

Pushed by [e-mail]:
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

Comment 20

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/985cd81569ff
https://hg.mozilla.org/mozilla-central/rev/07bac14a767b
https://hg.mozilla.org/mozilla-central/rev/5c40672bc6de
https://hg.mozilla.org/mozilla-central/rev/61eeb80f413c