1360746 - Split Forms and History engine prefs so each are able to be enabled independently

Status: RESOLVED WONTFIX

(Firefox :: Sync, enhancement, P1)

Description

[Mark Hammond [:markh] [:mhammond]]

The "form autofill" work planned for 56 is going to add 2 new sync engines - one for "payments/credit cards" and one for "address/profile" information (user-facing names for these TBD).

UX has decided that "History" and "Form History" will be split in all parts of the UI (eg, in about:preferences#privacy, instead of "Firefox will remember your browsing, download, form and search history, and keep cookie ...", there will be "Firefox will remember your browsing, download, and keep cookie ..." and new options for the existing "form and search history" and the new "profile/address" features.

They would also like a corresponding change for Sync prefs - "history" will be only browsing history (possibly with a user-facing string change), and there will be a new checkbox called "Forms" (exact string TBD) which will control the existing form history and the new "profile/address autofill history". (There will also be a new checkbox specifically for payments/credit-cards, but that will be a different bug.)

So tl;dr - we'll need a new preference specifically for forms (it is currently piggy-backed on the preference for history), which we can implement now. As the new sync engines are developed, they in-turn will be piggy-backed on the new "forms" preference. We'll obviously need to "migrate" the default value for this new forms preference based on the current value of the history pref. I believe no change will be needed to the "declined" logic, as the engines are already separate there.

Is there anything else I've forgotten?

cc: rnewman for awareness and to solicit general insights.
cc: Grisha and Steph as this may impact how mobile exposes these options, even before those platforms get the new features.
cc: rfkelly as a heads-up to expect an issue on the content-server to implement the above for the signup-flow, where these preferences are implemented on the web.

Comment 1

[Richard Newman [:rnewman]]

Random thoughts that come to mind:

- Android with native settings might not behave correctly if a user modifies their engine choices. Similarly, older desktops won't know how to display this new division. But I expect all clients to correctly persist meta/global when necessary, and respect settings unless touched by the user.
- The history engine pref is currently a default pref (true). If you plan to derive the forms pref from that, it'll need to be user-set, and stay user-set perhaps forever, no?
- Bug 608231 might be relevant at some point.

Comment 2

[:Grisha Kruglov]

(In reply to Richard Newman [:rnewman] from comment #1)
> - Android with native settings might not behave correctly if a user modifies
> their engine choices. Similarly, older desktops won't know how to display
> this new division. But I expect all clients to correctly persist meta/global
> when necessary, and respect settings unless touched by the user.

Sync itself is driven by what's in the meta/global engines/declined lists, so the engines will run as you'd expect them based on the two lists.

I've briefly looked through android's prefs code, and if we're just going to re-use the "forms" engine, the older clients will handle user interactions in a way that is probably non-ideal and will surprise users. We have a few cases:

Forms | History
1 1 - trivial case
1 0 - will be displayed as "history off". If user flips history to "off", prefs code will also flip Forms to off.
0 1 - will be displayed as "history on". If user flips history to "on", prefs code will also flip Forms to on.
0 0 - trivial case

And so we'll get into scenarios like these:
- user disables history and forms sync on a newer device
- user enables history sync on an older android device
- forms sync gets enabled as well without their consent.

The situation is made worse by the fact that android prefs UI does not at all describe that toggling history sync will also affect forms at all.

I expect that older desktop clients will behave in some similar fashion?

A counter proposal is to introduce a new sync engine, say "forms2". Newer clients that are aware of it can do the right thing and ignore changes to the state of legacy "forms" engine and may choose to flip that on/off to match behaviour of older clients. More importantly, changing sync prefs on older clients will not affect new clients in surprising ways.

Comment 3

[Richard Newman [:rnewman]]

(In reply to :Grisha Kruglov from comment #2)

> A counter proposal is to introduce a new sync engine, say "forms2". Newer
> clients that are aware of it can do the right thing and ignore changes to
> the state of legacy "forms" engine and may choose to flip that on/off to
> match behaviour of older clients. More importantly, changing sync prefs on
> older clients will not affect new clients in surprising ways.

This also gives an opportunity to fix the forms record format:

  Bug 997718
  Bug 697482
  Bug 745409
  Bug 985516

at the risk of running into Bug 608231 and other fun potholes.

Comment 4

[Mark Hammond [:markh] [:mhammond]]

(In reply to :Grisha Kruglov from comment #2)
> I've briefly looked through android's prefs code, and if we're just going to
> re-use the "forms" engine, the older clients will handle user interactions
> in a way that is probably non-ideal and will surprise users.

I don't see any way to avoid that - older clients simply don't have a way to separate those 2 engines and there's nothing we can do about that.

> Forms | History
> 1 1 - trivial case
> 1 0 - will be displayed as "history off". If user flips history to "off",
> prefs code will also flip Forms to off.
> 0 1 - will be displayed as "history on". If user flips history to "on",
> prefs code will also flip Forms to on.
> 0 0 - trivial case
> 
> And so we'll get into scenarios like these:
> - user disables history and forms sync on a newer device
> - user enables history sync on an older android device
> - forms sync gets enabled as well without their consent.

Yes, but as above, I don't see any other options. In particular, "forms sync gets enabled as well without their consent" is exactly what happens now. Landing this in 55 makes it less likely that users will hit the "with formautofill" and "without formautofill" cases.

> The situation is made worse by the fact that android prefs UI does not at
> all describe that toggling history sync will also affect forms at all.

Neither does desktop.

> I expect that older desktop clients will behave in some similar fashion?

Yep.

> A counter proposal is to introduce a new sync engine, say "forms2". Newer
> clients that are aware of it can do the right thing and ignore changes to
> the state of legacy "forms" engine and may choose to flip that on/off to
> match behaviour of older clients. More importantly, changing sync prefs on
> older clients will not affect new clients in surprising ways.

I assume here you are saying a real new engine? If so, ISTM this just creates other problems - eg, form history added on the new client isn't going to appear on the first - unless the new client tries to sync *both* engines, which seems a much larger can of worms.

The way I see it, old clients currently can't differentiate between forms and history, and this will continue. New clients will. If they have a mix of old and new clients, there is always going to be some degree of confusion as the options presented will be different. This confusion will slowly go away as clients update (and clients *not* updating is something we historically haven't cared about in various contexts, such as in the move from "legacy" to FxA sync).

I believe most users do not adjust their syncing preferences very often, so ISTM we are trying to handle a stack of edge cases (ie, a user with devices that don't auto-update within some reasonable time, and who changes their sync history preferences during that period) - and we can't avoid the issue that new devices will split forms and history in the UI while old ones don't.

We'd want Android to be updated along with Desktop, but I don't think there's any way to avoid some degree of confusion when users have both old and new implementations - so we should aim to minimize *both* confusion for users and complexity for us.

So I don't see why we can't just:

* create a new pref, services.sync.engine.forms, default value is irrelevant.
* as sync initializes, check if another pref, say, "services.sync.engine.forms.migrated" is false and if so, we'll set services.sync.engine.forms to the value of services.sync.engine.history
* accept that if a user modifies the engine prefs on a new client, there's going to some degree of confusion in old clients as it's impossible to represent that in its UI.
* profit? :)

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Recent UX decision means we will be adding 2 new engines each with their own pref, so this doesn't need to be done.