1480023 - The “Save” button is grayed out after editing the CC-number back to the valid form

Status: VERIFIED FIXED

(Firefox :: WebPayments UI, defect, P1)

Description

[Timea Cernea [:tbabos][inactive]]

Attachment: Video of the issue

[Affected versions]:
Nightly 63.0a1

[Affected platforms]:
Windows 10 x64

[Prerequisites]:
- set the pref dom.payments.request.enabled to "true"
- have a profile with saved cards

[Steps to reproduce]:
1. Go to “https://rsolomakhin.github.io/pr/single/” and click on “Buy” button
2. Select a shipping address
3. Click on “Add” for a new Payment method
4. Type in a name and select an expiration date
5. Type in the cc-number field a valid number
6. Click out of the active field
7. Edit the cc-number to make it invalid: type in additional numbers
8. Click out of the active field
9. Click on the “Save” button to check if it is functional (it is not grayed out)
10. Edit the cc-number to the valid form

[Expected Result]:
The “Save” button should be functional and enabled since all the data is correct. 

[Actual Result]:
The “Save” button is grayed out even if the CC-number is valid after editing it. 

[Notes]:
Any further edits (add/ delete digits) of the cc-number will not make the “Save” button functional. The only workaround is to delete the entire cc-number and copy/paste it back.

Comment 1

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1480023 - Ensure input event is handled in the correct order by waiting for EditAutofillForm constructor before adding BasicCardForm's input & change handlers.

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

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 8997253 [details]
Bug 1480023 - Ensure input event is handled in the correct order by waiting for EditAutofillForm constructor before adding BasicCardForm's input & change handlers.

https://reviewboard.mozilla.org/r/261118/#review268266

::: browser/components/payments/res/containers/basic-card-form.js:89
(Diff revision 1)
> +      // By adding event listeners after the EditCreditCard constructor,
> +      // validity will be updated before our handlers get the event
> +      form.addEventListener("input", this);
> +      form.addEventListener("invalid", this);

I'll admit I still don't fully understand what the problem was… could you explain a little more what was happening wrong?

Comment 3

[Sam Foster [:sfoster] (he/him)]

BasicCardForm's input handler is what calls updateSaveButtonState. It checks the validity of the cc-number and cc-name fields. 
The actual validation is done in EditAutofillForm though. On each input event, it will pass the cc-number value into isCCNumber(). When it determines it is valid, it marks it as valid by setCustomValidity(""). 

The BasicCardForm's input handler is registered when the form is first injected. The EditAutofillForm adds its event listeners during its constructor, which is called later. So the BasicCardForm event handlers get the event first. At this point, the field is still marked invalid with the invalidCardNumberString, so form.checkValidity() check returns false and the save button remains disabled. 

Ensuring the BasicCardForm's event listeners are added *after* those of the EditAutofillForm means the cc-number value has already been validated and the invalid string cleared when BasicCardForm gets the event and checks form.checkValidity() - which is what we want.

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8997253 [details]
Bug 1480023 - Ensure input event is handled in the correct order by waiting for EditAutofillForm constructor before adding BasicCardForm's input & change handlers.

https://reviewboard.mozilla.org/r/261118/#review268292

::: browser/components/payments/res/containers/basic-card-form.js:89
(Diff revision 1)
> +      // By adding event listeners after the EditCreditCard constructor,
> +      // validity will be updated before our handlers get the event
> +      form.addEventListener("input", this);
> +      form.addEventListener("invalid", this);

Ah, okay, that makes sense now, I didn't realize both files were using "input" event listeners on the same element. The order of the event listeners getting called is defined in DOM L3 so it's fine to rely on that. Could you change the comment to more specifically talk about the order of the event listeners rather than talking about just the "EditCreditCard constructor".

Comment 5

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8997253 [details]
Bug 1480023 - Ensure input event is handled in the correct order by waiting for EditAutofillForm constructor before adding BasicCardForm's input & change handlers.

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b075d5d61bda
Ensure input event is handled in the correct order by waiting for EditAutofillForm constructor before adding BasicCardForm's input & change handlers. r=MattN

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/b075d5d61bda

Comment 8

[Timea Cernea [:tbabos][inactive]]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0

Verified - Fixed on the latest Nightly 63.0a1 (2018-08-06) on Windows 10 x64, Ubuntu 16.04, Mac OS 10.13, Windows 7 x64.
The “Save” button is functional after editing the CC-number back to the valid form.