1398656 - Always build the form autofill system add-on but disable it by default on release

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect, P1)

Description

[Luke Chang [:lchang] (inactive)]

The form autofill system add-on isn't currently built in release build. We should enable it once we get pre-release sign-off.

Comment 1

[Luke Chang [:lchang] (inactive)]

Attachment: Bug 1398656 - Enable form autofill system add-on in release build

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

Comment 2

[Matthew N. [:MattN]]

Attachment: Bug 1398656 - Always build the form autofill system add-on but disable it by default on release.

The pref will gradually be flipped via Shield to "detect" on release.

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

Comment 3

[Matthew N. [:MattN]]

[Tracking Requested - why for this release]: We will use Shield pref-flipping instead of balrog for the rollout and this makes that possible.

Comment 4

[Matthew N. [:MattN]]

I still have to test the preprocessing

Comment 5

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8907876 [details]
Bug 1398656 - Always build the form autofill system add-on but disable it by default on release.

https://reviewboard.mozilla.org/r/179556/#review184772

Thanks for taking this.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e5eab26b9de
Always build the form autofill system add-on but disable it by default on release. r=lchang

Comment 7

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8907876 [details]
Bug 1398656 - Always build the form autofill system add-on but disable it by default on release.

Approval Request Comment
[Feature/Bug causing the regression]:
Feature Shipment.

[User impact if declined]:
Form Autofill won't be built in release build.

[Is this code covered by automated tests?]:
Unnecessary as it's build configuration.

[Has the fix been verified in Nightly?]:
The change is for release channel only.

[Needs manual test from QE? If yes, steps to reproduce]: 
No.

[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 builds the system add-on in release channel as it is in beta channel.

[String changes made/needed]:
N/A

Comment 8

[Gerry Chang [:gchang]]

Track 56+ as this feature is disabled by default.

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5e5eab26b9de

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8907876 [details]
Bug 1398656 - Always build the form autofill system add-on but disable it by default on release.

Given m-b is 57 and m-r is 56 now, I move approval‑mozilla‑beta to approval‑mozilla‑release. Autofill will be disabled dy default. This should be merged to m-r.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/84e4b6c526bb (FIREFOX_56b13_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/044f93820039

Comment 12

[Gabi Cheta [:Gabi] Desktop QA]

Using Fx 56.0 RC-build 1, the prefcode "extensions.formautofill.available" has the value "staged-rollout", is this intended?

Comment 13

[Luke Chang [:lchang] (inactive)]

Hi Gabi,

Yes, any value other than "on" and "detect" makes formautofill disable. We do intend to disable it by default and will turn it on gradually by Shield.

Comment 14

[Gabi Cheta [:Gabi] Desktop QA]

Verified on Firefox 56.0 that autofill is disabled by default, under Windows 10x64, Ubuntu 16.04x86 and under macOS 10.12.

Comment 15

[Matthew N. [:MattN]]

(In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking you) from comment #4)
> I still have to test the preprocessing

I guess it didn't get tested before landing…

(In reply to Gabi Cheta from comment #12)
> Using Fx 56.0 RC-build 1, the prefcode "extensions.formautofill.available"
> has the value "staged-rollout", is this intended?

No, it's only supposed to be staged-rollout if the channel is "release".

Let me see if there's an easy fix then we can decide what to do here…

Comment 16

[Luke Chang [:lchang] (inactive)]

(In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking you) from comment #15)
> (In reply to Gabi Cheta from comment #12)
> > Using Fx 56.0 RC-build 1, the prefcode "extensions.formautofill.available"
> > has the value "staged-rollout", is this intended?
> 
> No, it's only supposed to be staged-rollout if the channel is "release".
> Let me see if there's an easy fix then we can decide what to do here…

Hi Matt, I thought RC-build is using channel "release".

Comment 17

[Matthew N. [:MattN]]

Don't think so… and I tested the official beta build and it's also set to "staged-rollout".

Comment 18

[Luke Chang [:lchang] (inactive)]

I see. You mean the problem is that we falsely disable it on beta build, too.

Comment 19

[Matthew N. [:MattN]]

So it looks like you may be right that the RC builds are built with the update channel of "release" but then the channel is switched back to "beta" afterwards through some magic. Not sure if this is documented anywhere…

(In reply to Luke Chang [:lchang] from comment #18)
> You mean the problem is that we falsely disable it on beta build, too.

Well I was more worried about release not getting the right value but yes, the other concern was that beta users lost autofill. I think that's actually fine so that the RC is testing what users will get before our staged rollout but I intentionally used an update channel check instead of an early beta check to avoid it getting disabled for beta users. I didn't realize there was some magic to build with one channel but ship with another.

In summary, I think everything is fine here but for the next beta cycle we should try to avoid this situation by doing the gradual rollout also for RC builds.

Comment 20

[Luke Chang [:lchang] (inactive)]

Yeah, I just noticed the same thing that my beta build gets RC-build after updating and I didn't know that would happen before. I'll pay more attention once we are going to gradual rollout the credit card's part. However, I don't know what you meant about doing gradual rollout also for RC build. Does that mean using gradual rollout mechanism but enable 100% users directly on RC builds?

BTW, for address' part, I think it should be set back to "detect" in 57-release by default, right?