From 02b4c6e9c58188dfadf0eae9a5d854942336dddd Mon Sep 17 00:00:00 2001 From: Barry Chen Date: Tue, 1 May 2018 13:11:10 -0500 Subject: [PATCH] s/Photon/WebExtension/ for page action. (#3756) (#3967) --- addon/bootstrap.js | 61 ------------------- addon/webextension/background/main.js | 17 +----- .../background/startBackground.js | 50 +++------------ addon/webextension/manifest.json.template | 8 +++ test/test.js | 3 +- 5 files changed, 22 insertions(+), 117 deletions(-) diff --git a/addon/bootstrap.js b/addon/bootstrap.js index f03388b4b0..626eae9096 100644 --- a/addon/bootstrap.js +++ b/addon/bootstrap.js @@ -15,8 +15,6 @@ 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"); @@ -177,7 +175,6 @@ function start(webExtension) { return webExtension.startup(startupReason).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 @@ -192,10 +189,6 @@ function start(webExtension) { function stop(webExtension, reason) { if (reason !== APP_SHUTDOWN) { LibraryButton.uninit(); - if (photonPageAction) { - photonPageAction.remove(); - photonPageAction = null; - } } return Promise.resolve(webExtension.shutdown(reason)); } @@ -226,57 +219,3 @@ function handleMessage(msg, sender, sendReply) { } } } - -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; - } - }); - }); -} diff --git a/addon/webextension/background/main.js b/addon/webextension/background/main.js index e4d2edb5db..5d23d5544c 100644 --- a/addon/webextension/background/main.js +++ b/addon/webextension/background/main.js @@ -15,11 +15,6 @@ this.main = (function() { 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; @@ -53,10 +48,7 @@ this.main = (function() { 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) { @@ -91,7 +83,8 @@ this.main = (function() { 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)) { @@ -278,10 +271,6 @@ this.main = (function() { 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", () => { diff --git a/addon/webextension/background/startBackground.js b/addon/webextension/background/startBackground.js index c5335b2ed8..088b884321 100644 --- a/addon/webextension/background/startBackground.js +++ b/addon/webextension/background/startBackground.js @@ -1,6 +1,6 @@ /* 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 @@ -29,6 +29,14 @@ this.startBackground = (function() { "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"), @@ -53,9 +61,6 @@ this.startBackground = (function() { return true; }); - let photonPageActionPort = null; - initPhotonPageAction(); - let loadedPromise; function loadIfNecessary() { @@ -83,42 +88,5 @@ this.startBackground = (function() { 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; })(); diff --git a/addon/webextension/manifest.json.template b/addon/webextension/manifest.json.template index 2639279c4d..0b4e9bf907 100644 --- a/addon/webextension/manifest.json.template +++ b/addon/webextension/manifest.json.template @@ -30,6 +30,14 @@ ] } ], + "page_action": { + "browser_style": true, + "default_icon" : { + "32": "icons/icon-v2.svg" + }, + "default_title": "__MSG_contextMenuLabel__", + "show_matches": [""] + }, "icons": { "32": "icons/icon-v2.svg" }, diff --git a/test/test.js b/test/test.js index 31d21c7b34..e6dced8d0c 100644 --- a/test/test.js +++ b/test/test.js @@ -31,7 +31,8 @@ const fs = require("fs"); const util = require("util"); const PAGE_ACTION_BUTTON_ID = "pageActionButton"; -const SHOOTER_BUTTON_ID = "pageAction-panel-screenshots"; +// ID of WebExtension page action button +const SHOOTER_BUTTON_ID = "pageAction-panel-screenshots_mozilla_org"; // Applies to the old-style toolbar button: const TOOLBAR_SHOOTER_BUTTON_ID = "screenshots_mozilla_org-browser-action"; const pageActionButtonSelector = By.id(PAGE_ACTION_BUTTON_ID);