Need an asterisk for in-field label on credit card number and CVV field.

VERIFIED FIXED in Firefox 63

Status

()

enhancement
P1
normal
VERIFIED FIXED
11 months ago
10 months ago

People

(Reporter: jgong, Assigned: jaws)

Tracking

unspecified
Firefox 63
Points:
---

Firefox Tracking Flags

(firefox63 verified)

Details

(Whiteboard: [webpayments] [user-testing])

Attachments

(1 attachment)

Reporter

Description

11 months ago
Need an asterisk for in-field label on credit card number and CVV field.
Whiteboard: [webpayments][triage] → [webpayments] [triage] [user-testing]

Updated

11 months ago
Whiteboard: [webpayments] [triage] [user-testing] → [webpayments] [user-testing]
Flags: qe-verify+
QA Contact: hani.yacoub
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Priority: P2 → P1
This bug says to add an asterisk to the CVV placeholder, which I did, but some debit cards such as Maestro don't have a CVV. Are we going to just style this as required but not enforce it? Or should we put the asterisk on there conditionally after we can determine which type of card it is?
Flags: needinfo?(MattN+bmo)

Comment 3

11 months ago
mozreview-review
Comment on attachment 8996112 [details]
Bug 1477114 - Add an asterisk to the required fields of the credit card form as well as the CVV placeholder.

https://reviewboard.mozilla.org/r/260362/#review267902

r=me with the test and refactoring suggestions. I guess the form-autofill.css issue is more a question than an issue.

::: browser/components/payments/res/containers/autofill-form.css:1
(Diff revision 1)
> +/* This Source Code Form is subject to the terms of the Mozilla Public

I can see that it makes sense to move this shared code into a new file for both card and address forms - and the CVV field on the summary page. But why "autofill-form.css" rather than just "form.css" or "form-common.cs" or something?

::: browser/components/payments/res/containers/basic-card-form.js:176
(Diff revision 1)
>        } else {
>          billingAddressSelect.value = Object.keys(addresses)[0];
>        }
>      }
>  
> +    for (let formElement of this.form.elements) {

We may want to toggle required-ness on the fly without having to re-`render()`, so could you move this to its own (internal) method and call that from here?

::: browser/components/payments/res/containers/basic-card-form.js:176
(Diff revision 1)
>        } else {
>          billingAddressSelect.value = Object.keys(addresses)[0];
>        }
>      }
>  
> +    for (let formElement of this.form.elements) {

Can you add a test to test_basic_card_form.html to verify that toggling a required attribute on say the expiry month and year do indeed toggle that attribute on the nearest label?
Attachment #8996112 - Flags: review+
Attachment #8996112 - Flags: review?(MattN+bmo)
Assignee

Comment 4

11 months ago
mozreview-review-reply
Comment on attachment 8996112 [details]
Bug 1477114 - Add an asterisk to the required fields of the credit card form as well as the CVV placeholder.

https://reviewboard.mozilla.org/r/260362/#review267902

> I can see that it makes sense to move this shared code into a new file for both card and address forms - and the CVV field on the summary page. But why "autofill-form.css" rather than just "form.css" or "form-common.cs" or something?

Naming is hard ;) I switched to form.css.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 7

11 months ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d402c7b803c6
Add an asterisk to the required fields of the credit card form as well as the CVV placeholder. r=sfoster
Comment hidden (mozreview-request)

Comment 10

11 months ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8b2d475dff2c
Add an asterisk to the required fields of the credit card form as well as the CVV placeholder. r=sfoster
Flags: needinfo?(jaws)
After our decision today there should be an asterisk on all credit card fields like the UI spec had at one point. We won't support cards not associated with a credit card network.
Flags: needinfo?(MattN+bmo)

Comment 12

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8b2d475dff2c
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63

Comment 13

11 months ago
Hi Matthew,

Based on the Invision doc (https://mozilla.invisionapp.com/share/SDFY4PA4EQ7#/screens/310857527), there is an asterisk for the month and year of expiration date as well. 
Since there is a bug submitted for the expiration date (to be required or not) I can close this issue as verified - fixed given that the CC-number, Name and CVV code fields have an asterisk. 

Does it sound good on your side or a different approach is needed?
Flags: needinfo?(MattN+bmo)
Yes, the asterisk should get added in bug 1480719
Flags: needinfo?(MattN+bmo)

Comment 15

10 months ago
Thanks for the response!
Marking this issue as Verified - Fixed based on Comment 13 and Comment 14.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.