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

RESOLVED FIXED in Firefox 53

Status

()

Toolkit
Form Manager
P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: scottwu, Assigned: scottwu)

Tracking

(Blocks: 1 bug)

unspecified
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [form autofill:M1])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

a year ago
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".

Updated

a year ago
Priority: -- → P3
Whiteboard: [form autofill:M1]
Comment hidden (mozreview-request)
(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?
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?
Flags: needinfo?(MattN+bmo)
(Assignee)

Comment 5

a year ago
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 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.
Attachment #8822384 - Flags: review?(MattN+bmo) → review+
Flags: needinfo?(MattN+bmo)
(Assignee)

Comment 7

a year ago
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!
Keywords: checkin-needed

Comment 8

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ed96c5d75808
Change Form Autofill feature flag to browser.formautofill.experimental r=MattN
Keywords: checkin-needed

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ed96c5d75808
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.