[Form Autofill] Features on submission are not disabled even the enabled prefs are set to false

VERIFIED FIXED in Firefox 56

Status

()

defect
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: aklotz, Assigned: lchang)

Tracking

unspecified
mozilla58
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox56+ fixed, firefox57 verified, firefox58 verified)

Details

(Whiteboard: [form autofill:MVP])

Attachments

(2 attachments)

Reporter

Description

2 years ago
STR:

1. Go to a website with a credit card form.
2. Fill in details, but leave one field invalid so that the purchase is rejected after submitting.
3. Submit Form.
4. When doorhanger drops down asking whether to remember your credit card number, click the "Never" button.
5. When the form fails and prompts you to correct the missing details, click submit again.

Expected: No more prompting about remembering your credit card.
Actual: Firefox again asks whether it should remember your credit card.

I most recently experienced this while filling out the form at:
https://druhfarrell.nationbuilder.com/tshirt

But I also saw it last night while submitting a payment on ticketmaster.com.
I notice that it also happens on the auto-save feature of the address' part so change the title accordingly. Thanks for reporting it.
Summary: [Form Autofill] Credit card autofill continues to prompt to remember user's card even after previously selecting "never" → [Form Autofill] Features on submission are not disabled even the enabled prefs are set to false
Whiteboard: [form autofill:MVP]
Assignee

Updated

2 years ago
Assignee: nobody → lchang
Status: NEW → ASSIGNED

Comment 3

2 years ago
mozreview-review
Comment on attachment 8913944 [details]
Bug 1404531 - [Form Autofill] Features on submission should be disabled when the prefs are set to false.

https://reviewboard.mozilla.org/r/185326/#review190374
Attachment #8913944 - Flags: review?(schung) → review+
STR for "Autofill Addresses":

0. If testing on non-nightly build, go to "about:config" and change the pref "extensions.formautofill.available" to "on" so that you can see the "Autofill Addresses" checkbox in Preferences page.

1. Go to "about:preferences" and open "Privacy & Security" page.
2. Uncheck "Autofill Addresses" (under "Forms & Passwords") to disable this feature.
3. Click "Saved Addresses..." button aside the checkbox and make sure there isn't any profile saved. (Remove them all if there's any.)
4. Go to any website containing address forms, e.g. demo page: https://luke-chang.github.io/autofill-demo/basic.html
5. Manually fill data in at least three fields.
6. Click "Submit" button.
7. Go back to "about:preferences" and open "Privacy & Security" page again.
8. Click "Saved Addresses..." button.

Expected: No profile saved.
Actual: A new profile saved.

Comment 5

2 years ago
Pushed by lchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c760f2f5aa0a
[Form Autofill] Features on submission should be disabled when the prefs are set to false. r=steveck
Reminder: The beta uplift depends on bug 1400147.

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c760f2f5aa0a
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Please verify STRs in both comment 0 and comment 4. Thanks.
Flags: qe-verify+
Comment on attachment 8913944 [details]
Bug 1404531 - [Form Autofill] Features on submission should be disabled when the prefs are set to false.

Approval Request Comment
[Feature/Bug causing the regression]:
Feature.

[User impact if declined]:
Form Autofill (address) won't be disabled completely even users uncheck the pref.

[Is this code covered by automated tests?]:
Yes. Landed with the patch.

[Has the fix been verified in Nightly?]:
Verified locally and qe-verify+ requested.

[Needs manual test from QE? If yes, steps to reproduce]: 
Yes. Please follow STRs in both comment 0 and comment 4.

[List of other uplifts needed for the feature/fix]:
Bug 1400147 (has already been uplifted.)

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
It just disables the feature within the add-on scope.

[String changes made/needed]:
N/A
Attachment #8913944 - Flags: approval-mozilla-beta?
Comment hidden (obsolete)
(In reply to Luke Chang [:lchang] from comment #10)
> [User impact if declined]:
> Form Autofill (address) won't be disabled completely even users uncheck the
> pref.

CreditCard Autofill won't ship with Fx57 and is hidden from 57beta, so it only affect Address Autofill.

> [Needs manual test from QE? If yes, steps to reproduce]: 
> Yes. Please follow STRs in both comment 0 and comment 4.

Therefore, comment 4 is the one for verifying the address' part. No need to verify comment 0 on the beta build. Thanks.
Comment on attachment 8913944 [details]
Bug 1404531 - [Form Autofill] Features on submission should be disabled when the prefs are set to false.

Seems like a must fix for 57, Beta57+
Attachment #8913944 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8914217 [details] [diff] [review]
bug1404531_for_56release.patch

Approval Request Comment
[Feature/Bug causing the regression]:
Feature.

[User impact if declined]:
Form Autofill (address) won't be disabled completely even users uncheck the pref.

(Note that CreditCard Autofill won't ship with Fx56, so it only affect Address Autofill.)

[Is this code covered by automated tests?]:
Yes. Landed with the patch.

[Has the fix been verified in Nightly?]:
Verified locally and qe-verify+ requested.

[Needs manual test from QE? If yes, steps to reproduce]: 
Yes. Please follow STRs in comment 4 for address' part.

[List of other uplifts needed for the feature/fix]:
N/A

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
It just disables the feature within the add-on scope.

[String changes made/needed]:
N/A
Attachment #8914217 - Flags: approval-mozilla-release?
Sounds like we're still offering to save data, but this may not be a pressing reason to do a new 56 release. 
Let's verify the fix in beta and see if this is impacting users broadly.
Andrei, can your team verify this fix in 57? Thanks.
Flags: needinfo?(andrei.vaida)
Verified as fixed using build 57.0b7 20171009192146 on Windows 10x64, Ubuntu 16.4 and MacOS 10.11.
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #16)
> Sounds like we're still offering to save data, but this may not be a
> pressing reason to do a new 56 release. 
> Let's verify the fix in beta and see if this is impacting users broadly.

This is a major issue for autofill as it means that we don't respect the user's preference. We won't do our gradual rollout (bug 1405217) without this fixed.

Even if it doesn't warrant a dot-release, it would be easiest to land this in mozilla-release for deployment:
* If we do a dot release, it could ride along with that.
* If we don't do a dot release, we can build the system add-on from mozilla-release and deploy the fix via Balrog.

If we really don't want to land in mozilla-release then we could build the SAO of a mozilla-release in our own clone and push to try to build the .xpi but having mozilla-release's continuous integration and reflecting what we're actually shipping is best IMO.
Blocks: 1405217
Flags: needinfo?(andrei.vaida)
Matt, still no obvious dot release driver here, do you want to go ahead with the system addon plan?  
I'll find you on IRC or send email to discuss.
Flags: needinfo?(MattN+bmo)
After discussing with Matt on IRC we are going to go ahead and update the system addon to fix the bug, and plan to ship that through balrog.  That will need some QA and then we can start the gradual rollout. 
In the meantime, as Matt suggests in comment 19, we can uplift the fix to m-r, in case we end up with a dot release (so far, we don't have a clear driver for one, which is a good thing)
Comment on attachment 8914217 [details] [diff] [review]
bug1404531_for_56release.patch

OK to uplift to m-r as a ridealong in case we have a 56 dot release.
Attachment #8914217 - Flags: approval-mozilla-release? → approval-mozilla-release+
Flags: needinfo?(MattN+bmo)
Keywords: checkin-needed
I'll make sure this gets uplifted to m-r if/when the decision to spin a 56.0.2 release is made.
Keywords: checkin-needed
Assignee

Updated

2 years ago
Blocks: 1410414
At this point we are starting to put together a 56.0.2, so go ahead as you have time!  I'm thinking to try to go to build on Tuesday.
I verified this issue using Nightly 58.0a1 with Build ID 20171108110838 on Windows 10 x64 and  Windows 7 x32.
I will mark this as verified fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.