Closed Bug 1432355 Opened 6 years ago Closed 6 years ago

Policy: Disable private browsing

Categories

(Firefox :: Enterprise Policies, enhancement, P1)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 60
Tracking Status
firefox60 --- verified

People

(Reporter: Felipe, Assigned: mkaply)

References

Details

Attachments

(3 files)

Policy: Disable private browsing
Freddy directed us to Josh Matthews as subject matter expert.  He is covering Private Browsing during Ehsans' absence.
Some notes after research:

Returning false from all of these functions - https://searchfox.org/mozilla-central/source/toolkit/modules/PrivateBrowsingUtils.jsm - completely disables private browsing. We would need to update the about:privatebrowsing page as well.

There are pretty obvious hooks in the various context menus where we could remove private browsing if we wanted to including activity stream.

In my tests, removing the private browsing button from customizableUI got rid of the menu and the toolbar button.

For the taskbar on Windows, we'll probably need to add a custom pref around here:

https://searchfox.org/mozilla-central/source/browser/modules/WindowsJumpLists.jsm#107

If we want to get rid of it.
Josh,

Can you think of any more clever way to disable private browing?
Flags: needinfo?(josh)
Attached patch Very early passSplinter Review
This is an early pass at disabling private browsing.

I tried to figure out to make it the least intrusive as possible.

Still need to figure out how to disable the private browsing from the taskbar on Windows.

I decided to have the state of private browsing on PrivateBrowsingUtils versus using the policy everywhere. That way if we ever wanted to implement a pref for this one, it would be easy.
Attachment #8949536 - Flags: feedback?(felipc)
I have not been able to come up with more clever than the current patch. Returning false values from the various functions in PrivateBrowsingUtils makes me quite nervous.
Flags: needinfo?(josh)
> I have not been able to come up with more clever than the current patch. Returning false values from the various functions in PrivateBrowsingUtils makes me quite nervous.

Yeah, I completely agree. I think it makes more sense to just resolve the user endpoints and then override about:privatebrowsing.

Are you comfortable with the patch as is? My last thing to do is take care if the "open private window" taskbar action on Windows.
Comment on attachment 8949536 [details] [diff] [review]
Very early pass

Review of attachment 8949536 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/browser.js
@@ +4069,5 @@
>    var wintype = document.documentElement.getAttribute("windowtype");
>  
>    var extraFeatures = "";
>    if (options && options.private) {
> +    if (!PrivateBrowsingUtils.enabled) {

I would hoist this check into the previous if statement, instead. Right now we have this weird mix of loading about:privatebrowsing but the window not actually being considered private.
> I would hoist this check into the previous if statement, instead. Right now we have this weird mix of loading about:privatebrowsing but the window not actually being considered private.

Yeah, my plan in this case was to load the about:privatebrowsing window because it would have a message that private browsing had been disabled.

I'll come up with a nicer way (and comments).
i was trying to hard to still show about:privatebrowing if a private window somehow got opened (to show the block message).

Instead, I just concentrated on removing all entry points to private browsing.
Comment on attachment 8949829 [details]
Bug 1432355 - Add a policy to disable private browsing.

https://reviewboard.mozilla.org/r/219140/#review224906


Code analysis found 5 defects in this patch:
 - 5 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/base/content/browser.js:1374
(Diff revision 1)
>  
> +    if (!PrivateBrowsingUtils.enabled) {
> +      document.getElementById("Tools:PrivateBrowsing").hidden = true;
> +      // Setting disabled doesn't disable the shortcut, so we just remove
> +      // the keybinding.
> +      pbKeyElement = document.getElementById("key_privatebrowsing");

Error: 'pbkeyelement' is not defined. [eslint: no-undef]

::: browser/base/content/browser.js:1375
(Diff revision 1)
> +    if (!PrivateBrowsingUtils.enabled) {
> +      document.getElementById("Tools:PrivateBrowsing").hidden = true;
> +      // Setting disabled doesn't disable the shortcut, so we just remove
> +      // the keybinding.
> +      pbKeyElement = document.getElementById("key_privatebrowsing");
> +      pbKeyElement.parentNode.removeChild(pbKeyElement);

Error: Use element.remove() instead of element.parentnode.removechild(element) [eslint: mozilla/avoid-removeChild]

::: browser/base/content/browser.js:1375
(Diff revision 1)
> +    if (!PrivateBrowsingUtils.enabled) {
> +      document.getElementById("Tools:PrivateBrowsing").hidden = true;
> +      // Setting disabled doesn't disable the shortcut, so we just remove
> +      // the keybinding.
> +      pbKeyElement = document.getElementById("key_privatebrowsing");
> +      pbKeyElement.parentNode.removeChild(pbKeyElement);

Error: 'pbkeyelement' is not defined. [eslint: no-undef]

::: browser/base/content/browser.js:1375
(Diff revision 1)
> +    if (!PrivateBrowsingUtils.enabled) {
> +      document.getElementById("Tools:PrivateBrowsing").hidden = true;
> +      // Setting disabled doesn't disable the shortcut, so we just remove
> +      // the keybinding.
> +      pbKeyElement = document.getElementById("key_privatebrowsing");
> +      pbKeyElement.parentNode.removeChild(pbKeyElement);

Error: 'pbkeyelement' is not defined. [eslint: no-undef]

::: toolkit/modules/PrivateBrowsingUtils.jsm:17
(Diff revision 1)
>  // line for the current session.
>  var gTemporaryAutoStartMode = false;
>  
>  this.PrivateBrowsingUtils = {
> +  get enabled() {
> +    return Services.policies.isAllowed("privatebrowsing")

Error: Missing semicolon. [eslint: semi]
Comment on attachment 8949829 [details]
Bug 1432355 - Add a policy to disable private browsing.

https://reviewboard.mozilla.org/r/219140/#review224928

The changes in PrivateBrowsingUtils.jsm look fine to me. I'm not really qualified to review changes to the rest of code in browser/, however.
Attachment #8949829 - Flags: review?(josh)
Comment on attachment 8949829 [details]
Bug 1432355 - Add a policy to disable private browsing.

https://reviewboard.mozilla.org/r/219140/#review225792

This is awesome! Very exciting :)

In the bug to remove access to about addons (bug 1429123) the UX direction that we ended up with was not to remove the UI entry points, but simply let it fail with the "blocked page" message. However, for PB, I agree that the easiest at least for now is to do this approach..

::: browser/components/enterprisepolicies/Policies.jsm:98
(Diff revision 2)
> +    onBeforeUIStartup(manager, param) {
> +      if (param == true) {
> +        manager.disallowFeature("privatebrowsing");
> +        manager.disallowFeature("about:privatebrowsing", true);
> +        setAndLockPref("browser.privatebrowsing.autostart", false);
> +        CustomizableUI.destroyWidget("privatebrowsing-button");

On another bug, Gijs mentioned that it might be too early to load CustomizableUI here.

The best thing to do is to use it from nsBrowserGlue.js' `_scheduleStartupIdleTasks`, where you'd check .isAllowed() or PrivateBrowsingUtils.enabled before calling destroyWidget.
(Or there's an equivalent in browser.js if this needs to run once per browser window)

Although there was another question about whether destroyWidget itself was right or not.. but that conversation didn't reach a conclusion (https://bugzilla.mozilla.org/show_bug.cgi?id=1429123#c22)

::: browser/components/places/content/controller.js:1521
(Diff revision 2)
>    }
>  
>    updatePlacesCommand("placesCmd_open");
>    updatePlacesCommand("placesCmd_open:window");
>    updatePlacesCommand("placesCmd_open:privatewindow");
> +  document.getElementById("placesCmd_open:privatewindow").hidden = !PrivateBrowsingUtils.enabled;

nit: this is probably better added at the end of this group of `updatePlacesCommand` instead of in the middle of it

::: browser/modules/WindowsJumpLists.jsm:107
(Diff revision 2)
>                              // shutdown. Thus true for consistency.
>    },
> +];
>  
> -  // Open new private window
> +// Open new private window
> -  {
> +var privateWindowTask = {

s/var/let
Comment on attachment 8949536 [details] [diff] [review]
Very early pass

(feedback provided on the newer patch)
Attachment #8949536 - Flags: feedback?(felipc) → feedback+
if _scheduleStartupIdleTasks is too late, you can use some other entry point like delayedStartup in browser.js
> The best thing to do is to use it from nsBrowserGlue.js' `_scheduleStartupIdleTasks`, where you'd check .isAllowed() or PrivateBrowsingUtils.enabled before calling destroyWidget.
(Or there's an equivalent in browser.js if this needs to run once per browser window)

Would it make more sense to provide a way to call this properly from the policy manager?

I'm thinking we'll probably want to use CustomizableUI in more cases.
Comment on attachment 8949829 [details]
Bug 1432355 - Add a policy to disable private browsing.

https://reviewboard.mozilla.org/r/219140/#review227886

https://bugzilla.mozilla.org/show_bug.cgi?id=1429123 ended up not disabling the entrypoints but showing an explanatory page if you tried to open it. Wouldn't that be better here, too? We can add code to about:privatebrowsing that shows that private browsing is disabled, and make the UI continue to open a window - just not a private one.

For the private browsing button, I'd prefer if it were simply disabled.

Also, here as well as with many of the other policies, I'm unsure how much we want to harden things. Like, is it OK that these policies can be trivially circumvented? It would be more tamperproof, for instance, if the window management code that actually *read* the 'private' feature flag ignored it if privatebrowsing was disabled.
Attachment #8949829 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8949829 [details]
Bug 1432355 - Add a policy to disable private browsing.

https://reviewboard.mozilla.org/r/219140/#review227886

> https://bugzilla.mozilla.org/show_bug.cgi?id=1429123 ended up not disabling the entrypoints but showing an explanatory page if you tried to open it. Wouldn't that be better here, too? We can add code to about:privatebrowsing that shows that private browsing is disabled, and make the UI continue to open a window - just not a private one.

I think this is a very different situation than about:addons because of the number of entry points as well as the fact that not all entry points open up about:privatebrowsing. You would just end up with multiple menuitems that do the same things.

> For the private browsing button, I'd prefer if it were simply disabled.

What do disabled buttons do in the customize window? Are they still draggable?

> Like, is it OK that these policies can be trivially circumvented

Can you give an example of how that would happen without devtools? Our goal is basically to prevent a normal user from accessing these features.
Comment on attachment 8949829 [details]
Bug 1432355 - Add a policy to disable private browsing.

https://reviewboard.mozilla.org/r/219140/#review227886

I ran some tests and disabling isn't want we want here.

A user can interact with Customize and drag the item to the overflow menu and then at that point it is disabled. 

That's a bad user experience.

We shouldn't have the button if a user can't use it.
(In reply to Mike Kaply [:mkaply] from comment #20)
> > Like, is it OK that these policies can be trivially circumvented
> 
> Can you give an example of how that would happen without devtools? Our goal
> is basically to prevent a normal user from accessing these features.

You'd need privileged script execution, but as you know there are a lot of ways for that to happen. Devtools are one; autoconfig would be one; people have figured out how to use userChrome.css to bootstrap into system-privileged JS code; there have been other cases in the past where we don't effectively protect from e.g. prefs that have HTML in them that we insert without sanitization. Obviously I'm not going to enumerate any *current* security issues I'm aware of.

Anyway, half of my point is that I don't think that people who run a corporate network and who want to disable pb mode are going to put 2 and 2 together and realize they have to block devtools, and installing add-ons, and userChrome.css, and... I don't think expecting them to figure that out is reasonable - I think we should provide something out of the box that actually does what it says on the tin, without having to disable the other features.
(In reply to Mike Kaply [:mkaply] from comment #21)
> Comment on attachment 8949829 [details]
> Bug 1432355 - Add a policy to disable private browsing.
> 
> https://reviewboard.mozilla.org/r/219140/#review227886
> 
> I ran some tests and disabling isn't want we want here.
> 
> A user can interact with Customize and drag the item to the overflow menu
> and then at that point it is disabled. 
> 
> That's a bad user experience.
> 
> We shouldn't have the button if a user can't use it.

Why not? We have lots of buttons that are disabled some of the time (or permanently disabled in permanent private browsing mode, IIRC).
> Anyway, half of my point is that I don't think that people who run a corporate network and who want to disable pb mode are going to put 2 and 2 together and realize they have to block devtools, and installing add-ons, and userChrome.css, and... I don't think expecting them to figure that out is reasonable - I think we should provide something out of the box that actually does what it says on the tin, without having to disable the other features.

Actually, that's exactly what an admin should know and do. What we're going on with a lot of these features is what has already been available to admins via the CCK2 and it has been used for many years without issue.

Eliminating the user's ability to access these features is what we are primarily going for.

We can put together longer term plans to harden these features but at this point our primary goal is parity with what was already available to admins.

> Why not? We have lots of buttons that are disabled some of the time (or permanently disabled in permanent private browsing mode, IIRC).

We're going to have a fundamental disagreement here. In setting policy on a computer, you don't give user options that they can't use. The fact that our UI for the customize dialog allow you to drag a button to the overflow menu that the user can't just use is simply broke. We shouldn't provide the button if a user can't use it.

That's how policy works. You don't typically disable it, you don't give the user even the option to think they can choose it.

And you only tell the user something is disabled if they accidentally went to it.

There's no assumption here that the user should know that private browsing was removed. They should never have to think about it even being there.
(In reply to Mike Kaply [:mkaply] from comment #24)
> > Why not? We have lots of buttons that are disabled some of the time (or permanently disabled in permanent private browsing mode, IIRC).
> 
> We're going to have a fundamental disagreement here. In setting policy on a
> computer,

OK, so your argument is that 'disabled by policy' is different from 'disabled'. That's fine, but that was far from obvious. It also quite obviously has different implications for customize mode. In customize mode, the 'subscribe' button is always disabled (customize mode doesn't have an rss feed). It's disabled a lot of the time! But not always, so the user should be able to put it somewhere.

> you don't give user options that they can't use.

This is the opposite of the decision for the add-ons manager, so we're not being very consistent...
> OK, so your argument is that 'disabled by policy' is different from 'disabled'.

I see where the terminology is a problem. "removed by policy" is a better term.

> This is the opposite of the decision for the add-ons manager, so we're not being very consistent...

I wasn't involved in that decision and I disagree with it :).

I'm going to do some investigation with Chrome to see if I can come up with a consistent policy.
I believe that Private Browsing is a good exception for the rule because leaving access to it might confuse users into thinking that they are using PB when they are not (which would be bad for privacy). Even if we display a warning message, they might overlook it. So it probably makes sense to remove the UI for getting into it to not give a false sense  of it being available.
Comment on attachment 8949829 [details]
Bug 1432355 - Add a policy to disable private browsing.

https://reviewboard.mozilla.org/r/219140/#review227916

::: browser/components/enterprisepolicies/Policies.jsm:103
(Diff revision 3)
> +  "DisablePrivateBrowsing": {
> +    onBeforeUIStartup(manager, param) {
> +      if (param == true) {
> +        manager.disallowFeature("privatebrowsing");
> +        manager.disallowFeature("about:privatebrowsing", true);
> +        setAndLockPref("browser.privatebrowsing.autostart", false);

I think it'd be safer to lock and set this pref earlier. I'm not sure there's anything wrong with doing it here now, but we reader user prefs earlier and run other code earlier, and so code that makes startup decisions about what data storage to use etc. should have the right information pretty much immediately.

::: browser/components/nsBrowserGlue.js:1230
(Diff revision 3)
>        LanguagePrompt.init();
>      });
> +
> +    if (!PrivateBrowsingUtils.enabled) {
> +      Services.tm.idleDispatchToMainThread(() => {
> +        CustomizableUI.destroyWidget("privatebrowsing-button");

Even if we want to hide this, then I think setting it to hidden from within CustomizableWidgets would be safer than removing it, and practically would have the same effect without ever changing the loading order of anything.

Alternatively, make adding the widget in CUIWidgets conditional on PBU.enabled.

::: browser/components/places/content/controller.js:1526
(Diff revision 3)
>    updatePlacesCommand("placesCmd_cut");
>    updatePlacesCommand("placesCmd_copy");
>    updatePlacesCommand("placesCmd_paste");
>    updatePlacesCommand("placesCmd_delete");
> +
> +  document.getElementById("placesCmd_open:privatewindow").hidden = !PrivateBrowsingUtils.enabled;

This should only have to happen once, so this is probably not the right place.

::: browser/components/preferences/in-content/privacy.js:483
(Diff revision 3)
> +      document.getElementById("privateBrowsingAutoStart").hidden = true;
> +      document.querySelector("menuitem[value='dontremember']").hidden = true;

Just from looking at the code, I think this still allows picking 'custom settings' and then unticking 'Remember my browsing and download history', 'Remember search and form history' etc. Is that really correct?
I checked on Chrome and the Incognito Mode menu is removed it is not allowed.

Preferences that are locked are disabled but remain (the same as what we do in most cases).

In this case, I did hide two items in preferences because they don't make sense if private browsing is enabled.

In the typical disabling of prefs, you're showing the user "You can't do this" - like set the homepage. 

I think a lot of these are on a case by case basis, but you generally don't want to give a user access to something if it's simply not going to work.

The goal of policy is to remove things that people don't need for their usecase.
Comment on attachment 8949829 [details]
Bug 1432355 - Add a policy to disable private browsing.

https://reviewboard.mozilla.org/r/219140/#review227916

> Just from looking at the code, I think this still allows picking 'custom settings' and then unticking 'Remember my browsing and download history', 'Remember search and form history' etc. Is that really correct?

Yes. This is only about disabling private browsing. A user is still free to clear their history, etc. Chrome doesn't have anything comparable to these items so we need to decide if we want a separate policy that covers these.
The prefs in question are places.history.enabled and browser.formfill.enable are the prefs in question.

In the CCK2 world, an admin would have simply locked those two prefs to what they wanted. We might want to consider a policy for these specifically.

Once we get the major policies done, we're going to look at policies for individual prefs. I'd rather do that than allowing folks to set/lock any pref.
Comment on attachment 8949829 [details]
Bug 1432355 - Add a policy to disable private browsing.

https://reviewboard.mozilla.org/r/219140/#review227940

This is missing:

- https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul#528-530
- Activity stream (https://dxr.mozilla.org/mozilla-central/source/browser/extensions/activity-stream/common/Actions.jsm#48 and others).

What happens with https://dxr.mozilla.org/mozilla-central/rev/861067332bac96a44bbf41ef366f58a30476057b/browser/components/nsBrowserContentHandler.js#465 ? I think we should probably avoid opening it (maybe open about:privatebrowsing instead which will show the 'your admin has disabled this' page?) rather than opening it in non-private-browsing which may leak details from the normal browsing profile that the user doesn't want to leak.

Likewise, what happens when an add-on calls `windows.create()` with the `incognito` flag set to true? Maybe the resulting promise should reject? Probably a good idea to talk to webextensions people about that.

::: browser/components/customizableui/CustomizableWidgets.jsm:940
(Diff revision 4)
>        let forgetButton = aEvent.target.querySelector("#PanelUI-panic-view-button");
>        forgetButton.removeEventListener("command", this);
>      },
>    });
> +
> +  if (PrivateBrowsingUtils.enabled) {

Uh, pretty sure this doesn't want to be inside the 
`if (Services.prefs.getBoolPref("privacy.panicButton.enabled"))`

pref check.
Attachment #8949829 - Flags: review?(gijskruitbosch+bugs)
To answer your extension question, Chrome fails in that situation. So an extension that opens an incognito window simply doesn't work.
Comment on attachment 8949829 [details]
Bug 1432355 - Add a policy to disable private browsing.

https://reviewboard.mozilla.org/r/219140/#review228238

::: browser/components/extensions/ext-windows.js:132
(Diff revision 5)
>              let incognito = PrivateBrowsingUtils.isBrowserPrivate(tab.linkedBrowser);
>              if (createData.incognito !== null && createData.incognito != incognito) {
>                return Promise.reject({message: "`incognito` property must match the incognito state of tab"});
>              }
> +            if (createData.incognito && !PrivateBrowsingUtils.enabled) {
> +              return Promise.reject({message: "`incognito` cannot be used if incognito mode is disabled"});

Might be worth checking with a webextension peer like bsilverberg or mixedpuppy that this is correct. Looks OK to me, but I'm not that familiar with webextensions.

::: browser/components/nsBrowserContentHandler.js:424
(Diff revision 5)
>        // NS_ERROR_INVALID_ARG is thrown when flag exists, but has no param.
> -      if (cmdLine.handleFlag("private-window", false)) {
> +      // We also use this to handle when private browsing is turned off
> +      if (cmdLine.handleFlag("private-window", false) ||
> +          !PrivateBrowsingUtils.enabled) {
>          openWindow(null, this.chromeURL, "_blank",
>            "chrome,dialog=no,private,all" + this.getFeatures(cmdLine),

This opens a private window still.

Also, I don't know how commandline parsing will be affected if we pretend that the -private-window arg didn't come with a flag. Will it try to parse the argument as a 'generic' argument then?

Also, related to the first point, this doesn't handle the case where people pass only -private-window without an arg - that should still not open a private window.

So instead, I would propose that inside the `if (privateWindowParam)` block in the try block, we use:

```js
let forcePrivate = true;
let resolvedURI;
if (!PrivateBrowsingUtils.enabled) {
  // Load about:privatebrowsing in a normal tab, which will display an error indicating
  // access to private browsing has been disabled.
  forcePrivate = false;
  resolvedURI = Services.io.newURI("about:privatebrowsing");
} else {
  // resolve URI as we do now;
}
handURIToExistingBrowser(resolvedURI, nsIBrowserDOMWindow.OPEN_NEWTAB, cmdLine, forcePrivate,
                         Services.scriptSecurityManager.getSystemPrincipal());
```

and in the catch block, make the `features` flag's inclusion of ',private' conditional on `PBU.enabled`. That should address all these concerns.

::: browser/extensions/activity-stream/lib/SectionsManager.jsm:202
(Diff revision 5)
>        options.contextMenuOptions = options.availableContextMenuOptions.filter(
>          o => !this.CONTEXT_MENU_PREFS[o] || Services.prefs.getBoolPref(this.CONTEXT_MENU_PREFS[o]));
> +      options.contextMenuOptions = options.availableContextMenuOptions.filter(
> +        o => o != "OpenInPrivateWindow" || PrivateBrowsingUtils.enabled);

This overwrites the previous assignment but doesn't use it (uses the `available` version a second time) so it's wrong (won't take the context menu prefs into account anymore).

Instead, make the arrow function passed to filter() have an actual block, and check all of the conditions. Note that you'll need an actual `return;` statement in that case. Maybe something like:

```js
options.contextMenuOptions = options.availableContextMenuOptions.filter(option => {
  let rv = !this.CONTEXT_MENU_PREFS[option] || Services.prefs.getBoolPref(this.CONTEXT_MENU_PREFS[option]);
  // Special-case the 'open in a private window' context menu
  if (rv && option == "OpenInPrivateWindow") {
    return PrivateBrowsingUtils.enabled;
  }
  return rv;
});
```
Attachment #8949829 - Flags: review?(gijskruitbosch+bugs)
Shane:

Can you take a look at the change to

browser/components/extensions/ext-windows.js

in the above patch and see if you're OK with it?

I verified that on Chrome if you disable incognito mode, web extensions that open private windows simply do nothing. I'm emulating that behavior on Firefox.
Flags: needinfo?(mixedpuppy)
Comment on attachment 8949829 [details]
Bug 1432355 - Add a policy to disable private browsing.

https://reviewboard.mozilla.org/r/219140/#review228582

LGTM, thanks!
Attachment #8949829 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to Mike Kaply [:mkaply] from comment #37)

> I verified that on Chrome if you disable incognito mode, web extensions that
> open private windows simply do nothing. I'm emulating that behavior on
> Firefox.

lgtm
Flags: needinfo?(mixedpuppy)
Note that the AS portion probably wants upstreaming to their github repo in addition to landing on m-c. I don't know exactly how that works in practice; if you're not sure either, asking Mardak, ursula or k88hudson would be a good idea.
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Updated the way I hid the places menuitem based on a test failure.
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s f0c95ae4a9c0672346ad7e9e05c2bc44f1e86d55 -d 109cd0a34ffe: rebasing 449001:f0c95ae4a9c0 "Bug 1432355 - Add a policy to disable private browsing. r=Gijs" (tip)
merging browser/base/content/browser.js
merging browser/base/content/nsContextMenu.js
merging browser/components/customizableui/CustomizableWidgets.jsm
merging browser/components/enterprisepolicies/Policies.jsm
merging browser/components/enterprisepolicies/schemas/policies-schema.json
merging browser/components/enterprisepolicies/tests/browser/browser.ini
merging browser/components/preferences/in-content/privacy.js
merging browser/components/syncedtabs/TabListView.js
merging browser/extensions/activity-stream/lib/SectionsManager.jsm
merging browser/modules/WindowsJumpLists.jsm
merging toolkit/modules/PrivateBrowsingUtils.jsm
warning: conflicts while merging browser/components/customizableui/CustomizableWidgets.jsm! (edit, then use 'hg resolve --mark')
warning: conflicts while merging browser/extensions/activity-stream/lib/SectionsManager.jsm! (edit, then use 'hg resolve --mark')
warning: conflicts while merging toolkit/modules/PrivateBrowsingUtils.jsm! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
So I thought I had this ready to go, but I'm blocked on one more case in Activity Stream (Top Sites).

I'm continuing to research.
See Also: → 1441984
After discussion with the Activity Stream folks, I am going to leave the AS specific code to them and check in everything except that.

So the code I'm checking in has already been reviewed.
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/520a7d1de634
Add a policy to disable private browsing. r=Gijs
Blocks: 1441984
https://hg.mozilla.org/mozilla-central/rev/520a7d1de634
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
We tested “disable private browsing” policy using JSON file.  Private browsing can be blocked by this policy and we verified this manually as fixed.

Test steps and runs are available here: https://testrail.stage.mozaws.net/index.php?/plans/view/7734

The bug will also be retested with ADMX files when it is ready for testing.
Depends on: 1442749
We retested this on beta builds[FX60] with ADM and JSON policy formats and it is verified as fixed.

Test cases and runs are here- https://testrail.stage.mozaws.net/index.php?/plans/view/8760
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.