1895932 - Add sidebar prefs to nimbus

Status: RESOLVED FIXED

(Firefox :: Sidebar, task, P2)

Description

[Sarah Clements [:sclements]]

We'll need to add the sidebar.revamp and vertical tabs prefs to the nimbus featureManifest.yaml for the release experiment.

Comment 1

[Ania]

sidebar - feature
sidebarRevamp: variable, bool
sidebarVerticalTabs: variable, bool
sidebarDefaultTools: variable, json. Values could be "history", "syncedtabs", and "aichat".
See more in the sidebar probes sheet (experimentation section)
Feature manifest

Comment 2

[Sarah Clements [:sclements]]

(In reply to Ania from comment #1)

sidebarDefaultTools: variable, json. Values could be "history", "syncedtabs", and "aichat".

For this, we should create a new pref that is nimbus controlled and use that nimbus variable as the default value https://searchfox.org/mozilla-central/rev/b3c85ac11d004fdb582577cd8f674efa44b0e253/browser/components/sidebar/browser-sidebar.js#1239 for sidebar.main.tools. I think we'll also need to remove some of the code Ed added the only shows the AI chat bot if browser.ml.chat.enabled is true.

Comment 3

[Ed Lee :Mardak]

With bug 1909986, we'll generally prevent other chatbot entrypoints in the revamp experience when aichat isn't a tool. This should mean "treatment e" can be configured to enable chatbot but excluded from default tool (presumably sidebar.main.tools remote default pref value without aichat) while still showing up in customize. Maybe something like…

sidebar:
  variables:
    defaultTools:
      description: …
      type: string
      setPref:
        branch: default
        pref: "sidebar.main.tools"

This also means other treatment branches (or even outside of an experiment) will behave more expected if the default aichat tool is later customized to not be listed.

Comment 4

[Ed Lee :Mardak]

Actually, my previous comment probably won't work as is. It seems like we probably shouldn't be using nimbus pref setting functionality with any of our existing prefs that are exposed in UI, so that includes those from firefox labs or sidebar customize (vertical, tools) because Nimbus behavior is to unenroll users on pref changes independent of default or user branch. https://experimenter.info/desktop-pref-experiments/#user-preference-changes

Looks like review checker works around this nimbus unenrolling with a separate userEnabled pref from the not-user-facing feature flag https://searchfox.org/mozilla-central/search?q=userEnabled&path=shopping but given that we already have various UI for our prefs, I'm not sure if that's the correct pattern. I'll see if there's a better way to handle

Comment 5

[Ed Lee :Mardak]

We'll do something similar to bug 1910509 but for sidebar probably from browser-sidebar per window as there's currently no service/sys.mjs singleton. Chatbot already checks to avoid repeat application of user prefs and setting default pref multiple times should be fine: https://searchfox.org/mozilla-central/rev/e968519d806b140c402c3b3932cd5f6cd7cc42ac/browser/components/genai/GenAI.sys.mjs#217

Comment 6

[:Gijs (out for now; he/him)]

(In reply to Ed Lee :Mardak from comment #5)

We'll do something similar to bug 1910509 but for sidebar probably from browser-sidebar per window as there's currently no service/sys.mjs singleton. Chatbot already checks to avoid repeat application of user prefs and setting default pref multiple times should be fine: https://searchfox.org/mozilla-central/rev/e968519d806b140c402c3b3932cd5f6cd7cc42ac/browser/components/genai/GenAI.sys.mjs#217

Could we not create a singleton?

The longer-term plan is for most of the browser window stuff to not live in these per-window scripts anyway. Certainly a singleton for access to "is this feature enabled" seems like a sensible idea vs. creating observers etc. etc. for each window.

Comment 7

[Ed Lee :Mardak]

sclements did mention there's existing discussions (with sfoster?) around a singleton that could be happening in parallel -- which bug? existing patches? I could indeed go ahead and create that as part of this bug and have other patches rebase. This would be cleaner not per-window.

Comment 8

[Ed Lee :Mardak]

Attachment: Bug 1895932 - Add sidebar prefs to nimbus r=#sidebar-reviewers!

Add new Sidebar singleton with startup init that allows setting user and default branch prefs specified in the nimbus variable for sidebar.

Comment 9

[Ed Lee :Mardak]

looks like i'll be first!

We hadn't started the singleton yet, just talked about it in our team meeting yesterday so that's great that you'll be getting this started for us.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9a32760feaa
Add sidebar prefs to nimbus r=sidebar-reviewers,Gijs,sclements

Comment 11

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/d9a32760feaa