Bug 1366041 - Add "Take Screenshot" button to Page Action Menu. draft
authorDrew Willcoxon <adw@mozilla.com>
Sat, 29 Jul 2017 03:18:18 -0700
changeset 618020 3512f83f8a548e80e84a0bee4fd7f4ecb28aa601
parent 618014 c8fc70087b07f78058ae7dfba19e405c0ac29f84
child 639941 7cf9e56640930918d6977cda5c0d5c8f10367a1a
push id71191
push userdwillcoxon@mozilla.com
push dateSat, 29 Jul 2017 10:20:12 +0000
bugs1366041
milestone56.0a1
Bug 1366041 - Add "Take Screenshot" button to Page Action Menu. MozReview-Commit-ID: 79pvbraUJnH
browser/extensions/screenshots/bootstrap.js
browser/extensions/screenshots/test/browser/browser_screenshots_ui_check.js
browser/extensions/screenshots/test/browser/head.js
browser/extensions/screenshots/webextension/background/main.js
browser/extensions/screenshots/webextension/background/startBackground.js
browser/extensions/screenshots/webextension/manifest.json
--- a/browser/extensions/screenshots/bootstrap.js
+++ b/browser/extensions/screenshots/bootstrap.js
@@ -12,16 +12,18 @@ Cu.import("resource://gre/modules/XPCOMU
 XPCOMUtils.defineLazyModuleGetter(this, "AddonManager",
                                   "resource://gre/modules/AddonManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Console",
                                   "resource://gre/modules/Console.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
                                   "resource://gre/modules/Services.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "LegacyExtensionsUtils",
                                   "resource://gre/modules/LegacyExtensionsUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PageActions",
+                                  "resource:///modules/PageActions.jsm");
 
 let addonResourceURI;
 let appStartupDone;
 const appStartupPromise = new Promise((resolve, reject) => {
   appStartupDone = resolve;
 });
 
 const prefs = Services.prefs;
@@ -107,29 +109,34 @@ function handleStartup() {
   } else if (shouldDisable()) {
     stop(webExtension, ADDON_DISABLE);
   }
 }
 
 function start(webExtension) {
   webExtension.startup(startupReason).then((api) => {
     api.browser.runtime.onMessage.addListener(handleMessage);
+    initPhotonPageAction(api);
   }).catch((err) => {
     // The startup() promise will be rejected if the webExtension was
     // already started (a harmless error), or if initializing the
     // WebExtension failed and threw (an important error).
     console.error(err);
     if (err.message !== "This embedded extension has already been started") {
       // TODO: Should we send these errors to Sentry? #2420
     }
   });
 }
 
 function stop(webExtension, reason) {
   webExtension.shutdown(reason);
+  if (photonPageAction) {
+    photonPageAction.remove();
+    photonPageAction = null;
+  }
 }
 
 function handleMessage(msg, sender, sendReply) {
   if (!msg) {
     return;
   }
 
   if (msg.funcName === "getTelemetryPref") {
@@ -144,8 +151,74 @@ function handleMessage(msg, sender, send
       if (addon) {
         addon.uninstall();
       }
       sendReply({type: "success", value: !!addon});
     });
     return true;
   }
 }
+
+let photonPageAction;
+
+// Sets up the Photon page action.  Ideally, in the future, WebExtension page
+// actions and Photon page actions would be one in the same, but they aren't
+// right now.
+function initPhotonPageAction(api) {
+  let id = "screenshots";
+  photonPageAction = PageActions.actionForID(id);
+  if (photonPageAction) {
+    // The page action has already been set up (which shouldn't happen, but
+    // check anyway).
+    return;
+  }
+
+  let {Management: {global: {tabTracker}}} = Cu.import("resource://gre/modules/Extension.jsm", {});
+
+  let port = null;
+  let title = "Take a Screenshot";
+  let baseIconPath = addonResourceURI.spec + "webextension/";
+  let iconURL = baseIconPath + "icons/icon-32-v2.svg";
+
+  // Get a port to the WebExtension side.
+  api.browser.runtime.onConnect.addListener((listenerPort) => {
+    if (listenerPort.name == "photonPageActionPort") {
+      port = listenerPort;
+
+      // The WebExtension side of the port sends over the action's localized
+      // title and possibly updated iconURL.
+      port.onMessage.addListener((message) => {
+        if (message.title) {
+          if (photonPageAction) {
+            photonPageAction.title = message.title;
+          } else {
+            title = message.title;
+          }
+        }
+        if (message.iconPath) {
+          iconURL = baseIconPath + message.iconPath;
+          if (photonPageAction) {
+            photonPageAction.iconURL = iconURL;
+          }
+        }
+      });
+    }
+  });
+
+  // Add the page action.
+  photonPageAction = PageActions.addAction(new PageActions.Action({
+    id,
+    title,
+    iconURL,
+    _insertBeforeActionID: null,
+    onCommand(event, buttonNode) {
+      if (port) {
+        let browserWin = buttonNode.ownerDocument.defaultView;
+        port.postMessage({
+          tab: {
+            url: browserWin.gBrowser.selectedBrowser.currentURI.spec,
+            id: tabTracker.getId(browserWin.gBrowser.selectedTab),
+          },
+        });
+      }
+    },
+  }));
+}
--- a/browser/extensions/screenshots/test/browser/browser_screenshots_ui_check.js
+++ b/browser/extensions/screenshots/test/browser/browser_screenshots_ui_check.js
@@ -9,13 +9,13 @@ function checkElements(expectPresent, l)
 add_task(async function() {
   await promiseScreenshotsEnabled();
 
   registerCleanupFunction(async function() {
     await promiseScreenshotsReset();
   });
 
   await BrowserTestUtils.waitForCondition(
-    () => document.getElementById("screenshots_mozilla_org-browser-action"),
+    () => document.getElementById("pageAction-panel-screenshots"),
     "Screenshots button should be present", 100, 100);
 
-  checkElements(true, ["screenshots_mozilla_org-browser-action"]);
+  checkElements(true, ["pageAction-panel-screenshots"]);
 });
--- a/browser/extensions/screenshots/test/browser/head.js
+++ b/browser/extensions/screenshots/test/browser/head.js
@@ -8,47 +8,43 @@ let enabledOnStartup = false;
 function promiseScreenshotsEnabled() {
   if (!Services.prefs.getBoolPref("extensions.screenshots.system-disabled", false)) {
     info("Screenshots was already enabled, assuming enabled by default for tests");
     enabledOnStartup = true;
     return Promise.resolve(true);
   }
   info("Screenshots is not enabled");
   return new Promise((resolve, reject) => {
-    let listener = {
-      onWidgetAfterCreation(widgetid) {
-        if (widgetid == "screenshots_mozilla_org-browser-action") {
-          info("screenshots_mozilla_org-browser-action button created");
-          CustomizableUI.removeListener(listener);
-          resolve(false);
-        }
+    let interval = setInterval(() => {
+      let action = PageActions.actionForID("screenshots");
+      if (action) {
+        info("screenshots page action created");
+        clearInterval(interval);
+        resolve(false);
       }
-    }
-    CustomizableUI.addListener(listener);
+    }, 100);
     info("Set Screenshots disabled pref to false.");
     Services.prefs.setBoolPref("extensions.screenshots.system-disabled", false);
   });
 }
 
 function promiseScreenshotsDisabled() {
   if (Services.prefs.getBoolPref("extensions.screenshots.system-disabled", false)) {
     info("Screenshots already disabled");
     return Promise.resolve(true);
   }
   return new Promise((resolve, reject) => {
-    let listener = {
-      onWidgetDestroyed(widgetid) {
-        if (widgetid == "screenshots_mozilla_org-browser-action") {
-          CustomizableUI.removeListener(listener);
-          info("screenshots_mozilla_org-browser-action destroyed");
-          resolve(false);
-        }
+    let interval = setInterval(() => {
+      let action = PageActions.actionForID("screenshots");
+      if (!action) {
+        info("screenshots page action removed");
+        clearInterval(interval);
+        resolve(false);
       }
-    }
-    CustomizableUI.addListener(listener);
+    }, 100);
     info("Set Screenshots disabled pref to true.");
     Services.prefs.setBoolPref("extensions.screenshots.system-disabled", true);
   });
 }
 
 function promiseScreenshotsReset() { // eslint-disable-line no-unused-vars
   if (enabledOnStartup) {
     info("Reset is enabling Screenshots addon");
--- a/browser/extensions/screenshots/webextension/background/main.js
+++ b/browser/extensions/screenshots/webextension/background/main.js
@@ -13,17 +13,17 @@ this.main = (function() {
 
   let hasSeenOnboarding;
 
   browser.storage.local.get(["hasSeenOnboarding"]).then((result) => {
     hasSeenOnboarding = !!result.hasSeenOnboarding;
     if (!hasSeenOnboarding) {
       setIconActive(false, null);
       // Note that the branded name 'Firefox Screenshots' is not localized:
-      browser.browserAction.setTitle({
+      startBackground.photonPageActionPort.postMessage({
         title: "Firefox Screenshots"
       });
     }
   }).catch((error) => {
     log.error("Error getting hasSeenOnboarding:", error);
   });
 
   exports.setBackend = function(newBackend) {
@@ -50,23 +50,18 @@ this.main = (function() {
     }
   }
 
   function setIconActive(active, tabId) {
     let path = active ? "icons/icon-highlight-32-v2.svg" : "icons/icon-32-v2.svg";
     if ((!hasSeenOnboarding) && !active) {
       path = "icons/icon-starred-32-v2.svg";
     }
-    browser.browserAction.setIcon({path, tabId}).catch((error) => {
-      // FIXME: use errorCode
-      if (error.message && /Invalid tab ID/.test(error.message)) {
-        // This is a normal exception that we can ignore
-      } else {
-        catcher.unhandled(error);
-      }
+    startBackground.photonPageActionPort.postMessage({
+      iconPath: path
     });
   }
 
   function toggleSelector(tab) {
     return analytics.refreshTelemetryPref()
       .then(() => selectorLoader.toggle(tab.id, hasSeenOnboarding))
       .then(active => {
         setIconActive(active, tab.id);
@@ -92,17 +87,18 @@ this.main = (function() {
       }
     });
   }
 
   function shouldOpenMyShots(url) {
     return /^about:(?:newtab|blank)/i.test(url) || /^resource:\/\/activity-streams\//i.test(url);
   }
 
-  // This is called by startBackground.js, directly in response to browser.browserAction.onClicked
+  // This is called by startBackground.js, directly in response to clicks on
+  // the Photon page action
   exports.onClicked = catcher.watchFunction((tab) => {
     if (tab.incognito) {
       senderror.showError({
         popupMessage: "PRIVATE_WINDOW"
       });
       return;
     }
     if (shouldOpenMyShots(tab.url)) {
@@ -269,17 +265,17 @@ this.main = (function() {
       });
     }
   }));
 
   communication.register("hasSeenOnboarding", () => {
     hasSeenOnboarding = true;
     catcher.watchPromise(browser.storage.local.set({hasSeenOnboarding}));
     setIconActive(false, null);
-    browser.browserAction.setTitle({
+    startBackground.photonPageActionPort.postMessage({
       title: browser.i18n.getMessage("contextMenuLabel")
     });
   });
 
   communication.register("abortFrameset", () => {
     sendEvent("abort-start-shot", "frame-page");
     // Note, we only show the error but don't report it, as we know that we can't
     // take shots of these pages:
--- a/browser/extensions/screenshots/webextension/background/startBackground.js
+++ b/browser/extensions/screenshots/webextension/background/startBackground.js
@@ -1,18 +1,20 @@
 /* globals browser, main, communication */
 /* This file handles:
-     browser.browserAction.onClicked
+     clicks on the Photon page action
      browser.contextMenus.onClicked
      browser.runtime.onMessage
    and loads the rest of the background page in response to those events, forwarding
    the events to main.onClicked, main.onClickedContextMenu, or communication.onMessage
 */
 
 this.startBackground = (function() {
+  let exports = {};
+
   const backgroundScripts = [
     "log.js",
     "makeUuid.js",
     "catcher.js",
     "background/selectorLoader.js",
     "background/communication.js",
     "background/auth.js",
     "background/senderror.js",
@@ -22,24 +24,16 @@ this.startBackground = (function() {
     "background/deviceInfo.js",
     "background/takeshot.js",
     "background/main.js"
   ];
 
   // Maximum milliseconds to wait before checking for migration possibility
   const CHECK_MIGRATION_DELAY = 2000;
 
-  browser.browserAction.onClicked.addListener((tab) => {
-    loadIfNecessary().then(() => {
-      main.onClicked(tab);
-    }).catch((error) => {
-      console.error("Error loading Screenshots:", error);
-    });
-  });
-
   browser.contextMenus.create({
     id: "create-screenshot",
     title: browser.i18n.getMessage("contextMenuLabel"),
     contexts: ["page"],
     documentUrlPatterns: ["<all_urls>"]
   });
 
   browser.contextMenus.onClicked.addListener((info, tab) => {
@@ -47,35 +41,59 @@ this.startBackground = (function() {
       main.onClickedContextMenu(info, tab);
     }).catch((error) => {
       console.error("Error loading Screenshots:", error);
     });
   });
 
   // Note this duplicates functionality in main.js, but we need to change
   // the onboarding icon before main.js loads up
+  let iconPath = null;
   browser.storage.local.get(["hasSeenOnboarding"]).then((result) => {
     let hasSeenOnboarding = !!result.hasSeenOnboarding;
     if (!hasSeenOnboarding) {
-      let path = "icons/icon-starred-32-v2.svg";
-      browser.browserAction.setIcon({path});
+      iconPath = "icons/icon-starred-32-v2.svg";
+      if (photonPageActionPort) {
+        photonPageActionPort.postMessage({
+          iconPath
+        });
+      }
     }
   }).catch((error) => {
     console.error("Error loading Screenshots onboarding flag:", error);
   });
 
   browser.runtime.onMessage.addListener((req, sender, sendResponse) => {
     loadIfNecessary().then(() => {
       return communication.onMessage(req, sender, sendResponse);
     }).catch((error) => {
       console.error("Error loading Screenshots:", error);
     });
     return true;
   });
 
+  // Set up this side of the Photon page action port.  The other side is in
+  // bootstrap.js.  Ideally, in the future, WebExtension page actions and Photon
+  // page actions would be one in the same, but they aren't right now.
+  let photonPageActionPort = browser.runtime.connect({ name: "photonPageActionPort" });
+  exports.photonPageActionPort = photonPageActionPort;
+  // Send over the localized title possibly updated iconURL of the page action.
+  photonPageActionPort.postMessage({
+    title: browser.i18n.getMessage("contextMenuLabel"),
+    iconPath
+  });
+  // Listen for clicks on the page action.
+  photonPageActionPort.onMessage.addListener((message) => {
+    loadIfNecessary().then(() => {
+      main.onClicked(message.tab);
+    }).catch((error) => {
+      console.error("Error loading Screenshots:", error);
+    });
+  });
+
   // We delay this check (by CHECK_MIGRATION_DELAY) just to avoid piling too
   // many things onto browser/add-on startup
   requestIdleCallback(() => {
     browser.runtime.sendMessage({funcName: "getOldDeviceInfo"}).then((result) => {
       if (result && result.type == "success" && result.value) {
         // There is a possible migration to run, so we'll load the entire background
         // page and continue the process
         return loadIfNecessary();
@@ -117,9 +135,10 @@ this.startBackground = (function() {
           };
           document.head.appendChild(tag);
         });
       });
     });
     return loadedPromise;
   }
 
+  return exports;
 })();
--- a/browser/extensions/screenshots/webextension/manifest.json
+++ b/browser/extensions/screenshots/webextension/manifest.json
@@ -6,24 +6,16 @@
   "author": "__MSG_addonAuthorsList__",
   "homepage_url": "https://github.com/mozilla-services/screenshots",
   "applications": {
     "gecko": {
       "id": "screenshots@mozilla.org"
     }
   },
   "default_locale": "en_US",
-  "browser_action": {
-    "default_icon": {
-      "16": "icons/icon-16-v2.svg",
-      "32": "icons/icon-32-v2.svg"
-    },
-    "default_title": "Firefox Screenshots",
-    "browser_style": false
-  },
   "background": {
     "scripts": [
       "build/buildSettings.js",
       "background/startBackground.js"
     ]
   },
   "content_scripts": [
     {