Closed Bug 1361237 Opened 7 years ago Closed 7 years ago

[Form Autofill] Implement offline heuristics to determine Credit Card related fields

Categories

(Toolkit :: Form Manager, enhancement, P1)

Product:
Toolkit
Component:
Form Manager
Platform:
x86
macOS
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: selee, Assigned: selee)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

(Whiteboard: [form autofill:M4] )

Attachments

(2 files, 1 obsolete file)

Bug 1361237 - Part 1: Use a set of regexp to recognize Credit Card autofill types.;
59 bytes, text/x-review-board-request
MattN
: review+
Details
Bug 1361237 - Part 2: Fix all Credit Card related test cases.;
59 bytes, text/x-review-board-request
MattN
: review+
Details 
Implement the offline heuristics to determine the following Credit Card fields:
* cc-name
* cc-number
* cc-exp
* cc-exp-month
* cc-exp-year
Blocks: 1333351
No longer blocks: 1333350
Depends on: 1368858
Whiteboard: [form autofill:M3] → [form autofill:M3] ETA:612
Whiteboard: [form autofill:M3] ETA:612 → [form autofill:M3]
Blocks: 1333350
Whiteboard: [form autofill:M3] → [form autofill:M4]
Comment on attachment 8863584 [details]
Bug 1361237 - Part 1: Use a set of regexp to recognize Credit Card autofill types.;

https://reviewboard.mozilla.org/r/135346/#review168592

I think we should match Chromium's logic/algorithm which I believe we discussed at the All Hands.
Attachment #8863584 - Flags: review?(MattN+bmo)
Comment on attachment 8874664 [details]
Bug 1361237 - Part 2: Fix all Credit Card related test cases.;

https://reviewboard.mozilla.org/r/146014/#review168596

I dont' understand what all the FIXME are for on uncommented fields.

::: browser/extensions/formautofill/test/unit/heuristics/third_party/test_CDW.js:38
(Diff revision 1)
> -      [
> +//      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-number"}, // ac-off
> - /* TODO: Credit Card
> -        {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-type"},
> -        {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-number"},
>          {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-month"},
> -        {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-year"},
> +//      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-year"}, // FIXME

This should probably be addressed or the comment should be improved ideally pointing to a bug

::: browser/extensions/formautofill/test/unit/heuristics/third_party/test_CostCo.js:52
(Diff revision 1)
>        [
> -/* TODO: credit card
> -        {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-type"},
> +//      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-type"},
> +//      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-number"}, // ac-off
> -        {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-number"}, // ac-off
>          {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-month"},
> -        {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-year"},
> +//      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-year"}, // FIXME

This also doesn't look good. Can you explain the problem

::: browser/extensions/formautofill/test/unit/heuristics/third_party/test_HomeDepot.js:19
(Diff revision 1)
>          {"section": "", "addressType": "", "contactType": "", "fieldName": "address-line1"},
>          {"section": "", "addressType": "", "contactType": "", "fieldName": "postal-code"},
>          {"section": "", "addressType": "billing", "contactType": "", "fieldName": "street-address"}, // <select>
>          {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-month"},
>          {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-year"},
> -//      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-number"},
> +        {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-number"}, // FIXME

Ditto
Attachment #8874664 - Flags: review?(MattN+bmo)
Comment on attachment 8874663 [details]
Bug 1361237 - Part 2: Separate the filter criteria of Credit Card and Address fields.;

https://reviewboard.mozilla.org/r/146012/#review168598

Please rebase this.
Attachment #8874663 - Flags: review?(MattN+bmo)
Attachment #8874663 - Attachment is obsolete: true
Comment on attachment 8874664 [details]
Bug 1361237 - Part 2: Fix all Credit Card related test cases.;

https://reviewboard.mozilla.org/r/146014/#review168596

all FIXME parts have been refined.

> This also doesn't look good. Can you explain the problem

The comment is fixed.
cc-exp-month works fine, but cc-exp-year can not be recognized by the regexp directly. I am thinking if I should implement a parser to fix this.
Blocks: 1392528
Comment on attachment 8863584 [details]
Bug 1361237 - Part 1: Use a set of regexp to recognize Credit Card autofill types.;

https://reviewboard.mozilla.org/r/135346/#review176282

rs=me assuming these came straight from Chromium
Attachment #8863584 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8874664 [details]
Bug 1361237 - Part 2: Fix all Credit Card related test cases.;

https://reviewboard.mozilla.org/r/146014/#review176288

r=me if each class of fixme issue gets it's own bug filed and the bug number is in the test comment. We shouldn't be leaving fixme/todo in code anymore without bug numbers now that we're passed laying the foundation. Each of the issues affecting the 12 target sites should be considered for the MVP since they're the goal of the project.
Attachment #8874664 - Flags: review?(MattN+bmo) → review+
Hey MattN, thanks for the review! Each FIXME comment is with a follow-up bug number. I marked all of them as MVP, and it can be adjusted later.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f452e201cbd9
Part 1: Use a set of regexp to recognize Credit Card autofill types.; r=MattN
https://hg.mozilla.org/integration/autoland/rev/e6c68af17717
Part 2: Fix all Credit Card related test cases.; r=MattN
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f452e201cbd9
https://hg.mozilla.org/mozilla-central/rev/e6c68af17717
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: