Bug 1566460 - Don't show the panel if there are no messages (#5190)

lib/ASRouter.jsm

@@ -730,7 +730,7 @@ class _ASRouter {
       blockMessageById: this.blockMessageById,
       dispatch: this.dispatch,
     });
-    ToolbarPanelHub.init({
+    ToolbarPanelHub.init(this.waitForInitialized, {
       getMessages: this.handleMessageRequest,
     });
 

lib/ToolbarPanelHub.jsm

@@ -30,8 +30,11 @@ class _ToolbarPanelHub {
     this._hideToolbarButton = this._hideToolbarButton.bind(this);
   }
 
-  init({ getMessages }) {
+  async init(waitForInitialized, { getMessages }) {
     this._getMessages = getMessages;
+    // Wait for ASRouter messages to become available in order to know
+    // if we can show the What's New panel
+    await waitForInitialized;
     if (this.whatsNewPanelEnabled) {
       // Enable the application menu button so that the user can access
       // the panel outside of the toolbar button
@@ -57,6 +60,14 @@ class _ToolbarPanelHub {
     }
   }
 
+  get messages() {
+    return this._getMessages({
+      template: "whatsnew_panel_message",
+      triggerId: "whatsNewPanelOpened",
+      returnAll: true,
+    });
+  }
+
   get whatsNewPanelEnabled() {
     return Services.prefs.getBoolPref(WHATSNEW_ENABLED_PREF, false);
   }
@@ -66,21 +77,25 @@ class _ToolbarPanelHub {
   }
 
   // Turns on the Appmenu (hamburger menu) button for all open windows and future windows.
-  enableAppmenuButton() {
-    EveryWindow.registerCallback(
-      APPMENU_BUTTON_ID,
-      this._showAppmenuButton,
-      this._hideAppmenuButton
-    );
+  async enableAppmenuButton() {
+    if ((await this.messages).length) {
+      EveryWindow.registerCallback(
+        APPMENU_BUTTON_ID,
+        this._showAppmenuButton,
+        this._hideAppmenuButton
+      );
+    }
   }
 
   // Turns on the Toolbar button for all open windows and future windows.
-  enableToolbarButton() {
-    EveryWindow.registerCallback(
-      TOOLBAR_BUTTON_ID,
-      this._showToolbarButton,
-      this._hideToolbarButton
-    );
+  async enableToolbarButton() {
+    if ((await this.messages).length) {
+      EveryWindow.registerCallback(
+        TOOLBAR_BUTTON_ID,
+        this._showToolbarButton,
+        this._hideToolbarButton
+      );
+    }
   }
 
   // When the panel is hidden we want to run some cleanup
@@ -103,11 +118,7 @@ class _ToolbarPanelHub {
 
   // Render what's new messages into the panel.
   async renderMessages(win, doc, containerId) {
-    const messages = (await this._getMessages({
-      template: "whatsnew_panel_message",
-      triggerId: "whatsNewPanelOpened",
-      returnAll: true,
-    })).sort((m1, m2) => {
+    const messages = (await this.messages).sort((m1, m2) => {
       // Sort by published_date in descending order.
       if (m1.content.published_date === m2.content.published_date) {
         return 0;

test/unit/asrouter/ASRouter.test.js

@@ -210,9 +210,13 @@ describe("ASRouter", () => {
         }
       );
 
-      assert.calledWithExactly(FakeToolbarPanelHub.init, {
-        getMessages: Router.handleMessageRequest,
-      });
+      assert.calledWithExactly(
+        FakeToolbarPanelHub.init,
+        Router.waitForInitialized,
+        {
+          getMessages: Router.handleMessageRequest,
+        }
+      );
 
       assert.calledWithExactly(
         FakeBookmarkPanelHub.init,

test/unit/lib/ToolbarPanelHub.test.js

@@ -15,11 +15,13 @@ describe("ToolbarPanelHub", () => {
   let addObserverStub;
   let removeObserverStub;
   let getBoolPrefStub;
+  let waitForInitializedStub;
 
   beforeEach(async () => {
     sandbox = sinon.createSandbox();
     globals = new GlobalOverrider();
     instance = new _ToolbarPanelHub();
+    waitForInitializedStub = sandbox.stub().resolves();
     fakeElementById = {
       setAttribute: sandbox.stub(),
       removeAttribute: sandbox.stub(),
@@ -86,21 +88,23 @@ describe("ToolbarPanelHub", () => {
   it("should not enableAppmenuButton() on init() if pref is not enabled", () => {
     getBoolPrefStub.returns(false);
     instance.enableAppmenuButton = sandbox.stub();
-    instance.init({ getMessages: () => {} });
+    instance.init(waitForInitializedStub, { getMessages: () => {} });
     assert.notCalled(instance.enableAppmenuButton);
   });
-  it("should enableAppmenuButton() on init() if pref is enabled", () => {
+  it("should enableAppmenuButton() on init() if pref is enabled", async () => {
     getBoolPrefStub.returns(true);
     instance.enableAppmenuButton = sandbox.stub();
-    instance.init({ getMessages: () => {} });
+
+    await instance.init(waitForInitializedStub, { getMessages: () => {} });
+
     assert.calledOnce(instance.enableAppmenuButton);
   });
   it("should unregisterCallback on uninit()", () => {
     instance.uninit();
     assert.calledTwice(everyWindowStub.unregisterCallback);
   });
-  it("should observe pref changes on init", () => {
-    instance.init({});
+  it("should observe pref changes on init", async () => {
+    await instance.init(waitForInitializedStub, {});
 
     assert.calledOnce(addObserverStub);
     assert.calledWithExactly(
@@ -145,13 +149,55 @@ describe("ToolbarPanelHub", () => {
       assert.notCalled(instance.uninit);
     });
   });
-  it("should registerCallback on enableAppmenuButton()", () => {
-    instance.enableAppmenuButton();
-    assert.calledOnce(everyWindowStub.registerCallback);
+  describe("#enableAppmenuButton", () => {
+    it("should registerCallback on enableAppmenuButton() if there are messages", async () => {
+      instance.init(waitForInitializedStub, {
+        getMessages: sandbox.stub().resolves([{}, {}]),
+      });
+      // init calls `enableAppmenuButton`
+      everyWindowStub.registerCallback.resetHistory();
+
+      await instance.enableAppmenuButton();
+
+      assert.calledOnce(everyWindowStub.registerCallback);
+      assert.calledWithExactly(
+        everyWindowStub.registerCallback,
+        "appMenu-whatsnew-button",
+        sinon.match.func,
+        sinon.match.func
+      );
+    });
+    it("should not registerCallback on enableAppmenuButton() if there are no messages", async () => {
+      instance.init(waitForInitializedStub, {
+        getMessages: sandbox.stub().resolves([]),
+      });
+      // init calls `enableAppmenuButton`
+      everyWindowStub.registerCallback.resetHistory();
+
+      await instance.enableAppmenuButton();
+
+      assert.notCalled(everyWindowStub.registerCallback);
+    });
   });
-  it("should registerCallback on enableToolbarButton()", () => {
-    instance.enableToolbarButton();
-    assert.calledOnce(everyWindowStub.registerCallback);
+  describe("#enableToolbarButton", () => {
+    it("should registerCallback on enableToolbarButton if messages.length", async () => {
+      instance.init(waitForInitializedStub, {
+        getMessages: sandbox.stub().resolves([{}, {}]),
+      });
+
+      await instance.enableToolbarButton();
+
+      assert.calledOnce(everyWindowStub.registerCallback);
+    });
+    it("should not registerCallback on enableToolbarButton if no messages", async () => {
+      instance.init(waitForInitializedStub, {
+        getMessages: sandbox.stub().resolves([]),
+      });
+
+      await instance.enableToolbarButton();
+
+      assert.notCalled(everyWindowStub.registerCallback);
+    });
   });
   it("should unhide appmenu button on _showAppmenuButton()", () => {
     instance._showAppmenuButton(fakeWindow);
@@ -174,7 +220,7 @@ describe("ToolbarPanelHub", () => {
       m => m.template === "whatsnew_panel_message"
     );
     messages[0].content.link_text = { string_id: "link_text_id" };
-    instance.init({
+    instance.init(waitForInitializedStub, {
       getMessages: sandbox
         .stub()
         .returns([messages[0], messages[2], messages[1]]),
@@ -204,14 +250,14 @@ describe("ToolbarPanelHub", () => {
     const uniqueDates = [
       ...new Set(messages.map(m => m.content.published_date)),
     ];
-    instance.init({
+    instance.init(waitForInitializedStub, {
       getMessages: sandbox.stub().returns(messages),
     });
     await instance.renderMessages(fakeWindow, fakeDocument, "container-id");
     assert.callCount(instance._createDateElement, uniqueDates.length);
   });
   it("should listen for panelhidden and remove the toolbar button", async () => {
-    instance.init({
+    instance.init(waitForInitializedStub, {
       getMessages: sandbox.stub().returns([]),
     });
     fakeDocument.getElementById
@@ -223,7 +269,7 @@ describe("ToolbarPanelHub", () => {
     assert.notCalled(fakeElementById.addEventListener);
   });
   it("should listen for panelhidden and remove the toolbar button", async () => {
-    instance.init({
+    instance.init(waitForInitializedStub, {
       getMessages: sandbox.stub().returns([]),
     });