1326153 - [Form Autofill] Change the feature flag to browser.formautofill.experimental

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect, P3)

Description

[Scott Wu [:scottwu]]

Currently the preference used to enable form autofill is "browser.formautofill.enabled", but this is better suited for toggling on/off the feature in preferences. We should instead use "browser.formautofill.experimental".

Comment 1

[Scott Wu [:scottwu]]

Attachment: Bug 1326153 - Change Form Autofill feature flag to browser.formautofill.experimental

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

Comment 2

[Scott Wu [:scottwu]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2e45d166354cfb5edb4ea3c63fff67422f4618c

Comment 3

[Matthew N. [:MattN]]

(In reply to Scott Wu [:scottwu] from comment #0)
> Currently the preference used to enable form autofill is
> "browser.formautofill.enabled", but this is better suited for toggling
> on/off the feature in preferences.

The pref is used to enable/disable the feature and I don't currently see why that wouldn't be the pref that we expose in preferences?

Comment 4

[Luke Chang [:lchang] (inactive)]

Hi Matt,

As we discuss earlier on IRC, we will register/unregister searching module of formautofill to toggle the function when we detect there is/isn't profile saved. I suppose this approach could apply in the case when we toggle the checkbox from Preferences, which means the module should be always initialized no matter the toggle in Preferences is on or off. Thus, we might need another pref for disabling the entire addon during developing. That's why I ask Scott to file this bug.

However, do you mean we can still link the checkbox in Preferences to the "browser.formautofill.enabled" in the bootstrap.js? I imagine we might need to restart addon to let the pref works, right?

Comment 5

[Scott Wu [:scottwu]]

I'd like to add that, yes the pref "browser.formautofill.enabled" would still be used for enable/disable the feature in preferences, but there needs a different pref that makes the control visible in preferences at first place. If the same pref is used and you turn the feature off from the preferences, you won't be able to turn it back on again because the very control would disappear on restart.

If we use the same flag, this would happen:
Turn the feature on manually from about:config -> restart browser -> The control shows up in preferences -> Turn the feature off in preferences -> restart browser -> Control disappears from the preferences and can't be turned back on except through about:config again

Comment 6

[Matthew N. [:MattN]]

Comment on attachment 8822384 [details]
Bug 1326153 - Change Form Autofill feature flag to browser.formautofill.experimental

https://reviewboard.mozilla.org/r/101324/#review102802

I think I see the distinction now:
a) Whether the extension code runs at all… mostly only useful for development since the actual extension is installed and enabled by default.
b) Whether the "feature" is enabled (usually represented by a checkbox in prefs).

I don't really like the "experimental" suffix since this doesn't feel like much of an experiemnt but I can't think of anything better so it's fine with me.

I agree that .enabled should be used for (b) above as that's the convention.

Sorry for the delay.

Comment 7

[Scott Wu [:scottwu]]

Yeah it's not exactly an experiment but we'll only use it for development and it wouldn't be a problem once we have the extension enabled by default.

Thanks Matt!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ed96c5d75808
Change Form Autofill feature flag to browser.formautofill.experimental r=MattN

Comment 9

[Iris Hsiao [:ihsiao]]

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