From 9abf802bc6dd8195052a2b5b3d0732d26137a87e Mon Sep 17 00:00:00 2001
From: Henry Wilkes <henry@torproject.org>
Date: Wed, 4 Oct 2023 19:16:56 +0100
Subject: [PATCH] Bug 41736: Hide NoScript extension's toolbar button by
 default.

This hides it from both the toolbar and the unified extensions panel.

We also hide the unified-extension-button if the panel would be empty:
not including the NoScript button when it is hidden. As a result, this
will be hidden by default until a user installs another extension (or
shows the NoScript button and unpins it).
---
 browser/base/content/browser-addons.js        | 69 ++++++++++++++++-
 .../extensions/parent/ext-browserAction.js    | 16 ++++
 .../shared/addons/unified-extensions.css      | 18 +++++
 .../extensions/content/aboutaddons.html       | 26 +++++++
 .../mozapps/extensions/content/aboutaddons.js | 77 +++++++++++++++++++
 5 files changed, 205 insertions(+), 1 deletion(-)

diff --git a/browser/base/content/browser-addons.js b/browser/base/content/browser-addons.js
index 708f3af68f184..4bf0c9bc5ea33 100644
--- a/browser/base/content/browser-addons.js
+++ b/browser/base/content/browser-addons.js
@@ -26,6 +26,9 @@ XPCOMUtils.defineLazyGetter(lazy, "l10n", function () {
   );
 });
 
+const HIDE_NO_SCRIPT_PREF = "extensions.hideNoScript";
+const HIDE_UNIFIED_WHEN_EMPTY_PREF = "extensions.hideUnifiedWhenEmpty";
+
 /**
  * Mapping of error code -> [error-id, local-error-id]
  *
@@ -1186,6 +1189,18 @@ var gUnifiedExtensions = {
     gNavToolbox.addEventListener("customizationstarting", this);
     CustomizableUI.addListener(this);
 
+    // Listen out for changes in extensions.hideNoScript and
+    // extension.hideUnifiedWhenEmpty, which can effect the visibility of the
+    // unified-extensions-button.
+    // See tor-browser#41581.
+    this._hideNoScriptObserver = () => this._updateVisibility();
+    Services.prefs.addObserver(HIDE_NO_SCRIPT_PREF, this._hideNoScriptObserver);
+    Services.prefs.addObserver(
+      HIDE_UNIFIED_WHEN_EMPTY_PREF,
+      this._hideNoScriptObserver
+    );
+    this._updateVisibility();
+
     this._initialized = true;
   },
 
@@ -1201,6 +1216,15 @@ var gUnifiedExtensions = {
 
     gNavToolbox.removeEventListener("customizationstarting", this);
     CustomizableUI.removeListener(this);
+
+    Services.prefs.removeObserver(
+      HIDE_NO_SCRIPT_PREF,
+      this._hideNoScriptObserver
+    );
+    Services.prefs.removeObserver(
+      HIDE_UNIFIED_WHEN_EMPTY_PREF,
+      this._hideNoScriptObserver
+    );
   },
 
   onLocationChange(browser, webProgress, _request, _uri, flags) {
@@ -1278,6 +1302,15 @@ var gUnifiedExtensions = {
         return false;
       }
 
+      // When an extensions is about to be removed, it may still appear in
+      // getActiveExtensions.
+      // This is needed for hasExtensionsInPanel, when called through
+      // onWidgetDestroy when an extension is being removed.
+      // See tor-browser#41581.
+      if (extension.hasShutdown) {
+        return false;
+      }
+
       // Ignore hidden and extensions that cannot access the current window
       // (because of PB mode when we are in a private window), since users
       // cannot do anything with those extensions anyway.
@@ -1292,6 +1325,20 @@ var gUnifiedExtensions = {
     return policies;
   },
 
+  /**
+   * Potentially hide the unified-extensions-button if it would be empty.
+   */
+  // See tor-browser#41581.
+  // The behaviour overlaps with a proposal in mozilla Bug 1778684, which has
+  // not been implemented, or had much recent activity as of 5th October 2023.
+  _updateVisibility() {
+    this.button.classList.toggle(
+      "hide-empty",
+      Services.prefs.getBoolPref(HIDE_UNIFIED_WHEN_EMPTY_PREF, true) &&
+        !this.hasExtensionsInPanel()
+    );
+  },
+
   /**
    * Returns true when there are active extensions listed/shown in the unified
    * extensions panel, and false otherwise (e.g. when extensions are pinned in
@@ -1300,7 +1347,13 @@ var gUnifiedExtensions = {
    * @returns {boolean} Whether there are extensions listed in the panel.
    */
   hasExtensionsInPanel() {
-    const policies = this.getActivePolicies();
+    let policies = this.getActivePolicies();
+    // If the NoScript button is hidden, we won't count it towards the list of
+    // extensions in the panel.
+    // See tor-browser#41581.
+    if (Services.prefs.getBoolPref(HIDE_NO_SCRIPT_PREF, true)) {
+      policies = policies.filter(policy => !policy.extension?.isNoScript);
+    }
 
     return !!policies
       .map(policy => this.browserActionFor(policy)?.widget)
@@ -1795,7 +1848,17 @@ var gUnifiedExtensions = {
     }
   },
 
+  onWidgetRemoved() {
+    this._updateVisibility();
+  },
+
+  onWidgetDestroyed() {
+    this._updateVisibility();
+  },
+
   onWidgetAdded(aWidgetId, aArea, aPosition) {
+    this._updateVisibility();
+
     // When we pin a widget to the toolbar from a narrow window, the widget
     // will be overflowed directly. In this case, we do not want to change the
     // class name since it is going to be changed by `onWidgetOverflow()`
@@ -1811,6 +1874,8 @@ var gUnifiedExtensions = {
   },
 
   onWidgetOverflow(aNode, aContainer) {
+    this._updateVisibility();
+
     // We register a CUI listener for each window so we make sure that we
     // handle the event for the right window here.
     if (window !== aNode.ownerGlobal) {
@@ -1821,6 +1886,8 @@ var gUnifiedExtensions = {
   },
 
   onWidgetUnderflow(aNode, aContainer) {
+    this._updateVisibility();
+
     // We register a CUI listener for each window so we make sure that we
     // handle the event for the right window here.
     if (window !== aNode.ownerGlobal) {
diff --git a/browser/components/extensions/parent/ext-browserAction.js b/browser/components/extensions/parent/ext-browserAction.js
index 92555c54a5b9a..79fe912d1a555 100644
--- a/browser/components/extensions/parent/ext-browserAction.js
+++ b/browser/components/extensions/parent/ext-browserAction.js
@@ -274,6 +274,22 @@ this.browserAction = class extends ExtensionAPIPersistent {
         node.append(button, menuButton);
         node.viewButton = button;
 
+        if (extension.isNoScript) {
+          // Hide NoScript by default.
+          // See tor-browser#41581.
+          const HIDE_NO_SCRIPT_PREF = "extensions.hideNoScript";
+          const changeNoScriptVisibility = () => {
+            node.hidden = Services.prefs.getBoolPref(HIDE_NO_SCRIPT_PREF, true);
+          };
+          // Since we expect the NoScript widget to only be destroyed on exit,
+          // we do not set up to remove the observer.
+          Services.prefs.addObserver(
+            HIDE_NO_SCRIPT_PREF,
+            changeNoScriptVisibility
+          );
+          changeNoScriptVisibility();
+        }
+
         return node;
       },
 
diff --git a/browser/themes/shared/addons/unified-extensions.css b/browser/themes/shared/addons/unified-extensions.css
index 029061a7cbb17..a1dadf24768d4 100644
--- a/browser/themes/shared/addons/unified-extensions.css
+++ b/browser/themes/shared/addons/unified-extensions.css
@@ -238,3 +238,21 @@ unified-extensions-item > .subviewbutton {
     border-color: transparent;
   }
 }
+
+/* Extra rule for tor-browser. See tor-browser#41581.
+ * We want to hide the unified-extensions-button when it is empty.
+ * However, this button is needed as an anchor for addon notifications. E.g.
+ * when installing another addon and permissions pop up.
+ * If we simply marked it as "hidden" then it would not be used as an anchor, so
+ * the popup would fall back to using the identity button as an anchor instead.
+ * So instead, we use "visibility: collapse" whilst it is empty *and* it is not
+ * being used as an anchor (the open attribute is missing). */
+#unified-extensions-button.hide-empty:not([open]) {
+  visibility: collapse;
+  /* Ensure getBoundingClientRect().width returns 0.
+   * Even though this button is collapsed, and therefore should not take up any
+   * layout space, getBoundingClientRect will still measure the padding.
+   * If this was not zero, OverflowableToolbar#getOverflowInfo would
+   * over-measure the children width and would always overflow. */
+  padding-inline: 0;
+}
diff --git a/toolkit/mozapps/extensions/content/aboutaddons.html b/toolkit/mozapps/extensions/content/aboutaddons.html
index 126bc625c9cf0..626e3ada1886f 100644
--- a/toolkit/mozapps/extensions/content/aboutaddons.html
+++ b/toolkit/mozapps/extensions/content/aboutaddons.html
@@ -508,6 +508,32 @@
           <div class="addon-detail-sitepermissions">
             <addon-sitepermissions-list></addon-sitepermissions-list>
           </div>
+          <!-- Add an option to show the NoScript toolbar button, if this is the
+             - NoScript addon. See tor-browser#41581. -->
+          <div
+            class="addon-detail-row addon-detail-row-noscript-visibility"
+            role="radiogroup"
+            hidden="hidden"
+          >
+            <span
+              class="addon-noscript-visibility-label"
+              data-l10n-id="basebrowser-addon-noscript-visibility-label"
+            ></span>
+            <div class="addon-detail-actions">
+              <label class="radio-container-with-text">
+                <input type="radio" name="noscript-visibility" value="show" />
+                <span
+                  data-l10n-id="basebrowser-addon-noscript-visibility-show"
+                ></span>
+              </label>
+              <label class="radio-container-with-text">
+                <input type="radio" name="noscript-visibility" value="hide" />
+                <span
+                  data-l10n-id="basebrowser-addon-noscript-visibility-hide"
+                ></span>
+              </label>
+            </div>
+          </div>
           <div class="addon-detail-row addon-detail-row-updates">
             <label data-l10n-id="addon-detail-updates-label"></label>
             <div class="addon-detail-actions">
diff --git a/toolkit/mozapps/extensions/content/aboutaddons.js b/toolkit/mozapps/extensions/content/aboutaddons.js
index 8275a2afefbb9..9de4edc0f7a8f 100644
--- a/toolkit/mozapps/extensions/content/aboutaddons.js
+++ b/toolkit/mozapps/extensions/content/aboutaddons.js
@@ -2062,6 +2062,8 @@ class AddonSitePermissionsList extends HTMLElement {
 }
 customElements.define("addon-sitepermissions-list", AddonSitePermissionsList);
 
+const HIDE_NO_SCRIPT_PREF = "extensions.hideNoScript";
+
 class AddonDetails extends HTMLElement {
   connectedCallback() {
     if (!this.children.length) {
@@ -2069,12 +2071,61 @@ class AddonDetails extends HTMLElement {
     }
     this.deck.addEventListener("view-changed", this);
     this.descriptionShowMoreButton.addEventListener("click", this);
+
+    // If this is for the NoScript extension, we listen for changes in the
+    // visibility of its toolbar button.
+    // See tor-browser#41581.
+    // NOTE: The addon should be set before being connected, so isNoScript will
+    // return a correct value.
+    if (this.isNoScript && !this._noScriptVisibilityObserver) {
+      this._noScriptVisibilityObserver = () => this.updateNoScriptVisibility();
+      Services.prefs.addObserver(
+        HIDE_NO_SCRIPT_PREF,
+        this._noScriptVisibilityObserver
+      );
+    }
   }
 
   disconnectedCallback() {
     this.inlineOptions.destroyBrowser();
     this.deck.removeEventListener("view-changed", this);
     this.descriptionShowMoreButton.removeEventListener("click", this);
+
+    if (this._noScriptVisibilityObserver) {
+      Services.prefs.removeObserver(
+        HIDE_NO_SCRIPT_PREF,
+        this._noScriptVisibilityObserver
+      );
+      // Clear in case this is called again, or if connectedCallback is called.
+      delete this._noScriptVisibilityObserver;
+    }
+  }
+
+  /**
+   * Whether this is a description for the NoScript extension.
+   *
+   * @type {boolean}
+   */
+  get isNoScript() {
+    return this.addon?.id === "{73a6fe31-595d-460b-a920-fcc0f8843232}";
+  }
+
+  /**
+   * Update the shown visibility value for the NoScript extension's toolbar
+   * button.
+   */
+  updateNoScriptVisibility() {
+    if (!this.isNoScript) {
+      return;
+    }
+    const visibility = Services.prefs.getBoolPref(HIDE_NO_SCRIPT_PREF, true)
+      ? "hide"
+      : "show";
+    for (const input of this.querySelectorAll(
+      ".addon-detail-row-noscript-visibility input"
+    )) {
+      input.checked = input.value === visibility;
+    }
   }
 
   handleEvent(e) {
@@ -2270,6 +2321,27 @@ class AddonDetails extends HTMLElement {
       "upgrade"
     );
 
+    // If this is the NoScript extension, we want to show an option to change
+    // the visibility of its toolbar button.
+    // See tor-browser#41581.
+    const visibilityRow = this.querySelector(
+      ".addon-detail-row-noscript-visibility"
+    );
+    visibilityRow.hidden = !this.isNoScript;
+    if (this.isNoScript) {
+      // Set up the aria-label for the role="radiogroup".
+      const visibilityLabel = visibilityRow.querySelector(
+        ".addon-noscript-visibility-label"
+      );
+      visibilityLabel.id = ExtensionCommon.makeWidgetId(
+        `${addon.id}-noscript-visibility-label`
+      );
+      visibilityRow.setAttribute("aria-labelledby", visibilityLabel.id);
+
+      // Set the initial displayed value.
+      this.updateNoScriptVisibility();
+    }
+
     if (addon.type != "extension") {
       // Don't show any private browsing related section for non-extension
       // addon types, because not relevant or they are either always allowed
@@ -2661,6 +2733,11 @@ class AddonCard extends HTMLElement {
           // Update the card if the add-on isn't active.
           this.update();
         }
+      } else if (name == "noscript-visibility") {
+        // Update the NoScript toolbar button visibility.
+        // See tor-browser#41581.
+        const hide = e.target.value !== "show";
+        Services.prefs.setBoolPref(HIDE_NO_SCRIPT_PREF, hide);
       }
     } else if (e.type == "mousedown") {
       // Open panel on mousedown when the mouse is used.
-- 
GitLab