Bug 1963764 - read the correct enterprise policy when deciding whether to hide Report Broken Site (also improve the related tests); r=Gijs

Differential Revision: https://phabricator.services.mozilla.com/D260014

Author: wisniewskit

browser/components/reportbrokensite/ReportBrokenSite.sys.mjs

@@ -455,9 +455,7 @@ export var ReportBrokenSite = new (class ReportBrokenSite {
     // Altering the disabled attribute on the command does not propagate
     // the change to the related menuitems (see bug 805653), so we change them all.
     const cmd = document.getElementById("cmd_reportBrokenSite");
-    const allowedByPolicy = Services.policies.isAllowed(
-      "DisableFeedbackCommands"
-    );
+    const allowedByPolicy = Services.policies.isAllowed("feedbackCommands");
     cmd.toggleAttribute("hidden", !allowedByPolicy);
     const app = document.ownerGlobal.PanelMultiView.getViewNode(
       document,

browser/components/reportbrokensite/test/browser/browser_parent_menuitems.js

@@ -16,81 +16,68 @@ add_task(async function testMenus() {
   ensureReportBrokenSitePreffedOff();
 
   const appMenu = AppMenu();
-  const menus = [appMenu, ProtectionsPanel(), HelpMenu()];
+  const helpMenu = HelpMenu();
+  const protectionsPanel = ProtectionsPanel();
 
-  async function forceMenuItemStateUpdate() {
-    ReportBrokenSite.enableOrDisableMenuitems(window);
-
-    // the hidden/disabled state of all of the menuitems may not update until one
-    // is rendered; then the related <command>'s state is propagated to them all.
-    await appMenu.open();
-    await appMenu.close();
+  async function ensure(menu, fn, test) {
+    await menu.open();
+    fn(menu.reportBrokenSite, `${fn.name}(${menu.menuDescription}) ${test} - `);
+    await menu.close();
   }
 
   await BrowserTestUtils.withNewTab("about:blank", async function () {
-    await forceMenuItemStateUpdate();
-    for (const { menuDescription, reportBrokenSite } of menus) {
-      isMenuItemHidden(
-        reportBrokenSite,
-        `${menuDescription} option hidden on invalid page when preffed off`
-      );
-    }
+    ReportBrokenSite.enableOrDisableMenuitems(window);
+    const test = "on invalid page when preffed off";
+    await ensure(appMenu, isMenuItemDisabled, test);
+    await ensure(helpMenu, isMenuItemDisabled, test);
+    ensureProtectionsPanelHidden(test);
   });
 
   await BrowserTestUtils.withNewTab(REPORTABLE_PAGE_URL, async function () {
-    await forceMenuItemStateUpdate();
-    for (const { menuDescription, reportBrokenSite } of menus) {
-      isMenuItemHidden(
-        reportBrokenSite,
-        `${menuDescription} option hidden on valid page when preffed off`
-      );
-    }
+    ReportBrokenSite.enableOrDisableMenuitems(window);
+    const test =
+      "on valid page when preffed off (fallback to original reporter)";
+    await ensure(appMenu, isMenuItemEnabled, test);
+    await ensure(helpMenu, isMenuItemEnabled, test);
+    await ensure(protectionsPanel, isMenuItemEnabled, test);
   });
 
   ensureReportBrokenSitePreffedOn();
 
   await BrowserTestUtils.withNewTab("about:blank", async function () {
-    await forceMenuItemStateUpdate();
-    for (const { menuDescription, reportBrokenSite } of menus) {
-      isMenuItemDisabled(
-        reportBrokenSite,
-        `${menuDescription} option disabled on invalid page when preffed on`
-      );
-    }
+    ReportBrokenSite.enableOrDisableMenuitems(window);
+    const test = "on invalid page when preffed on";
+    await ensure(appMenu, isMenuItemDisabled, test);
+    await ensure(helpMenu, isMenuItemDisabled, test);
+    ensureProtectionsPanelHidden(test);
   });
 
   await BrowserTestUtils.withNewTab(REPORTABLE_PAGE_URL, async function () {
-    await forceMenuItemStateUpdate();
-    for (const { menuDescription, reportBrokenSite } of menus) {
-      isMenuItemEnabled(
-        reportBrokenSite,
-        `${menuDescription} option enabled on valid page when preffed on`
-      );
-    }
+    ReportBrokenSite.enableOrDisableMenuitems(window);
+    const test = "on valid page when preffed on";
+    await ensure(appMenu, isMenuItemEnabled, test);
+    await ensure(helpMenu, isMenuItemEnabled, test);
+    await ensure(protectionsPanel, isMenuItemEnabled, test);
   });
 
   ensureReportBrokenSitePreffedOff();
 
   await BrowserTestUtils.withNewTab(REPORTABLE_PAGE_URL, async function () {
-    await forceMenuItemStateUpdate();
-    for (const { menuDescription, reportBrokenSite } of menus) {
-      isMenuItemHidden(
-        reportBrokenSite,
-        `${menuDescription} option hidden again when pref toggled back off`
-      );
-    }
+    ReportBrokenSite.enableOrDisableMenuitems(window);
+    const test = "still active when pref toggled back off";
+    await ensure(appMenu, isMenuItemEnabled, test);
+    await ensure(helpMenu, isMenuItemEnabled, test);
+    await ensure(protectionsPanel, isMenuItemEnabled, test);
   });
 
   ensureReportBrokenSitePreffedOn();
   ensureReportBrokenSiteDisabledByPolicy();
 
   await BrowserTestUtils.withNewTab(REPORTABLE_PAGE_URL, async function () {
-    await forceMenuItemStateUpdate();
-    for (const { menuDescription, reportBrokenSite } of menus) {
-      isMenuItemHidden(
-        reportBrokenSite,
-        `${menuDescription} option hidden when disabled by DisableFeedbackCommands enterprise policy`
-      );
-    }
+    ReportBrokenSite.enableOrDisableMenuitems(window);
+    const test = "when disabled by DisableFeedbackCommands enterprise policy";
+    await ensure(appMenu, isMenuItemHidden, test);
+    await ensure(helpMenu, isMenuItemHidden, test);
+    await ensure(protectionsPanel, isMenuItemHidden, test);
   });
 });

browser/components/reportbrokensite/test/browser/head.js

@@ -210,6 +210,13 @@ function ensureReasonRequired() {
   Services.prefs.setIntPref(PREFS.REASON, 2);
 }
 
+function ensureProtectionsPanelHidden(test) {
+  const { hidden } = document.getElementById(
+    "tracking-protection-icon-container"
+  );
+  ok(hidden, `Protections panel disabled ${test}`);
+}
+
 function isMenuItemEnabled(menuItem, itemDesc) {
   ok(!menuItem.hidden, `${itemDesc} menu item is shown`);
   ok(!menuItem.disabled, `${itemDesc} menu item is enabled`);