1447345 - Change enterprise policy that sets homepage to set it as the default rather than the user value for the pref

Status: RESOLVED FIXED

(Firefox :: Enterprise Policies, defect)

Description

[Robin Steuber (they/them) [:bytesized]]

It makes more sense for a homepage that is set by policy (and not locked) to be the default homepage. This would make it so that restoring the default value would restore the value that is the default for that company rather than the default for Firefox.

Comment 1

[Robin Steuber (they/them) [:bytesized]]

Attachment: Bug 1447345 - Change enterprise policy that sets homepage to set it as the default rather than the user value for the pref

Additionally removes a stray debugging line that I accidentally checked into the tree previously.

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

Comment 2

[:Felipe Gomes (needinfo for replies!)]

[Tracking Requested - why for this release]:
Enterprise Policies

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8960692 [details]
Bug 1447345 - Change enterprise policy that sets homepage to set it as the default rather than the user value for the pref

https://reviewboard.mozilla.org/r/229446/#review235180

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1fce5707743a
Change enterprise policy that sets homepage to set it as the default rather than the user value for the pref r=Felipe

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1fce5707743a

Comment 6

[Robin Steuber (they/them) [:bytesized]]

Comment on attachment 8960692 [details]
Bug 1447345 - Change enterprise policy that sets homepage to set it as the default rather than the user value for the pref

Approval Request Comment
[Feature/Bug causing the regression]: Enterprise Policy Manager
[User impact if declined]: Default homepage will be set to Firefox's default rather than the organization's default.
[Is this code covered by automated tests?]: Setting homepage by policy is covered by tests
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Will be tested by Abe
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Slightly changes the behavior of the homepage policy. Will not affect users that do not use the policy engine or that policy.
[String changes made/needed]:

Comment 7

[Mike Kaply [:mkaply]]

Actually we forgot something about this.

Because homepage is a complex pref, you have to set it as default differently.

See:

https://mike.kaply.com/2012/08/29/setting-the-default-firefox-homepage-with-autoconfig/

The policy is broken right now:

[Exception... "Component returned failure code: 0x80004004 (NS_ERROR_ABORT) [nsIPrefBranch.getComplexValue]"  nsresult: "0x80004004 (NS_ERROR_ABORT)"  location: "JS frame :: jar:file:///C:/Program%20Files/Nightly/64bits/testNightly/newer/coffee2/browser/omni.ja!/components/nsBrowserContentHandler.js :: get startPage :: line 592"  data: no]

this should be the only pref we have to do this weirdness with.

Comment 8

[:Felipe Gomes (needinfo for replies!)]

Hmm that's a very strange, how does the Locked case worked?  And did the previous version (before this change) worked too?

If so, I think we should just wontfix this bug and let it be as initial way. One less thing to uplift :)

Comment 9

[Mike Kaply [:mkaply]]

Before he was setting the user pref, but my guess is locking it was broke as well.

But I agree with Kirk. We should be setting the default value.

Comment 10

[Robin Steuber (they/them) [:bytesized]]

Gah. Since this patch has merged to mozilla-central but not mozilla-beta, it puts me in a weird state for fixing it up. I don't really want to update the patch since merging it to mozilla-central wouldn't work (as the current version has already merged.

So I want to write the fix as a second patch. But I want to rebase because of changes to the homepage UI in about:preferences that just merged. And I cannot rebase the current patch on the tip of mozilla-central since it has already merged to mozilla-central. But if I write my patch without rebasing the current patch, mozreview will get rid of the current patch when I upload the new one.

I think it is simplest if I just file a new bug for the fix.

Comment 11

[Robin Steuber (they/them) [:bytesized]]

Comment on attachment 8960692 [details]
Bug 1447345 - Change enterprise policy that sets homepage to set it as the default rather than the user value for the pref

Approval Request Comment
[Feature/Bug causing the regression]: Enterprise Policy Engine
[User impact if declined]: Setting homepage via policy will set the page once, but resetting to default will reset to the Firefox default rather than the organization default.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: To be tested by Abe
[List of other uplifts needed for the feature/fix]: The patch from Bug 1448085 must be merged after this patch
[Is the change risky?]: Minimal Risk
[Why is the change risky/not risky?]: Makes a minor change to the way that the enterprise policy works. Should not affect users not using enterprise policies
[String changes made/needed]: None

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 8960692 [details]
Bug 1447345 - Change enterprise policy that sets homepage to set it as the default rather than the user value for the pref

enterprise policy fix, approved for 60.0b8

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0e8ea2c1220a