Bug 1538464 - Ensure Pocket context menu items work as expected in sidebar extension or other windows. r=mconley

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

browser/base/content/nsContextMenu.js

@@ -335,6 +335,7 @@ class nsContextMenu {
     this.initViewItems();
     this.initImageItems();
     this.initMiscItems();
+    this.initPocketItems();
     this.initSpellingItems();
     this.initSaveItems();
     this.initSyncItems();
@@ -760,6 +761,51 @@ class nsContextMenu {
     );
   }
 
+  initPocketItems() {
+    const pocketEnabled = Services.prefs.getBoolPref(
+      "extensions.pocket.enabled"
+    );
+    let showSaveCurrentPageToPocket = false;
+    let showSaveLinkToPocket = false;
+
+    // We can skip all this is Pocket is not enabled.
+    if (pocketEnabled) {
+      let targetURL, targetURI;
+      // If the context menu is opened over a link, we target the link,
+      // if not, we target the page.
+      if (this.onLink) {
+        targetURL = this.linkURL;
+        // linkURI may be null if the URL is invalid.
+        targetURI = this.linkURI;
+      } else {
+        targetURL = this.browser?.currentURI?.spec;
+        targetURI = Services.io.newURI(targetURL);
+      }
+
+      const canPocket =
+        targetURI?.schemeIs("http") ||
+        targetURI?.schemeIs("https") ||
+        (targetURI?.schemeIs("about") && ReaderMode?.getOriginalUrl(targetURL));
+
+      // If the target is valid, decide which menu item to enable.
+      if (canPocket) {
+        showSaveLinkToPocket = this.onLink;
+        showSaveCurrentPageToPocket = !(
+          this.onTextInput ||
+          this.onLink ||
+          this.isContentSelected ||
+          this.onImage ||
+          this.onCanvas ||
+          this.onVideo ||
+          this.onAudio
+        );
+      }
+    }
+
+    this.showItem("context-pocket", showSaveCurrentPageToPocket);
+    this.showItem("context-savelinktopocket", showSaveLinkToPocket);
+  }
+
   initSpellingItems() {
     var canSpell =
       InlineSpellCheckerUI.canSpellCheck &&

browser/components/pocket/content/Pocket.jsm

@@ -53,10 +53,14 @@ var Pocket = {
   _urlToSave: null,
   _titleToSave: null,
   savePage(browser, url, title) {
-    if (!browser?.ownerDocument || !browser?.ownerGlobal?.PanelUI) {
+    // We want to target the top browser which has the Pocket panel UI,
+    // which might not be the browser saving the article.
+    const ownerGlobal = browser?.ownerGlobal?.top;
+    const ownerDocument = ownerGlobal?.document;
+
+    if (!ownerDocument || !ownerGlobal?.PanelUI) {
       return;
     }
-    let { ownerDocument, ownerGlobal } = browser;
 
     let widget = CustomizableUI.getWidget("save-to-pocket-button");
     let anchorNode = widget.areaType

browser/components/pocket/content/SaveToPocket.jsm

@@ -19,11 +19,6 @@ ChromeUtils.defineModuleGetter(
   "CustomizableUI",
   "resource:///modules/CustomizableUI.jsm"
 );
-ChromeUtils.defineModuleGetter(
-  this,
-  "ReaderMode",
-  "resource://gre/modules/ReaderMode.jsm"
-);
 ChromeUtils.defineModuleGetter(
   this,
   "AboutReaderParent",
@@ -37,7 +32,6 @@ XPCOMUtils.defineLazyGetter(this, "gStrings", () => {
     "chrome://global/locale/aboutReader.properties"
   );
 });
-
 var PocketCustomizableWidget = {
   init() {
     CustomizableUI.createWidget({
@@ -78,64 +72,12 @@ var PocketCustomizableWidget = {
   },
 };
 
-// PocketContextMenu
-// When the context menu is opened check if we need to build and enable pocket UI.
-var PocketContextMenu = {
-  init() {
-    Services.obs.addObserver(this, "on-build-contextmenu");
-  },
-  shutdown() {
-    Services.obs.removeObserver(this, "on-build-contextmenu");
-  },
-  observe(aSubject, aTopic, aData) {
-    let subject = aSubject.wrappedJSObject;
-    let document = subject.menu.ownerDocument;
-    let pocketEnabled = SaveToPocket.prefEnabled;
-
-    let showSaveCurrentPageToPocket = !(
-      subject.onTextInput ||
-      subject.onLink ||
-      subject.isContentSelected ||
-      subject.onImage ||
-      subject.onCanvas ||
-      subject.onVideo ||
-      subject.onAudio
-    );
-    let targetUrl, targetURI;
-    if (subject.onLink) {
-      targetUrl = subject.linkUrl;
-      // linkURI may be null if the URL is invalid.
-      targetURI = subject.linkURI;
-    } else {
-      targetUrl = subject.pageUrl;
-      targetURI = Services.io.newURI(targetUrl);
-    }
-    let canPocket =
-      pocketEnabled &&
-      targetURI &&
-      (targetURI.schemeIs("http") ||
-        targetURI.schemeIs("https") ||
-        (targetURI.schemeIs("about") && ReaderMode.getOriginalUrl(targetUrl)));
-
-    let showSaveLinkToPocket =
-      canPocket && !showSaveCurrentPageToPocket && subject.onLink;
-
-    let menu = document.getElementById("context-pocket");
-    menu.hidden = !(canPocket && showSaveCurrentPageToPocket);
-
-    menu = document.getElementById("context-savelinktopocket");
-    menu.hidden = !showSaveLinkToPocket;
-  },
-};
-
 var PocketOverlay = {
   startup() {
     PocketCustomizableWidget.init();
-    PocketContextMenu.init();
   },
   shutdown() {
     PocketCustomizableWidget.shutdown();
-    PocketContextMenu.shutdown();
   },
 };
 
@@ -287,12 +229,6 @@ var SaveToPocket = {
     if (enabled) {
       win.document.documentElement.removeAttribute("pocketdisabled");
     } else {
-      // Hide the context menu items to ensure separator logic works.
-      let savePageMenu = win.document.getElementById("context-pocket");
-      let saveLinkMenu = win.document.getElementById(
-        "context-savelinktopocket"
-      );
-      savePageMenu.hidden = saveLinkMenu.hidden = true;
       win.document.documentElement.setAttribute("pocketdisabled", "true");
     }
   },

browser/components/pocket/test/browser_pocket_ui_check.js

@@ -54,18 +54,32 @@ add_task(async function() {
   await popupShown;
 
   checkElementsShown(true, ["context-savelinktopocket"]);
-
   contextMenu.hidePopup();
   await popupHidden;
-  BrowserTestUtils.removeTab(tab);
 
   await promisePocketDisabled();
 
+  popupShown = BrowserTestUtils.waitForEvent(contextMenu, "popupshown");
+  popupHidden = BrowserTestUtils.waitForEvent(contextMenu, "popuphidden");
+  await BrowserTestUtils.synthesizeMouseAtCenter(
+    "a",
+    {
+      type: "contextmenu",
+      button: 2,
+    },
+    tab.linkedBrowser
+  );
+  await popupShown;
+
   checkElementsShown(false, [
     "context-pocket",
     "context-savelinktopocket",
     "save-to-pocket-button",
   ]);
 
+  contextMenu.hidePopup();
+  await popupHidden;
+  BrowserTestUtils.removeTab(tab);
+
   await promisePocketReset();
 });