Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

About runOncePerModification policies #1110

Open
qupig opened this issue May 19, 2024 · 2 comments
Open

About runOncePerModification policies #1110

qupig opened this issue May 19, 2024 · 2 comments

Comments

@qupig
Copy link
Contributor

qupig commented May 19, 2024

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: 👎👎👎

  • Set "browser.toolbars.bookmarks.visibility": { Status: "user", Value: "always|never|newtab" }
  • Run all profiles
  • Delete the first step policy

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

  • Set "browser.toolbars.bookmarks.visibility": { Status: "default", Value: "always|never|newtab" }

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

  • Set "browser.toolbars.bookmarks.visibility": { Status: "clear"}
  • Run all profiles
  • Set "browser.toolbars.bookmarks.visibility": { Status: "default", Value: "always|never|newtab" }

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): 👎

  • Whether in a separate policy or in the Preferences policy object

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
Copy link
Collaborator

mkaply commented May 20, 2024

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
Copy link
Contributor Author

qupig commented May 20, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants