1525340 - Add policy to disable CFR

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, defect, P1)

Description

[Mike Kaply [:mkaply]]

When CFR was moved to activity stream, it stopped honoring the Shield policy.

We need to add a new policy.

Note that we'll need to uplift this to 66. Not sure what to do about the translation.

Comment 1

[Mike Kaply [:mkaply]]

We should also not show CFR if xpinstall.enabled is false and locked.

Comment 2

[Mike Kaply [:mkaply]]

Attachment: Bug 1525340 - Disable CFR as part of XPI and shield policies.

Comment 3

[:Felipe Gomes (needinfo for replies!)]

I had already reported this issue on bug 1494778, but sadly it regressed. I feel like we shouldn't really need to change this pref (at least in the InstallAddonsPermission case), but as it's a simple fix that can be uplifted, let's do it.

Note that this means in 65, CFR will not be respecting a manual xpinstall.enabled pref change (only through the policy). We should probably file a new bug for that and really ensure that an AS test gets written for it so that it doesn't continue to regress.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

I filed bug 1526360

Comment 5

[:Felipe Gomes (needinfo for replies!)]

(In reply to Mike Kaply [:mkaply] from comment #1)

We should also not show CFR if xpinstall.enabled is false and locked.

Oh did you check if this was indeed failing? Or just assumed that there wasn't anything protecting against it.. I'm wondering if the bug that I filed is valid

Comment 6

[Mike Kaply [:mkaply]]

Oh did you check if this was indeed failing? Or just assumed that there wasn't anything protecting against it.. I'm wondering if the bug that I filed is valid

No I didn't because I couldn't figure out how to.

Maybe this actually does work and this was a false alarm?

Comment 7

[Mike Kaply [:mkaply]]

I verified that if you disable xpinstall via policy, you still get the recommendation and then when you go to install the addon, it doesn't work.

You can test this by flipping browser.newtabpage.activity-stream.asrouter.devtoolsEnabled and then going to about:newtab#asrouter

This lets you fire any of the CFR messags.

Comment 8

[Mike Kaply [:mkaply]]

Comment on attachment 9042191 [details]
Bug 1525340 - Disable CFR as part of XPI and shield policies.

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Not sure

User impact if declined

CFR can't be disabled via policy

I am not putting this on nightly/beta because we want to do a proper fix there.

Is this code covered by automated tests?

No

Has the fix been verified in Nightly?

No

Needs manual test from QE?

Yes

If yes, steps to reproduce

Disabling xpinstall via policy
Change the preference browser.newtabpage.activity-stream.asrouter.devtoolsEnabled to true
go to about:newtab#asrouter
and fake CFR messages

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Only adds some preferences setting into existing policy.

String changes made/needed

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Mike, were you going to push this to autoland?

Comment 10

[Ryan VanderMeulen [:RyanVM]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)

Mike, were you going to push this to autoland?

Sorry, I misread the approval request. Where is that follow-up work for 66/67 going to happen? IMO, it might be preferable to land this everywhere and then do that work in another bug on top of this one.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9042191 [details]
Bug 1525340 - Disable CFR as part of XPI and shield policies.

[Triage Comment]
Approving this for mozilla-release so it doesn't miss the 65.0.1 build, but I'd still like an answer to comment 10.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/12b02216df64

Comment 13

[Mike Kaply [:mkaply]]

Agreed. I'll land in autoland. Need to rebase.

Comment 14

[Emil Ghitta, QA [:emilghitta]]

This is verified fixed using Firefox 65.0.1 (BuildId:20190211233335) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 18.04 64bit

CFR gets successfully disabled while using the "InstallAddonsPermission" (set to "Default":false) and/or the "DisableFirefoxStudies" (set to true) policies. Also confirmed that it disables cfr from the about:newtab#asrouter page.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Hi Mike, any progress on this for 66+?

Comment 16

[Mike Kaply [:mkaply]]

I'll have it today.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38a2302d1e07
Disable CFR as part of XPI and shield policies. r=Felipe

Comment 18

[Ed Lee :Mardak]

Edit: moved to bug 1529078

Is the intention to only block add-on suggestions? CFR will soon suggest recommendations to pin tabs as part of bug 1501751 for 67. There's no add-ons to install for that.

Comment 19

[Mike Kaply [:mkaply]]

Is the intention to only block add-on suggestions? CFR will soon suggest recommendations to pin tabs as part of bug 1501751 for 67. There's no add-ons to install for that.

So we're going to start adding more and more things under the CFR "banner"?

The answer is we should block it all.

Comment 20

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/38a2302d1e07

Comment 21

[Mike Kaply [:mkaply]]

Comment on attachment 9042191 [details]
Bug 1525340 - Disable CFR as part of XPI and shield policies.

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

CFR

User impact if declined

CFR still happens in enterprise with xpinstall disabled.

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Adds two prefs to policy to test.

Note this probably will get the release patch, not the nightly patach.

String changes made/needed

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

How was this verified? Is there something QA can test here?

Comment 23

[Emil Ghitta, QA [:emilghitta]]

Sure, this can be manual tested. I have already drafted a test case which covers this policy change and verified the fix in 65.0.1 (comment 14).

This is also verified fixed in Firefox 67.0a1 (BuildId:20190220040540) using Windows 10 64bit, macOS 10.13.6 and Ubuntu 16.04 32bit.

Leaving the qe-verify+ until this gets verified in beta as well.

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9042191 [details]
Bug 1525340 - Disable CFR as part of XPI and shield policies.

Verified in nightly, ok for uplift for beta 11.

Comment 25

[Dorel Luca [:dluca]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6950e08762ed

Comment 26

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a4ed951479c1

Comment 27

[Emil Ghitta, QA [:emilghitta]]

This issue is verified fixed using Firefox 66.0b11 (BuildId:20190225143245) on Windows 10 64bit, macOS 10.12.6 and Ubuntu 16.04 64bit.

Comment 28

[Firefox Bug Husbandry Bot]

Please specify a root cause for this bug. See :tmaity for more information.