mozilla / policy-templates

Policy Templates for Firefox
Mozilla Public License 2.0
1.12k stars 398 forks source link

About `runOncePerModification` policies #1110

Open qupig opened 1 month ago

qupig commented 1 month ago

I'm not sure it's really easy for people to understand policies that use the runOncePerModification method.

At least not for me, I thought they were just changing the default value, but they're not.

They don't actually modify the default values, but substitute the user make one-time settings.

In other words, I would expected it to set the default value for any new profiles or the profiles with unmodified corresponding pref, rather than overwriting and modifying all profiles in one go.

I think the runOncePerModification has its value, but may not be transparent and expected to policy makers.

For example: DisplayBookmarksToolbar

  DisplayBookmarksToolbar: {
    onBeforeUIStartup(manager, param) {
      let visibility;
      if (typeof param === "boolean") {
        visibility = param ? "always" : "newtab";
      } else {
        visibility = param;
      }
      // This policy is meant to change the default behavior, not to force it.
      // If this policy was already applied and the user chose to re-hide the
      // bookmarks toolbar, do not show it again.
      runOncePerModification("displayBookmarksToolbar", visibility, () => {
        let visibilityPref = "browser.toolbars.bookmarks.visibility";
        Services.prefs.setCharPref(visibilityPref, visibility);
      });
    },
  },

When a pref intends to change the default value and respect user preferences: (Note: the following is only an analogy, not an actual execution method)

Actually runOncePerModification behaves more like: 👎👎👎

But I think the more common expectations are probably: 👍👍👍

Or, If you want to correct the pref changed before once: 👎

And this will not generate additional prefs like browser.policies.runOncePerModification.${actionName}

However, this approach is also discouraged because it is also impossible to distinguish whether it was previously set through policy or modified by the user.

I think the real need might be clearOnceSinceUnixTime (Also NOT recommended): 👎

Sometimes administrators have to reset preferences once, whether it's from an old policy or a user change.

But always keep in mind that once you want to respect user changes, you should not accidentally reset a preference. This is a very bad experience for users who will find that their changes have been reverted inexplicably.

mkaply commented 1 month ago

Yeah I was never a fan of runOncePerModification, but we couldn't come up with better ideas.

It definitely doesn't affect all profiles though. It only affects the profile is was loaded in.

clearOnce is an interesting idea.

qupig commented 1 month ago

It definitely doesn't affect all profiles though. It only affects the profile is was loaded in.

Unless you never use the profiles while the policy is in effect. And the policy is intended to be applied to every profile. But I really didn't express this rigorously.