Closed Bug 1626055 Opened 2 years ago Closed 1 year ago

Disable address autofill auto-saving until we deal with duplicate addresses

Categories

(Toolkit :: Form Autofill, task, P1)

Desktop
All
task

Tracking

()

VERIFIED FIXED
mozilla80
Tracking Status
firefox-esr68 --- wontfix
firefox-esr78 --- wontfix
firefox74 --- wontfix
firefox75 --- wontfix
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- wontfix
firefox79 --- wontfix
firefox80 --- verified

People

(Reporter: MattN, Assigned: jimm)

References

(Regressed 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

We are well aware of address autofill creating many duplicate and useless profiles for users with it enabled but we haven't prioritized fixing it (bug 1495882 and dependencies for duplicates) properly so users still end up with a bad experience having dozens of profiles. We should either fix the issues of unwanted profiles or consider mitigating the issue.

Here is my proposal:

  1. Don't enable the feature for RTL or CA/DE on Nightly i.e. make Nightly match release so more people don't suffer for no reason as we have no active development to ship autofill for RTL/CA/DE. (code to delete)
  2. Disable the code to automatically save addresses from form submissions. Users will have to do things like the Netscape days where you manually go to about:preferences to add your address profile in advance. This is much less discoverable but means users won't get annoyed by unwanted address profiles as they are all curated by hand.

Cindy, are you fine with this? I think Address autofill is a feature we need for parity but I'm not sure the current state is better than having it hand-curated by a minority of users who need it.

Flags: needinfo?(chsiang)
See Also: → 1556199

Wouldn't this also be an issue for US builds? My understanding is that we shipped the address capture to US release, is that correct?

Flags: needinfo?(MattN+bmo)

(In reply to Jim Mathies [:jimm] from comment #1)

Wouldn't this also be an issue for US builds? My understanding is that we shipped the address capture to US release, is that correct?

Yes, that's what part 2 of my proposal addresses. My proposal is to do both 1 and 2.

Flags: needinfo?(MattN+bmo)

Maybe we should add telemetry first to get a sense of the problem? That means we keep the features on for a while longer, but that's the status quote so we're not really doing any harm. I might be able to find someone to help put those probes together with some guidance on what we want to capture.

Flags: needinfo?(MattN+bmo)

(In reply to Jim Mathies [:jimm] from comment #3)

Maybe we should add telemetry first to get a sense of the problem?

We could put a poll in Slack for US employees (or CA/DE on Nightly) and save some time :) I think the results will make it clear that the auto-saving creates many duplicates.

I scrounged for data we do have which, combined with the anecdotal evidence from dozens of US employees, is clear enough that this is a significant problem worth addressing yesterday: 47.5% of Sync users on their first sync download 10 or more address records https://sql.telemetry.mozilla.org/queries/70870#178263 (this may not accurately represent our whole US user base but it already demonstrates the problem):

Max. # of address autofill records download on initial sync # of users
0-4 33.3%
5-9 19.5%
10-14 12%
15-19 8.16%
20+ 27%

That means we keep the features on for a while longer, but that's the status quote so we're not really doing any harm.

We are likely doing harm as I suspect this is a churn driver.

I might be able to find someone to help put those probes together with some guidance on what we want to capture.

I don't think it's necessary with the anecdotal evidence and the sync data above.

Severity: normal → S3
Flags: needinfo?(MattN+bmo)

Hi Matt,
Thanks for sharing the data. Let's move forward with your proposal to turn off the auto-save.

Flags: needinfo?(chsiang)

Thank you. I will work on this in the next week hopefully.

Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Summary: Consider disabling address autofill (or auto-saving) until we deal with duplicate addresses → Disabling address autofill auto-saving until we deal with duplicate addresses
Flags: qe-verify+

It's no longer clear to me if we are going ahead with this plan or addressing the cause of the duplicates (deps of bug 1495882). I hope to find out within the next week.

Summary: Disabling address autofill auto-saving until we deal with duplicate addresses → Disable address autofill auto-saving until we deal with duplicate addresses

Hey Matt, we decided to go ahead and disable the capture feature on release. Can you handle getting that change out or should I point someon at this?

Flags: needinfo?(MattN+bmo)

I think it would be better to have your team work on this as I have too many projects on the go. Now that there are engineers working on the code I think it would still be better to fix the two bugs at https://bugzilla.mozilla.org/showdependencytree.cgi?id=1495882&hide_resolved=1 instead as the scope isn't that different and disabling auto-saving only to bring it back a few months later will be confusing to users. Either option is better than nothing though.

If we disable auto-saving we should add a pref and checkbox for it below the address autofill option.

Flags: needinfo?(MattN+bmo)
Assignee: MattN+bmo → nobody
Status: ASSIGNED → NEW
Assignee: nobody → jmathies
Attached patch wip (obsolete) — Splinter Review

Looks like there are some tests associated with address autofill, pushed this to try to get a readout on what needs to be disabled. I also tested this in a good test case to confirm:

  1. address capture and merging is disabled
  2. after manually adding an address autofill dropdown still displays and fills.
Attachment #9163860 - Attachment is patch: true

Instead of removing the logic to capture, I think I'm going to tie this to a new pref. This way we can keep the address capture related tests running.

Attached patch wip (obsolete) — Splinter Review
Attachment #9163860 - Attachment is obsolete: true
Attached patch wip (obsolete) — Splinter Review
Attachment #9164051 - Attachment is obsolete: true

(In reply to Jim Mathies [:jimm] from comment #11)

Instead of removing the logic to capture, I think I'm going to tie this to a new pref. This way we can keep the address capture related tests running.

Yes, I even think it should be a user-facing preference nested below the address autofill checkbox.

Attachment #9164055 - Attachment is obsolete: true

(In reply to Matthew N. [:MattN] from comment #14)

(In reply to Jim Mathies [:jimm] from comment #11)

Instead of removing the logic to capture, I think I'm going to tie this to a new pref. This way we can keep the address capture related tests running.

Yes, I even think it should be a user-facing preference nested below the address autofill checkbox.

I don't have the time to put together new UI for this. Can we keep it as a hidden pref? It'll only be around for a few months.

Pushed by jmathies@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2c616f35af14
Disable address autofill auto-saving until we deal with duplicate addresses. r=MattN
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80

Verified > Fixed on 80.0b3 (20200803045446) on Windows 10 x64, Windows 7 x64 and Ubuntu 18.04.4.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Regressions: 1660756
You need to log in before you can comment on or make changes to this bug.