Closed
Bug 1390757
Opened 7 years ago
Closed 7 years ago
[Form Autofill] Enable credit card feature according to "extensions.formautofill.creditCards.enabled" pref
Categories
(Toolkit :: Form Manager, enhancement, P3)
Toolkit
Form Manager
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: ralin, Assigned: ralin)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [form autofill:MVP])
Attachments
(1 file)
FAP should respect this pref and adjust its support for credit card fields.
Assignee | ||
Updated•7 years ago
|
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
I was trying to figure out how the initialization could differ from the original if we have to consider two pref now. Do we need to update autofillSavedFieldNames as well? If the autofillSavedFieldNames size has to account for the activeStatus, autofillSavedFieldNames should be collected from the enabled collections instead of all. e.g. when only "extensions.formautofill.creditCards.enabled" is on with only address records in storage, we shouldn't see autofill active.
Comment 3•7 years ago
|
||
Yes, it makes sense. "autofillSavedFieldNames" is only updated when profile is changed. Maybe we can record the saved field names of addresses and credit cards separately so we don't need to update it when only pref is changed. BTW, are you going to update "startSearch" in the same commit or in a new one?
Assignee | ||
Comment 4•7 years ago
|
||
> Maybe we can record the saved field names of addresses and credit cards separately so we don't need to update it when only pref is changed.
Seems like a small change, I'll do in the same commit including "startSearch" part later. Thanks.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
Some TODOs: 1. Don't save autofillSavedFieldNames as a complex object, instead, separate into two properties of initialProcessData like autofillSavedAddressesFieldNames and autofillSavedCreditCardsFieldNames, it could have been more clearer. (will discuss with Luke in person) 2. Add test cases in test_activeStatus.js for different pref sets. 3. Update test_savedFieldNames.js due to the change of its structure. 4. Smarter fallback to form history according to pref. (maybe an extra mochitest is required for this)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8900217 [details] Bug 1390757 - Introduce pref extensions.formautofill.creditCards.enabled to form autofill activation process. https://reviewboard.mozilla.org/r/171582/#review177326 Good job! Thanks.
Attachment #8900217 -
Flags: review?(lchang) → review+
Comment 10•7 years ago
|
||
(In reply to Ray Lin[:ralin] from comment #6) > 1. Don't save autofillSavedFieldNames as a complex object, instead, separate > into two properties of initialProcessData like > autofillSavedAddressesFieldNames and autofillSavedCreditCardsFieldNames, it > could have been more clearer. (will discuss with Luke in person) Let's file a bug for following up this improvement.
Assignee | ||
Comment 11•7 years ago
|
||
Thanks for your rapid review :D I'm waiting for the try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8fe1c4f3bc9356c0582af1a67c93b26ada584e5 will mark checkin-needed once I can confirm the result is good to go. (In reply to Luke Chang [:lchang] from comment #10) > (In reply to Ray Lin[:ralin] from comment #6) > > 1. Don't save autofillSavedFieldNames as a complex object, instead, separate > > into two properties of initialProcessData like > > autofillSavedAddressesFieldNames and autofillSavedCreditCardsFieldNames, it > > could have been more clearer. (will discuss with Luke in person) > > Let's file a bug for following up this improvement. I'll file a bug for that.
Comment 13•7 years ago
|
||
Pushed by lchang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8fa30f1a3f4d Introduce pref extensions.formautofill.creditCards.enabled to form autofill activation process. r=lchang
Keywords: checkin-needed
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8fa30f1a3f4d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Added to Fx58 release notes.
relnote-firefox:
--- → 58+
Comment 16•6 years ago
|
||
This bug added the ability to control the feature being enabled or not but doesn't control making it available to users. We're not shipping credit card autofill in 58 because we want to record some telemetry first.
Flags: needinfo?(rkothari)
Hi Matt, this relnote was added only to the Beta58 release notes and not the 58 "release version" notes. Please let me know if these to be taken off of the beta58 release notes as well. Thanks!
Flags: needinfo?(rkothari)
You need to log in
before you can comment on or make changes to this bug.
Description
•