1429120 - Policies: display menu bar by default, display bookmarks toolbar by default

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, enhancement, P1)

Description

[:Felipe Gomes (needinfo for replies!)]

Two separate policies:

  - display the menu bar by default (is this only relevant on Windows? and Linux)
  - display the bookmarks toolbar by default

I think it doesn't matter much for sysadmins if this is enforced, or just an option made different from the default (i.e., which still allows the user to change it).

Comment 1

[:Felipe Gomes (needinfo for replies!)]

FWIW, if you need to set prefs, use the function implemented by bug 1428923

Comment 2

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1429120 - Create an enterprise policy to display the menu bar by default

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

Comment 3

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1429120 - Add enterprise policy to display the bookmarks toolbar by default

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

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8943816 [details]
Bug 1429120 - Create an enterprise policy to display the menu bar by default

https://reviewboard.mozilla.org/r/214194/#review219902

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8943817 [details]
Bug 1429120 - Add enterprise policy to display the bookmarks toolbar by default

https://reviewboard.mozilla.org/r/214196/#review219904

Comment 6

[:Felipe Gomes (needinfo for replies!)]

r+ for all the policies-related code, but please request review from Gijs for the actual implementation code of these two policies

Comment 7

[:Gijs (he/him)]

Comment on attachment 8943817 [details]
Bug 1429120 - Add enterprise policy to display the bookmarks toolbar by default

https://reviewboard.mozilla.org/r/214196/#review220180

::: browser/components/enterprisepolicies/Policies.jsm:75
(Diff revision 1)
> +        // This policy is meant to change the default behavior, not to force it.
> +        // If this policy was alreay applied and the user chose to re-hide the
> +        // bookmarks toolbar, do not show it again.
> +        if (!Services.prefs.getBoolPref(PREF_BOOKMARKS_ALREADY_DISPLAYED, false)) {
> +          log.debug("Showing the bookmarks toolbar");
> +          let xulStore = Cc["@mozilla.org/xul/xulstore;1"].getService(Ci.nsIXULStore);

Use `XPCOMUtils.defineLazyServiceGetter` at the top of this file to define a lazy getter for `gXULStore` or something.

::: browser/components/enterprisepolicies/Policies.jsm:76
(Diff revision 1)
> +        // If this policy was alreay applied and the user chose to re-hide the
> +        // bookmarks toolbar, do not show it again.
> +        if (!Services.prefs.getBoolPref(PREF_BOOKMARKS_ALREADY_DISPLAYED, false)) {
> +          log.debug("Showing the bookmarks toolbar");
> +          let xulStore = Cc["@mozilla.org/xul/xulstore;1"].getService(Ci.nsIXULStore);
> +          let browserDocumentUrl = "chrome://browser/content/browser.xul";

Nit: const, and use ALL_CAPS. You're probably going to want this for other things so might as well stick it at the toplevel.

::: browser/components/enterprisepolicies/schemas/policies.json:23
(Diff revision 1)
>        "enum": [true]
>      },
>  
> +    "display_bookmarks_toolbar": {
> +      "description": "Causes the bookmarks toolbar to be displayed by default.",
> +      "first_available": "59.0",

I expect this will need to land with 60.0 unless we're uplifting.

::: browser/components/enterprisepolicies/tests/browser/browser_policy_display_bookmarks.js:1
(Diff revision 1)
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

Normally we license tests as public domain via CC0, unless there's a compelling reason not to:

```
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
```

Comment 8

[:Gijs (he/him)]

Comment on attachment 8943816 [details]
Bug 1429120 - Create an enterprise policy to display the menu bar by default

https://reviewboard.mozilla.org/r/214194/#review220184

r=me with similar changes to the other review (global the doc URL, gXULStore; separately: test licensing and what version this ships in)

::: browser/components/enterprisepolicies/Policies.jsm:55
(Diff revision 1)
> +          let xulStore = Cc["@mozilla.org/xul/xulstore;1"].getService(Ci.nsIXULStore);
> +          let browserDocumentUrl = "chrome://browser/content/browser.xul";

Same nits as in the other file, then you can omit this duplicate code.

::: browser/components/enterprisepolicies/schemas/policies.json:15
(Diff revision 1)
>        "enum": [true]
>      },
>  
> +    "display_menu_bar": {
> +      "description": "Causes the menu bar to be displayed by default.",
> +      "first_available": "59.0",

Maybe 60 too?

Comment 9

[Robin Steuber (she/her) [:bytesized]]

Comment on attachment 8943816 [details]
Bug 1429120 - Create an enterprise policy to display the menu bar by default

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214194/diff/1-2/

Comment 10

[Robin Steuber (she/her) [:bytesized]]

Comment on attachment 8943817 [details]
Bug 1429120 - Add enterprise policy to display the bookmarks toolbar by default

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214196/diff/1-2/

Comment 11

[Robin Steuber (she/her) [:bytesized]]

Comment on attachment 8943816 [details]
Bug 1429120 - Create an enterprise policy to display the menu bar by default

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214194/diff/2-3/

Comment 12

[Robin Steuber (she/her) [:bytesized]]

Comment on attachment 8943817 [details]
Bug 1429120 - Add enterprise policy to display the bookmarks toolbar by default

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214196/diff/2-3/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc33122dc2fa
Create an enterprise policy to display the menu bar by default r=Felipe,Gijs
https://hg.mozilla.org/integration/autoland/rev/95506a51e4ab
Add enterprise policy to display the bookmarks toolbar by default r=Felipe,Gijs

Comment 14

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/dc33122dc2fa
https://hg.mozilla.org/mozilla-central/rev/95506a51e4ab

Comment 15

[Abe - QA (:Abe_LV)]

We tested "display menu bar and bookmarks toolbar by default" policy using JSON file. We verified this policy implementation as fixed, but still, it will be retested with ADMX files when it is ready for testing.

Test documentation is available here: https://testrail.stage.mozaws.net/index.php?/plans/view/7734

Comment 16

[Abe - QA (:Abe_LV)]

We retested this on beta builds with ADM and JSON policy formats and it is verified as fixed.