1429141 - Policy: Don't check if Firefox is the default browser on startup

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, defect, P1)

Description

[:Felipe Gomes (needinfo for replies!)]

Don't check if Firefox is the default browser on startup

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Attachment: Bug 1429141 - Policy: Don't check if Firefox is the default browser on startup.

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

Comment 2

[:Felipe Gomes (needinfo for replies!)]

So there were three ways to implement this:

- Calling ShellService.shouldCheckDefaultBrowser = false on startup
- Making the getter call Services.policies.isAllowed
- Locking the pref

I chose the locking the pref method because about:preferences already correctly reflects this by graying out the checkbox. The other two methods would give the users the impression of being able to change the setting but it would never take effect, so we would need to do extra work to fix that.

Comment 3

[:Gijs (he/him)]

Comment on attachment 8943990 [details]
Bug 1429141 - Policy: Don't check if Firefox is the default browser on startup.

https://reviewboard.mozilla.org/r/214316/#review220186

::: browser/components/enterprisepolicies/tests/browser/browser_policy_default_browser_check.js:6
(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/. */
> +
> +"use strict";
> +Cu.import("resource:///modules/ShellService.jsm");

I've seen other people complain that this adds global variable warnings. YMMV; if necessary use:

```js
const {ShellService} = ...;
```
to fix.

::: browser/components/enterprisepolicies/tests/browser/browser_policy_default_browser_check.js:16
(Diff revision 1)
> +  // testing profile. Let's start with true for a sanity check.
> +
> +  ShellService.shouldCheckDefaultBrowser = true;
> +  is(ShellService.shouldCheckDefaultBrowser, true, "Sanity check");
> +
> +  await setupPolicyEngineWithJson("config_dont_check_default_browser.json");

Given the long list of desired policies, I'm tempted to suggest that we want a helper that just takes a JS object instead of a separate file, otherwise the number of files required here will just keep ballooning.

Don't need to fix that here, but might be worth doing in a followup "soon"-ish so that we nip it in the bud before we have 50 of these files with 2 lines of relevant JSON and 3 lines of curly braces ;-)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c67fb9e69640
Policy: Don't check if Firefox is the default browser on startup. r=Gijs

Comment 5

[:Felipe Gomes (needinfo for replies!)]

(In reply to :Gijs (lower availability until Jan 27) from comment #3)
> Comment on attachment 8943990 [details]
> Bug 1429141 - Policy: Don't check if Firefox is the default browser on
> startup.
> 
> https://reviewboard.mozilla.org/r/214316/#review220186
> 
> :::
> browser/components/enterprisepolicies/tests/browser/
> browser_policy_default_browser_check.js:6
> (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/. */
> > +
> > +"use strict";
> > +Cu.import("resource:///modules/ShellService.jsm");
> 
> I've seen other people complain that this adds global variable warnings.
> YMMV; if necessary use:
> 
> ```js
> const {ShellService} = ...;
> ```
> to fix.

Done 

> 
> Don't need to fix that here, but might be worth doing in a followup
> "soon"-ish so that we nip it in the bud before we have 50 of these files
> with 2 lines of relevant JSON and 3 lines of curly braces ;-)

Good idea. I'll do that in a follow-up soon. I'll just make the test code write a temp file with a given JSON from the test and set the browser.policies.alternatePath to that file

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/87defcccf49a
Fix test when it runs twice in a row, e.g. on test-verify. r=me

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/c67fb9e69640
https://hg.mozilla.org/mozilla-central/rev/87defcccf49a

Comment 8

[Abe - QA (:Abe_LV)]

We have tested this on latest Nightly-60.0a1 [build that have this patch] and it does not check if Nighty is the default browser when this policy is in use. 
 
Test cases and runs are here: https://testrail.stage.mozaws.net/index.php?/plans/view/7734

The following operating systems are used for testing:
Windows 7 x86, Windows 10 x64, OSX 10.12 and Ubuntu 16.04 64bits.

Comment 9

[Abe - QA (:Abe_LV)]

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