Closed Bug 1406771 Opened 7 years ago Closed 7 years ago

Use placeholder text as an indicator for form autofill format

Categories

(Toolkit :: Form Manager, defect, P3)

57 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: jkt, Assigned: selee)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [form autofill:MVP])

Attachments

(1 file)

When using a shopify website I was presented with the following form:


<form>
      
        <label for="number" class="visually-hidden" aria-hidden="true">Credit Card Number</label>

        
          <input autocomplete="cc-number" id="number" name="number" class="visually-hidden" aria-hidden="true" data-honeypot-field="" tabindex="-1" type="tel">
        
      
...
        
      
        <label for="expiry" class="visually-hidden" aria-hidden="true">MM / YY</label>

        
          <input autocomplete="cc-exp" id="expiry" name="expiry" aria-describedby="error-for-expiry" data-current-field="expiry" class="input-placeholder-color--lvl-34" style="color: rgb(51, 51, 51); font-family: Roboto, sans-serif; padding: 0.94em 0.8em; transition: padding 0.2s ease-out 0s;" placeholder="MM / YY" type="tel">
          <span id="error-for-expiry" class="input-error-message visually-hidden"></span>
        
      
        <label for="verification_value" class="visually-hidden" aria-hidden="true">CVV</label>

        
...
    </form>

I clicked autofill in the CC number input and the expiry filled in with the wrong format: yyyy-mm instead of mm / yy
Blocks: 1020819
No longer blocks: 1392938
Whiteboard: [form autofill:MVP]
Hi Jonathan,

I found the same issue at [1] which is using Shopify as well. May I know the URL of the website that you tried? This is useful to verify the issue by myself. Thanks.

I also found [1] will correct the cc-exp value when I click on the cc-exp field to show the popup, but the cc-exp value won't be fixed when the popup shows on other fields (e.g. cc-number). I guess this is related to the `change` or `blur` event for the web site to correct the value. (See bug 1395928)

[1] https://www.deathwishcoffee.com
Assignee: nobody → selee
Blocks: 1392938
Status: NEW → ASSIGNED
Sean, I responded with the URL in Slack, I would prefer to keep this off Bugzilla as I rarely pay with CC online. Not that security by obscurity is a good tactic. Thanks for the quick patch.
Comment on attachment 8916558 [details]
Bug 1406771 - Use input's placeholder to adjust the format of cc-exp.

https://reviewboard.mozilla.org/r/187706/#review195292

::: browser/extensions/formautofill/FormAutofillHandler.jsm:292
(Diff revision 2)
> +    if (/mm\s*\/\s*yyyy/gi.test(element.placeholder)) {
> +      profile["cc-exp"] = String(profile["cc-exp-month"]).padStart(2, "0") + "\/" +
> +        profile["cc-exp-year"];
> +    } else if (/mm\s*\/\s*yy/gi.test(element.placeholder)) {
> +      profile["cc-exp"] = String(profile["cc-exp-month"]).padStart(2, "0") + "\/" +
> +        String(profile["cc-exp-year"]).substr(-2);
> +    }

"g" is unnecessary. BTW, I think we should handle more cases such as "mmyy", "yyyy/mm", etc if it's not too complicated. What I'm thinking of is something like:

```js
let result;

result = /(?:[^m]|\b)(m{1,2})([^my]*)(y{2,4})(?!y)/i.exec(placeholder);
if (result) {
  ccExp = String(ccExpMonth).padStart(result[1].length, "0") +
          result[2].trim() +
          String(ccExpYear).substr(-1 * result[3].length);
  return;
}
  
result = /(?:[^y]|\b)(y{2,4})([^my]*)(m{1,2})(?!m)/i.exec(placeholder);
if (result) {
  ccExp = String(ccExpYear).substr(-1 * result[1].length) +
          result[2].trim() +
          String(ccExpMonth).padStart(result[3].length, "0");
  return;
}
```

What do you think?
Attachment #8916558 - Flags: review?(lchang)
Please remember to assign a priority if a bug is being worked on.
Priority: -- → P3
Comment on attachment 8916558 [details]
Bug 1406771 - Use input's placeholder to adjust the format of cc-exp.

https://reviewboard.mozilla.org/r/187706/#review195292

> "g" is unnecessary. BTW, I think we should handle more cases such as "mmyy", "yyyy/mm", etc if it's not too complicated. What I'm thinking of is something like:
> 
> ```js
> let result;
> 
> result = /(?:[^m]|\b)(m{1,2})([^my]*)(y{2,4})(?!y)/i.exec(placeholder);
> if (result) {
>   ccExp = String(ccExpMonth).padStart(result[1].length, "0") +
>           result[2].trim() +
>           String(ccExpYear).substr(-1 * result[3].length);
>   return;
> }
>   
> result = /(?:[^y]|\b)(y{2,4})([^my]*)(m{1,2})(?!m)/i.exec(placeholder);
> if (result) {
>   ccExp = String(ccExpYear).substr(-1 * result[1].length) +
>           result[2].trim() +
>           String(ccExpMonth).padStart(result[3].length, "0");
>   return;
> }
> ```
> 
> What do you think?

Thanks for addressing this. I tweak it a little because the second argument (result[2]) seems loose. Do you agree with this change?
([^my]*)   ==>>   \s*([-/\\]*)\s
Comment on attachment 8916558 [details]
Bug 1406771 - Use input's placeholder to adjust the format of cc-exp.

https://reviewboard.mozilla.org/r/187706/#review195292

> Thanks for addressing this. I tweak it a little because the second argument (result[2]) seems loose. Do you agree with this change?
> ([^my]*)   ==>>   \s*([-/\\]*)\s

It makes sense. Thanks.
Comment on attachment 8916558 [details]
Bug 1406771 - Use input's placeholder to adjust the format of cc-exp.

https://reviewboard.mozilla.org/r/187706/#review196162
Attachment #8916558 - Flags: review?(lchang) → review+
Thanks for the review!

Now we can handle the following cases:
mmyyyy
mmyy
mm-yy
mm-yyyy
yyyymm
yymm
yy-mm
yyyy-mm

"-" can be replaced with "/" as well.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f8843cf82310
Use input's placeholder to adjust the format of cc-exp. r=lchang
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f8843cf82310
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: