Details
- Reviewers
mstriemer nordzilla - Commits
- rFIREFOXAUTOLAND416182a3b5d9: Bug 2013938 - Update policies to work correctly with AI kill switch. r=nordzilla
- Bugzilla Bug ID
- 2013938
Diff Detail
- Repository
- rFIREFOXAUTOLAND firefox-autoland
Event Timeline
Thanks so much Mike!
I started to do a bit of manual testing with this, and I noticed some behavior that we need to clarify.
Test Case
policies.json
{ "policies": { "GenerativeAI": { "Enabled": true, "Locked": true } } }
Before Toggling
All of this looks to be what I expect. The features are all Available/Enabled, and "Locked," meaning the UI cannot be changed.
After Toggling
What is the expected behavior here?
The current implementation does this:
The main toggle still moves the features to "Blocked" even though the dropdowns are disabled and the prefs are supposed to be "Locked".
I would expect the behavior to be more like this:
Where even though we've turned the switch on, these prefs are managed by their policy and cannot be mutated.
In order to achieve the latter behavior I described, I think that we will need the policy to set and lock the ai.conrol prefs for each feature, too.
Something like this:
diff --git a/browser/components/enterprisepolicies/Policies.sys.mjs b/browser/components/enterprisepolicies/Policies.sys.mjs index 2a6d3b297ff9..e355af7ecde2 100644 --- a/browser/components/enterprisepolicies/Policies.sys.mjs +++ b/browser/components/enterprisepolicies/Policies.sys.mjs @@ -1674,14 +1674,34 @@ export var Policies = { const defaultValue = "Enabled" in param ? param.Enabled : undefined; const features = [ - ["Chatbot", ["browser.ml.chat.enabled", "browser.ml.chat.page"]], - ["LinkPreviews", ["browser.ml.linkPreview.optin"]], - ["TabGroups", ["browser.tabs.groups.smart.userEnabled"]], - ["Translations", ["browser.translations.enable"]], - ["PDFAltText", ["pdfjs.enableAltText"]], + [ + "Chatbot", + ["browser.ml.chat.enabled", "browser.ml.chat.page"], + "browser.ai.control.sidebarChatbot", + ], + [ + "LinkPreviews", + ["browser.ml.linkPreview.optin"], + "browser.ai.control.linkPreviewKeyPoints", + ], + [ + "TabGroups", + ["browser.tabs.groups.smart.userEnabled"], + "browser.ai.control.smartTabGroups", + ], + [ + "Translations", + ["browser.translations.enable"], + "browser.ai.control.translations", + ], + [ + "PDFAltText", + ["pdfjs.enableAltText"], + "browser.ai.control.pdfjsAltText", + ], ]; - for (const [key, prefs] of features) { + for (const [key, prefs, aiControlPref] of features) { if (key == "Translations") { if (!(key in param)) { // If the TranslateEnabled policy is being used, don't change @@ -1696,6 +1716,11 @@ export var Policies = { for (const pref of prefs) { PoliciesUtils.setDefaultPref(pref, value, param.Locked); } + PoliciesUtils.setDefaultPref( + aiControlPref, + value ? "enabled" : "blocked", + param.Locked + ); } } },
This is what TranslateEnabled currently does. If that's not correct, then I need to change TranslateEnabled, which I previously tested under these expectations that even the switch could not mutate the dropdown menulists while "Locked", for example:
I'm also seeing an issue in the console logs, which looks to be a preexisting issue with TranslateEnabled since this is happening on the main branch with only the TranslateEnabled policy.
JavaScript error: resource://gre/modules/ActorManagerParent.sys.mjs, line 830: NotSupportedError: ChromeUtils.registerWindowActor: 'Translations' actor is already registered. JavaScript error: resource://gre/modules/ActorManagerParent.sys.mjs, line 830: NotSupportedError: ChromeUtils.registerProcessActor: 'TranslationsEngine' actor is already registered.
Depending on how involved the fix may be involving ActorManagerParent, we may not want to risk it in uplift, if it is already a long-standing issue that went unnoticed.
I haven't looked into it yet, but can look into it tomorrow.
| browser/components/enterprisepolicies/Policies.sys.mjs | ||
|---|---|---|
| 1699 |
Thank you for considering the TranslateEnabled policy independently. | |
I dug more into these errors this morning:
JavaScript error: resource://gre/modules/ActorManagerParent.sys.mjs, line 830: NotSupportedError: ChromeUtils.registerWindowActor: 'Translations' actor is already registered. JavaScript error: resource://gre/modules/ActorManagerParent.sys.mjs, line 830: NotSupportedError: ChromeUtils.registerProcessActor: 'TranslationsEngine' actor is already registered.
I tried a build back from June 2025, and this is a long-standing thing if someone attempts to set a pref value to true via policy, when the default is already true.
It causes the actor to be registered twice.
I've filed a bug and put up one attempt at a fix for review, but I'm not sure if we'll want to uplift into beta at this point, especially since this isn't on the primary UX path.
| browser/components/enterprisepolicies/schemas/policies-schema.json | ||
|---|---|---|
| 893–894 ↗ | (On Diff #1196982) | It looks like "Translations" accidentally got left out of the cleanup. |
I think this looks great, Mike, barring my one new inline comment.
Thanks so much!
I'm just going to do a bit of manual testing, and will likely approve before the end of my work day.
| browser/components/enterprisepolicies/schemas/policies-schema.json | ||
|---|---|---|
| 893–894 ↗ | (On Diff #1196982) | Update: it's not just that "Translations" wasn't removed, but that it accidentally replaced "Locked". (I didn't notice this until I started my manual testing.) |
Manual Testing
Done with the inline fix applied:
diff --git a/browser/components/enterprisepolicies/schemas/policies-schema.json b/browser/components/enterprisepolicies/schemas/policies-schema.json index 8547f35646f9..426a9fc0d388 100644 --- a/browser/components/enterprisepolicies/schemas/policies-schema.json +++ b/browser/components/enterprisepolicies/schemas/policies-schema.json @@ -890,7 +890,7 @@ "Enabled": { "type": "boolean" }, - "Translations": { + "Locked": { "type": "boolean" } }
Case 1: Default enabled, not locked
{ "policies": { "GenerativeAI": { "Enabled": true, "Locked": false } } }
Switch Off
{F57565992}
Switch On
{F57566015}
Case 2: Default disabled, not locked
{ "policies": { "GenerativeAI": { "Enabled": false, "Locked": false } } }
Switch Off
{F57566207}
Switch On
{F57566234}
Case 3: Default enabled, locked
{ "policies": { "GenerativeAI": { "Enabled": true, "Locked": true } } }
Switch Off
Switch On
Case 4: Default disabled, locked
{ "policies": { "GenerativeAI": { "Enabled": false, "Locked": true } } }
Switch Off
Switch On
Case 5: Default enabled, locked, with per-feature overrides
{ "policies": { "GenerativeAI": { "Enabled": true, "Chatbot": false, "LinkPreviews": true, "TabGroups": false, "Locked": true } } }
Switch Off
Switch On
Case 6: Default disabled, locked, with per-feature overrides
{ "policies": { "GenerativeAI": { "Enabled": false, "Chatbot": true, "LinkPreviews": false, "TabGroups": true, "Locked": true } } }
Switch Off
Switch On
Case 7: Default disabled, locked, chatbot enabled, with TranslateEnabled
{ "policies": { "GenerativeAI": { "Enabled": false, "Chatbot": true, "Locked": true }, "TranslateEnabled": true } }
Switch Off
Switch On
Conclusion
Manual testing looks correct to me, with the inline fix applied before landing.
r+
:
- This has undergone extensive manual testing, but has no automated tests at this time.
| browser/components/enterprisepolicies/schemas/policies-schema.json | ||
|---|---|---|
| 893–894 ↗ | (On Diff #1196982) | That's what I get for rushing. Thanks! |
