Bug 1518722 - default pocket context menus to hidden to fix behavior when setting pocket pref to false, r=mconley
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 14 Jan 2019 23:24:39 +0000
changeset 513859 6479cf440470253698f46ebb688ac58fa15480ae
parent 513858 226b130b6360f05f7329b3a836177af5bb32fd1d
child 513860 d915d432405f0510b12f3de956dcc64344111809
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley
bugs1518722
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1518722 - default pocket context menus to hidden to fix behavior when setting pocket pref to false, r=mconley Differential Revision: https://phabricator.services.mozilla.com/D16037
browser/components/pocket/content/SaveToPocket.jsm
browser/components/pocket/test/browser_pocket_ui_check.js
browser/components/pocket/test/head.js
--- a/browser/components/pocket/content/SaveToPocket.jsm
+++ b/browser/components/pocket/content/SaveToPocket.jsm
@@ -202,56 +202,74 @@ var SaveToPocket = {
     }
     // Only define the pref getter now, so we don't get notified for the
     // migrated pref above.
     XPCOMUtils.defineLazyPreferenceGetter(
       this, "prefEnabled", "extensions.pocket.enabled", true, this.onPrefChange.bind(this));
     if (this.prefEnabled) {
       PocketOverlay.startup();
     } else {
+      // We avoid calling onPrefChange or similar here, because we don't want to
+      // shut down things that haven't started up, or broadcast unnecessary messages.
       this.updateElements(false);
+      Services.obs.addObserver(this, "browser-delayed-startup-finished");
     }
     Services.mm.addMessageListener("Reader:OnSetup", this);
     Services.mm.addMessageListener("Reader:Clicked-pocket-button", this);
   },
 
+  observe(subject, topic, data) {
+    if (topic == "browser-delayed-startup-finished") {
+      subject.QueryInterface(Ci.nsIDOMWindow);
+      // We only get here if pocket is disabled; the observer is removed when
+      // we're enabled.
+      this.updateElementsInWindow(subject, false);
+    }
+  },
+
   _readerButtonData: {
     id: "pocket-button",
     image: "chrome://pocket/content/panels/img/pocket-outline.svg",
     width: 20,
     height: 20,
   },
 
   onPrefChange(pref, oldValue, newValue) {
     if (!newValue) {
       Services.mm.broadcastAsyncMessage("Reader:RemoveButton", { id: "pocket-button" });
       PocketOverlay.shutdown();
+      Services.obs.addObserver(this, "browser-delayed-startup-finished");
     } else {
+      Services.obs.removeObserver(this, "browser-delayed-startup-finished");
       PocketOverlay.startup();
       // The title for the button is extracted from browser.xul where it comes from a DTD.
       // If we don't have this, there's also no possibility of there being a reader
       // mode tab already loaded. We'll get an Reader:OnSetup message when that happens.
       if (this._readerButtonData.title) {
         Services.mm.broadcastAsyncMessage("Reader:AddButton", this._readerButtonData);
       }
     }
     this.updateElements(newValue);
   },
 
   updateElements(enabled) {
     // loop through windows and show/hide all our elements.
+    for (let win of browserWindows()) {
+      this.updateElementsInWindow(win, enabled);
+    }
+  },
+
+  updateElementsInWindow(win, enabled) {
     let elementIds = [
       "context-pocket", "context-savelinktopocket",
       "appMenu-library-pocket-button",
     ];
-    for (let win of browserWindows()) {
-      let document = win.document;
-      for (let id of elementIds) {
-        document.getElementById(id).hidden = !enabled;
-      }
+    let document = win.document;
+    for (let id of elementIds) {
+      document.getElementById(id).hidden = !enabled;
     }
   },
 
   receiveMessage(message) {
     if (!this.prefEnabled) {
       return;
     }
     switch (message.name) {
--- a/browser/components/pocket/test/browser_pocket_ui_check.js
+++ b/browser/components/pocket/test/browser_pocket_ui_check.js
@@ -52,10 +52,19 @@ add_task(async function() {
 
   await promisePocketDisabled();
 
   checkElements(false, ["appMenu-library-pocket-button",
                         "context-pocket", "context-savelinktopocket"]);
   buttonBox = document.getElementById("pocket-button-box");
   is(buttonBox.hidden, true, "Button should have been hidden");
 
+  let newWin = await BrowserTestUtils.openNewBrowserWindow();
+  checkElements(false, ["appMenu-library-pocket-button",
+                        "context-pocket", "context-savelinktopocket"],
+                newWin);
+  buttonBox = newWin.document.getElementById("pocket-button-box");
+  is(buttonBox.hidden, true, "Button should have been hidden");
+
+  await BrowserTestUtils.closeWindow(newWin);
+
   await promisePocketReset();
 });
--- a/browser/components/pocket/test/head.js
+++ b/browser/components/pocket/test/head.js
@@ -38,14 +38,14 @@ function promisePocketReset() {
   if (enabledOnStartup) {
     info("reset is enabling pocket addon");
     return promisePocketEnabled();
   }
   info("reset is disabling pocket addon");
   return promisePocketDisabled();
 }
 
-function checkElements(expectPresent, l) {
+function checkElements(expectPresent, l, win = window) {
   for (let id of l) {
-    let el = document.getElementById(id) || gNavToolbox.palette.querySelector("#" + id);
+    let el = win.document.getElementById(id) || win.gNavToolbox.palette.querySelector("#" + id);
     is(!!el && !el.hidden, expectPresent, "element " + id + (expectPresent ? " is" : " is not") + " present");
   }
 }