1411190 - [Form Autofill] Credit card is not saved for a few top shopping sites

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect)

Description

[Gabi Cheta [:Gabi] Desktop QA]

[Environment:]
Windows 10x64, Ubuntu 14.04, Mac Os 10.12

Nightly 58.0a1 Build ID: 20171023220222

[Description:]
Credit Card added into checkout screen is not saved for a few top shopping sites


[Steps:]


1. Open Firefox
2. Navigate to amazon.com or groupon.com checkout page
3. Go to the payment information window
4. Add a new credit card
5. Click to save credit card 
6. When credit card door hanger is displayed click Save Credit Card option
7. Go to Options/Privacy and Security 
8. Check that the credit card is saved under Saved Credit Cards



[Actual Result:]
Credit Card is not saved


[Expected Result:]
The added credit card should be saved under Saved Credit Cards once the Save Creidt Card doorhanger option is clicked

[Note:]
Regression: changeset: 615798b405f4994de32c6a9b0a464601accb0b0e
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d72d02a5f8ac69198e85fe88e0acd7cca9b8c994&tochange=615798b405f4994de32c6a9b0a464601accb0b0e

Comment 1

[Ray Lin[:ralin]]

Take this, looks like we got worse detection on these websites after specifically doing extra parsing for cc-exp-*.

Comment 2

[Ray Lin[:ralin]]

In Amazon, I found that among our parsing fields, a <button> is in the middle of <select> like: 
"<SELECT#ccMonth.a-native-dropdown.native-dropdown>","<BUTTON.a-button-text.a-declarative>","<SELECT#ccYear.a-native-dropdown.native-dropdown>"
that makes the first expiration date parsing logic[0] fails to predict, since two <select>s aren't consecutive.

I can fix the issue by passing eligible fields instead of form.elements at [1] which filters out the noises like <button> in this case. IMHO, we never make use of and just return null for the ineligible fields in getInfo()[2], it seems unreasonable that we have to pass entire unfiltered form.elements to FieldScanner. I'll confirm that with Sean tomorrow. Thanks.


[0]: http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/browser/extensions/formautofill/FormAutofillHeuristics.jsm#493-498

[1]: http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/browser/extensions/formautofill/FormAutofillHeuristics.jsm#546

[2]: http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/browser/extensions/formautofill/FormAutofillHeuristics.jsm#569-571

Comment 3

[Ray Lin[:ralin]]

Attachment: Bug 1411190 - Collect and parse only eligible fields in form autofill heuristics.

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

Comment 4

[Ray Lin[:ralin]]

Comment on attachment 8921708 [details]
Bug 1411190 - Collect and parse only eligible fields in form autofill heuristics.

Hey Sean,


Could you have a look at this patch and give me some feedback? Thanks!

Comment 5

[Ray Lin[:ralin]]

Comment on attachment 8921708 [details]
Bug 1411190 - Collect and parse only eligible fields in form autofill heuristics.

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

Comment 6

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8921708 [details]
Bug 1411190 - Collect and parse only eligible fields in form autofill heuristics.

https://reviewboard.mozilla.org/r/192724/#review198508

LGTM, Thanks.

Once we found that the noneligible fields would be helpful for parsers to detemine fields or the order of form.elements and fieldDetails is important, we probably need to modify the parser to check the noneligible fields.

Comment 7

[Ray Lin[:ralin]]

Comment on attachment 8921708 [details]
Bug 1411190 - Collect and parse only eligible fields in form autofill heuristics.

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

Comment 8

[Ray Lin[:ralin]]

(In reply to Sean Lee [:seanlee][:weilonge] from comment #6)
> Once we found that the noneligible fields would be helpful for parsers to
> detemine fields or the order of form.elements and fieldDetails is important,
> we probably need to modify the parser to check the noneligible fields.
Second this, we'll see whether ineligible fields help and decide to march on the form.elements or not then. Thanks.

Comment 9

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8921708 [details]
Bug 1411190 - Collect and parse only eligible fields in form autofill heuristics.

https://reviewboard.mozilla.org/r/192724/#review198518

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a236894d38b3
Collect and parse only eligible fields in form autofill heuristics. r=lchang,seanlee

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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