Closed Bug 1525340 Opened 7 months ago Closed 7 months ago

Add policy to disable CFR

Categories

(Firefox :: Enterprise Policies, defect, P1, major)

defect

Tracking

()

VERIFIED FIXED
Firefox 67
Tracking Status
firefox-esr60 --- unaffected
firefox65 + verified
firefox66 blocking verified
firefox67 + verified

People

(Reporter: mkaply, Assigned: mkaply)

References

Details

Attachments

(1 file)

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.

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

QA Contact: emil.ghitta

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.

(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

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?

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 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

Attachment #9042191 - Flags: approval-mozilla-release?

Mike, were you going to push this to autoland?

Flags: needinfo?(mozilla)

(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 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.

Attachment #9042191 - Flags: approval-mozilla-release? → approval-mozilla-release+
Flags: qe-verify+

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

Flags: needinfo?(mozilla)
Whiteboard: [qa-triaged]

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.

Flags: qe-verify+

Hi Mike, any progress on this for 66+?

Flags: needinfo?(mozilla)

I'll have it today.

Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/38a2302d1e07
Disable CFR as part of XPI and shield policies. r=Felipe

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.

Blocks: 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.

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

The answer is we should block it all.

Flags: needinfo?(mozilla)
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

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

Attachment #9042191 - Flags: approval-mozilla-beta?

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

Flags: needinfo?(mozilla)
Flags: qe-verify+

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.

Status: RESOLVED → VERIFIED
Flags: needinfo?(mozilla) → in-qa-testsuite+

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.

Attachment #9042191 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Severity: normal → major

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

Flags: qe-verify+
QA Whiteboard: [qa-triaged]
Whiteboard: [qa-triaged]
You need to log in before you can comment on or make changes to this bug.