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

Status: VERIFIED FIXED

(Toolkit :: Form Manager, defect)

Description

[(No longer employed by Mozilla) Aaron Klotz]

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.

Comment 1

[Luke Chang [:lchang] (inactive)]

I notice that it also happens on the auto-save feature of the address' part so change the title accordingly. Thanks for reporting it.

Comment 2

[Luke Chang [:lchang] (inactive)]

Attachment: Bug 1404531 - [Form Autofill] Features on submission should be disabled when the prefs are set to false.

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

Comment 3

[Steve Chung [:steveck]]

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

Comment 4

[Luke Chang [:lchang] (inactive)]

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

[Pulsebot]

Pushed by [e-mail]:
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

Comment 6

[Luke Chang [:lchang] (inactive)]

Attachment: bug1404531_for_56release.patch

Comment 7

[Luke Chang [:lchang] (inactive)]

Reminder: The beta uplift depends on bug 1400147.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 9

[Luke Chang [:lchang] (inactive)]

Please verify STRs in both comment 0 and comment 4. Thanks.

Comment 10

[Luke Chang [:lchang] (inactive)]

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

Comment 11

[Luke Chang [:lchang] (inactive)]

(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.

Correct: Form Autofill (address & credit card) won't be disabled ...

Comment 12

[Luke Chang [:lchang] (inactive)]

(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 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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+

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/780761b37a04

Comment 15

[Luke Chang [:lchang] (inactive)]

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

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Andrei, can your team verify this fix in 57? Thanks.

Comment 18

[Gabi Cheta [:Gabi] Desktop QA]

Verified as fixed using build 57.0b7 20171009192146 on Windows 10x64, Ubuntu 16.4 and MacOS 10.11.

Comment 19

[Matthew N. [:MattN]]

(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.

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

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 22

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

I'll make sure this gets uplifted to m-r if/when the decision to spin a 56.0.2 release is made.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Per IRC discussion.

https://hg.mozilla.org/releases/mozilla-release/rev/db7c9639552f00fdd13386e24c042ed398cca53a

Comment 25

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 26

[Valentina Claudia Ona]

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.