[Affected versions]: - latest Nightly 65.0a1 - Beta 64.0b5 [Affected platforms]: - Windows 10 x64 - macOS 10.13 - Ubuntu 16.04 x64 [Preconditions]: - enable “DisableFirefoxScreenshots” policy via polices.json or GPO [Steps to reproduce]: 1. Launch Firefox. 2. Access any webpage and take a screenshot via context menu. [Expected result]: - Firefox screenshots is disabled. [Actual result]: - Firefox screenshots is enabled. [Regression range]: - Last good revision: e18f1acd0782f6333ee892c7ebc9711d9a5e550a First bad revision: 70a5ef5872f895009bfdf2958616ca0ecece807f Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e18f1acd0782f6333ee892c7ebc9711d9a5e550a&tochange=70a5ef5872f895009bfdf2958616ca0ecece807f [Additional notes]: - In order to reproduce this issue more easily I have attached the policies.json file

This shouldn't block bug 1445943 since this issue is also present with JSON policies. Bug 1445943 is simply adding support for policies via `defaults write` and configuration profiles but does not change the way policies are being applied by Firefox.

I think bug 1488971 + bug 1498410 combined caused this bug. IIUC, in bug 1488971, monitor of pref change was moved outside of extension (and delayed to window restored), but the check on startup was still in bootstrap.js [1], and that check was then removed as part of bug 1498410. [1]: https://hg.mozilla.org/mozilla-central/rev/3113a83b3455#l3.48

Jared, can you take a look at this please? Note that pocket moved to a WebExtension but was still able to honor the pref. Not sure how.

The addons team asked that we move pref-based management into the addons code, and out of the webextension. Maybe aswan has ideas.

To clarify, I can take a look as well, but aswan may have better ideas

> Note that pocket moved to a WebExtension but was still able to honor the pref. Not sure how. I don't see pocket in the list of webextensions in browser/extensions[1], so maybe their code was just merged into the tree. Are there any known issues with webcompat-reporter (assuming it's exposed via an enterprise policy)? It seems to be using almost the same nsBrowserGlue check as screenshots[2]. [1] https://searchfox.org/mozilla-central/source/browser/extensions [2] https://searchfox.org/mozilla-central/source/browser/components/nsBrowserGlue.js#1457-1469

pocket is not a webextension. The code that monitors that pref is apparently only checking for runtime changes to the pref, if the value flips before the observer is attached (and I assume policies are applied early during startup?) then nothing will ever notice.

MozReview-Commit-ID: FGUaKbXhit2

Do we want to get this patch uplifted to 64?

Comment on attachment 9023033 [details] Bug 1503893 - Ensure Screenshots disable pref is checked at startup; r?aswan Hey, could you take a look at this patch? It's marked critical, and I suspect will be targeting uplift into 64

Yes, please.

I think the needinfo was just to remind me that this was also on Phabricator? If I misunderstood and there's something else please re-flag me

Pushed by jhirsch@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0ecf239647a4 Ensure Screenshots disable pref is checked at startup; r=aswan

I wonder why the test at https://searchfox.org/mozilla-central/source/browser/components/enterprisepolicies/tests/browser/disable_fxscreenshots/ never failed. Is it the case that if this pref was set on a fresh profile (before the first run of Firefox Screenshots), it would work correctly? (because that's how this test behaves)

Comment on attachment 9023033 [details] Bug 1503893 - Ensure Screenshots disable pref is checked at startup; r?aswan [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1488971 User impact if declined: Firefox Screenshots will not be disabled by the enterprise group policy Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: Yes Needs manual test from QE?: Yes If yes, steps to reproduce: Set the 'DisableFirefoxScreenshots' policy, start / restart Firefox, verify none of the Screenshots UI is visible (Library menu item, page action item, context menu item). Visit screenshots.firefox.com/shots and verify no shots are shown. List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): The change is small and only affects Screenshots by checking a pref's state at startup, rather than only listening for the pref to be changed. String changes made/needed: None

This bug is no longer reproducible on latest Nightly 65.0a1 (2018-11-15) across platforms: Windows 10 x64, macOS 10.13 and Ubuntu 16.04 x64. Firefox Screenshots is successfully disabled (from Library, Context Menu and Page Actions) after the policy is applied. Needinfo myself as a reminder, to verify this on Beta once the patch is approved and uplifted.

Comment on attachment 9023033 [details] Bug 1503893 - Ensure Screenshots disable pref is checked at startup; r?aswan fix regression with screenshots and enterprise policies, approved for 64.0b10

This issue is also verified fixed on 64.0b10 running Windows 10 x64, macOS 10.12 and Ubuntu 16.04 x64.