Bug 1466575 - Replace Photon pageAction with WebExtension pageAction in Screenshots; r?_6a68 draft
authorBarry Chen <bchen@mozilla.com>
Wed, 01 Aug 2018 10:15:25 -0500
changeset 828130 0c0348fb39b0ce7e1556852b297b565adb858f9b
parent 828125 937c64cef999caecf6478792683879105877a6db
child 828131 1b4c56700169867600e1552bf96fe2c592344a76
push id118634
push userbmo:bchen@mozilla.com
push dateThu, 09 Aug 2018 23:59:14 +0000
reviewers_6a68
bugs1466575
milestone63.0a1
Bug 1466575 - Replace Photon pageAction with WebExtension pageAction in Screenshots; r?_6a68 MozReview-Commit-ID: 9pkwRRN2y7O
browser/extensions/screenshots/bootstrap.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
@@ -10,18 +10,16 @@ ChromeUtils.import("resource://gre/modul
 ChromeUtils.defineModuleGetter(this, "AddonManager",
                                "resource://gre/modules/AddonManager.jsm");
 ChromeUtils.defineModuleGetter(this, "AppConstants",
                                "resource://gre/modules/AppConstants.jsm");
 ChromeUtils.defineModuleGetter(this, "CustomizableUI",
                                "resource:///modules/CustomizableUI.jsm");
 ChromeUtils.defineModuleGetter(this, "LegacyExtensionsUtils",
                                "resource://gre/modules/LegacyExtensionsUtils.jsm");
-ChromeUtils.defineModuleGetter(this, "PageActions",
-                               "resource:///modules/PageActions.jsm");
 ChromeUtils.defineModuleGetter(this, "Services",
                                "resource://gre/modules/Services.jsm");
 
 let addonResourceURI;
 let appStartupDone;
 let appStartupPromise = new Promise((resolve, reject) => {
   appStartupDone = resolve;
 });
@@ -173,35 +171,30 @@ function handleStartup() {
     stop(webExtension, ADDON_DISABLE);
   }
 }
 
 function start(webExtension) {
   return webExtension.startup(startupReason, addonData).then((api) => {
     api.browser.runtime.onMessage.addListener(handleMessage);
     LibraryButton.init(webExtension);
-    initPhotonPageAction(api, webExtension);
   }).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) {
   if (reason !== APP_SHUTDOWN) {
     LibraryButton.uninit();
-    if (photonPageAction) {
-      photonPageAction.remove();
-      photonPageAction = null;
-    }
   }
   return Promise.resolve(webExtension.shutdown(reason));
 }
 
 function handleMessage(msg, sender, sendReply) {
   if (!msg) {
     return;
   }
@@ -222,62 +215,8 @@ function handleMessage(msg, sender, send
     if (!allowedScalars.includes(scalar)) {
       sendReply({type: "error", name: `incrementCount passed an unrecognized scalar ${scalar}`});
     } else {
       Services.telemetry.scalarAdd(`screenshots.${scalar}`, 1);
       sendReply({type: "success", value: true});
     }
   }
 }
-
-let photonPageAction;
-
-// If the current Firefox version supports Photon (57 and later), this sets up
-// a Photon page action and removes the UI for the WebExtension browser action.
-// Does nothing otherwise.  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, webExtension) {
-  const id = "screenshots";
-  let port = null;
-
-  const {tabManager} = webExtension.extension;
-
-  // Make the page action.
-  photonPageAction = PageActions.actionForID(id) || PageActions.addAction(new PageActions.Action({
-    id,
-    title: "Take a Screenshot",
-    iconURL: webExtension.extension.getURL("icons/icon-v2.svg"),
-    _insertBeforeActionID: null,
-    onCommand(event, buttonNode) {
-      if (port) {
-        const browserWin = buttonNode.ownerGlobal;
-        const tab = tabManager.getWrapper(browserWin.gBrowser.selectedTab);
-        port.postMessage({
-          type: "click",
-          tab: {id: tab.id, url: tab.url}
-        });
-      }
-    },
-  }));
-
-  // Establish a port to the WebExtension side.
-  api.browser.runtime.onConnect.addListener((listenerPort) => {
-    if (listenerPort.name !== "photonPageActionPort") {
-      return;
-    }
-    port = listenerPort;
-    port.onMessage.addListener((message) => {
-      switch (message.type) {
-      case "setProperties":
-        if (message.title) {
-          photonPageAction.setTitle(message.title);
-        }
-        if (message.iconPath) {
-          photonPageAction.setIconURL(webExtension.extension.getURL(message.iconPath));
-        }
-        break;
-      default:
-        console.error("Unrecognized message:", message);
-        break;
-      }
-    });
-  });
-}
--- a/browser/extensions/screenshots/webextension/background/main.js
+++ b/browser/extensions/screenshots/webextension/background/main.js
@@ -8,24 +8,16 @@ this.main = (function() {
   const pasteSymbol = (window.navigator.platform.match(/Mac/i)) ? "\u2318" : "Ctrl";
   const { sendEvent } = analytics;
 
   const manifest = browser.runtime.getManifest();
   let backend;
 
   let hasSeenOnboarding = browser.storage.local.get(["hasSeenOnboarding"]).then((result) => {
     const onboarded = !!result.hasSeenOnboarding;
-    if (!onboarded) {
-      setIconActive(false, null);
-      // Note that the branded name 'Firefox Screenshots' is not localized:
-      startBackground.photonPageActionPort.postMessage({
-        type: "setProperties",
-        title: "Firefox Screenshots"
-      });
-    }
     hasSeenOnboarding = Promise.resolve(onboarded);
     return hasSeenOnboarding;
   }).catch((error) => {
     log.error("Error getting hasSeenOnboarding:", error);
   });
 
   exports.setBackend = function(newBackend) {
     backend = newBackend;
@@ -48,20 +40,17 @@ this.main = (function() {
     if (/^https?:\/\//.test(permission)) {
       exports.setBackend(permission);
       break;
     }
   }
 
   function setIconActive(active, tabId) {
     const path = active ? "icons/icon-highlight-32-v2.svg" : "icons/icon-v2.svg";
-    startBackground.photonPageActionPort.postMessage({
-      type: "setProperties",
-      iconPath: path
-    });
+    browser.pageAction.setIcon({tabId, path});
   }
 
   function toggleSelector(tab) {
     return analytics.refreshTelemetryPref()
       .then(() => selectorLoader.toggle(tab.id, hasSeenOnboarding))
       .then(active => {
         setIconActive(active, tab.id);
         return active;
@@ -86,17 +75,18 @@ this.main = (function() {
       }
     });
   }
 
   function shouldOpenMyShots(url) {
     return /^about:(?:newtab|blank|home)/i.test(url) || /^resource:\/\/activity-streams\//i.test(url);
   }
 
-  // This is called by startBackground.js, directly in response to clicks on the Photon page action
+  // This is called by startBackground.js, where is registered as a click
+  // handler for the webextension page action.
   exports.onClicked = catcher.watchFunction((tab) => {
     catcher.watchPromise(hasSeenOnboarding.then(onboarded => {
       if (shouldOpenMyShots(tab.url)) {
         if (!onboarded) {
           catcher.watchPromise(analytics.refreshTelemetryPref().then(() => {
             sendEvent("goto-onboarding", "selection-button", {incognito: tab.incognito});
             return forceOnboarding();
           }));
@@ -285,21 +275,16 @@ this.main = (function() {
 
   communication.register("closeSelector", (sender) => {
     setIconActive(false, sender.tab.id);
   });
 
   communication.register("hasSeenOnboarding", () => {
     hasSeenOnboarding = Promise.resolve(true);
     catcher.watchPromise(browser.storage.local.set({hasSeenOnboarding: true}));
-    setIconActive(false, null);
-    startBackground.photonPageActionPort.postMessage({
-      type: "setProperties",
-      title: browser.i18n.getMessage("contextMenuLabel")
-    });
   });
 
   communication.register("abortStartShot", () => {
     // Note, we only show the error but don't report it, as we know that we can't
     // take shots of these pages:
     senderror.showError({
       popupMessage: "UNSHOOTABLE_PAGE"
     });
--- a/browser/extensions/screenshots/webextension/background/startBackground.js
+++ b/browser/extensions/screenshots/webextension/background/startBackground.js
@@ -1,11 +1,11 @@
 /* globals browser, main, communication */
 /* This file handles:
-     clicks on the Photon page action
+     clicks on the WebExtension 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
 */
 const startTime = Date.now();
 
 this.startBackground = (function() {
@@ -24,16 +24,24 @@ this.startBackground = (function() {
     "build/shot.js",
     "build/thumbnailGenerator.js",
     "background/analytics.js",
     "background/deviceInfo.js",
     "background/takeshot.js",
     "background/main.js"
   ];
 
+  browser.pageAction.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) => {
@@ -48,19 +56,16 @@ this.startBackground = (function() {
     loadIfNecessary().then(() => {
       return communication.onMessage(req, sender, sendResponse);
     }).catch((error) => {
       console.error("Error loading Screenshots:", error);
     });
     return true;
   });
 
-  let photonPageActionPort = null;
-  initPhotonPageAction();
-
   let loadedPromise;
 
   function loadIfNecessary() {
     if (loadedPromise) {
       return loadedPromise;
     }
     loadedPromise = Promise.resolve();
     backgroundScripts.forEach((script) => {
@@ -78,47 +83,10 @@ this.startBackground = (function() {
           };
           document.head.appendChild(tag);
         });
       });
     });
     return loadedPromise;
   }
 
-  function initPhotonPageAction() {
-    // 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.
-    photonPageActionPort = browser.runtime.connect({ name: "photonPageActionPort" });
-    photonPageActionPort.onMessage.addListener((message) => {
-      switch (message.type) {
-      case "click":
-        loadIfNecessary().then(() => {
-          return browser.tabs.get(message.tab.id);
-        }).then((tab) => {
-          main.onClicked(tab);
-        }).catch((error) => {
-          console.error("Error loading Screenshots:", error);
-        });
-        break;
-      default:
-        console.error("Unrecognized message:", message);
-        break;
-      }
-    });
-    photonPageActionPort.postMessage({
-      type: "setProperties",
-      title: browser.i18n.getMessage("contextMenuLabel")
-    });
-
-    // Export these so that main.js can use them.
-    Object.defineProperties(exports, {
-      "photonPageActionPort": {
-        enumerable: true,
-        get() {
-          return photonPageActionPort;
-        }
-      }
-    });
-  }
-
   return exports;
 })();
--- a/browser/extensions/screenshots/webextension/manifest.json
+++ b/browser/extensions/screenshots/webextension/manifest.json
@@ -25,16 +25,24 @@
         "build/buildSettings.js",
         "log.js",
         "catcher.js",
         "selector/callBackground.js",
         "sitehelper.js"
       ]
     }
   ],
+  "page_action": {
+    "browser_style": true,
+    "default_icon" : {
+      "32": "icons/icon-v2.svg"
+    },
+    "default_title": "__MSG_contextMenuLabel__",
+    "show_matches": ["<all_urls>"]
+  },
   "icons": {
     "32": "icons/icon-v2.svg"
   },
   "web_accessible_resources": [
     "blank.html",
     "icons/cancel.svg",
     "icons/download.svg",
     "icons/copy.svg",