Bug 1466575 - Re-enable Screenshots WebExtension page action for Firefox 64; r=ianbicking
authorJared Hirsch <ohai@6a68.net>
Wed, 12 Sep 2018 02:22:00 +0000
changeset 484304 c93a8b8b0c661526e3cc9f801120dfad4d8cec79
parent 484303 841a5d5dc1f81387c160c1f102df5d23d6a0678a
child 484305 3bf051cda7f624d987ec0925076fb639b1a6470f
push id241
push userfmarier@mozilla.com
push dateMon, 24 Sep 2018 21:48:02 +0000
reviewersianbicking
bugs1466575, 1483033
milestone64.0a1
Bug 1466575 - Re-enable Screenshots WebExtension page action for Firefox 64; r=ianbicking This patch was temporarily reverted to avoid bug 1483033 for Firefox 63. MozReview-Commit-ID: 4VaQgZQCVlE Differential Revision: https://phabricator.services.mozilla.com/D5621
browser/components/uitour/UITour.jsm
browser/components/uitour/test/browser_UITour_availableTargets.js
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/components/uitour/UITour.jsm
+++ b/browser/components/uitour/UITour.jsm
@@ -233,17 +233,17 @@ var UITour = {
         return aDocument.getElementById("pageAction-urlbar-sendToDevice") ||
                aDocument.getElementById("pageAction-panel-sendToDevice");
       },
     }],
     ["screenshots", {
       query: (aDocument) => {
         aDocument.ownerGlobal.BrowserPageActions.placeLazyActionsInPanel();
         return aDocument.getElementById("pageAction-urlbar-screenshots") ||
-               aDocument.getElementById("pageAction-panel-screenshots");
+               aDocument.getElementById("pageAction-panel-screenshots_mozilla_org");
       },
     }],
   ]),
 
   init() {
     log.debug("Initializing UITour");
     // Lazy getter is initialized here so it can be replicated any time
     // in a test.
--- a/browser/components/uitour/test/browser_UITour_availableTargets.js
+++ b/browser/components/uitour/test/browser_UITour_availableTargets.js
@@ -77,17 +77,17 @@ add_UITour_task(async function test_avai
 add_UITour_task(async function test_availableTargets_removeUrlbarPageActionsAll() {
   pageActionsHelper.setActionsUrlbarState(false);
   UITour.clearAvailableTargetsCache();
   let data = await getConfigurationPromise("availableTargets");
   let expecteds = getExpectedTargets();
   ok_targets(data, expecteds);
   let expectedActions = [
     [ "pocket", "pageAction-panel-pocket" ],
-    [ "screenshots", "pageAction-panel-screenshots" ],
+    [ "screenshots", "pageAction-panel-screenshots_mozilla_org" ],
     [ "pageAction-bookmark", "pageAction-panel-bookmark" ],
     [ "pageAction-copyURL", "pageAction-panel-copyURL" ],
     [ "pageAction-emailLink", "pageAction-panel-emailLink" ],
     [ "pageAction-sendToDevice", "pageAction-panel-sendToDevice" ],
   ];
   for (let [ targetName, expectedNodeId ] of expectedActions) {
     await assertTargetNode(targetName, expectedNodeId);
   }
@@ -97,17 +97,17 @@ add_UITour_task(async function test_avai
 add_UITour_task(async function test_availableTargets_addUrlbarPageActionsAll() {
   pageActionsHelper.setActionsUrlbarState(true);
   UITour.clearAvailableTargetsCache();
   let data = await getConfigurationPromise("availableTargets");
   let expecteds = getExpectedTargets();
   ok_targets(data, expecteds);
   let expectedActions = [
     [ "pocket", "pocket-button-box" ],
-    [ "screenshots", "pageAction-urlbar-screenshots" ],
+    [ "screenshots", "pageAction-panel-screenshots_mozilla_org" ],
     [ "pageAction-bookmark", "star-button-box" ],
     [ "pageAction-copyURL", "pageAction-urlbar-copyURL" ],
     [ "pageAction-emailLink", "pageAction-urlbar-emailLink" ],
     [ "pageAction-sendToDevice", "pageAction-urlbar-sendToDevice" ],
   ];
   for (let [ targetName, expectedNodeId ] of expectedActions) {
     await assertTargetNode(targetName, expectedNodeId);
   }
@@ -152,11 +152,11 @@ var pageActionsHelper = {
   },
 };
 
 function ensureScreenshotsEnabled() {
   SpecialPowers.pushPrefEnv({ set: [
     [ "extensions.screenshots.disabled", false ],
   ]});
   return BrowserTestUtils.waitForCondition(() => {
-    return PageActions.actionForID("screenshots");
+    return PageActions.actionForID("screenshots_mozilla_org");
   }, "Should enable Screenshots");
 }
--- 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;
 });
@@ -157,35 +155,30 @@ function handleStartup() {
     start(webExtension);
   }
 }
 
 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;
   }
@@ -206,62 +199,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/test/browser/browser_screenshots_ui_check.js
+++ b/browser/extensions/screenshots/test/browser/browser_screenshots_ui_check.js
@@ -1,11 +1,11 @@
 "use strict";
 
-const BUTTON_ID = "pageAction-panel-screenshots";
+const BUTTON_ID = "pageAction-panel-screenshots_mozilla_org";
 
 function checkElements(expectPresent, l) {
   for (const id of l) {
     is(!!document.getElementById(id), expectPresent, "element " + id + (expectPresent ? " is" : " is not") + " present");
   }
 }
 
 async function togglePageActionPanel() {
--- a/browser/extensions/screenshots/test/browser/head.js
+++ b/browser/extensions/screenshots/test/browser/head.js
@@ -11,17 +11,17 @@ function promiseScreenshotsEnabled() {
   if (!Services.prefs.getBoolPref("extensions.screenshots.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) => {
     const interval = setInterval(() => {
-      const action = PageActions.actionForID("screenshots");
+      const action = PageActions.actionForID("screenshots_mozilla_org");
       if (action) {
         info("screenshots page action created");
         clearInterval(interval);
         resolve(false);
       }
     }, 100);
     info("Set Screenshots disabled pref to false.");
     Services.prefs.setBoolPref("extensions.screenshots.disabled", false);
--- 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",