From 37cb69c21b066708206113df1fd2c6d5c957f501 Mon Sep 17 00:00:00 2001
From: Drew Willcoxon <adw@mozilla.com>
Date: Fri, 27 Oct 2017 17:39:57 -0400
Subject: [PATCH] Bug 1395387 - Reconcile WebExtension page actions and Photon
 page actions: WebExtensions changes. r=mixedpuppy

MozReview-Commit-ID: n2eR3q1aZF

--HG--
extra : rebase_source : 56a68a6a268e23c05d57960040be4a441566fa03
---
 browser/base/content/browser.css              |   8 -
 .../components/extensions/ExtensionPopups.jsm |  14 +-
 .../components/extensions/ext-pageAction.js   | 157 +++++-------------
 ...owser_ext_browserAction_pageAction_icon.js |   4 +-
 ...owserAction_pageAction_icon_permissions.js |   2 +-
 .../test/browser/browser_ext_menus.js         |   5 +-
 .../extensions/test/browser/head.js           |   8 +-
 .../test/browser/head_pageAction.js           |   4 +-
 8 files changed, 58 insertions(+), 144 deletions(-)

diff --git a/browser/base/content/browser.css b/browser/base/content/browser.css
index 8ab8ae191bbf1..ee29c6ed64010 100644
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -387,10 +387,6 @@ toolbarpaletteitem > toolbaritem[sdkstylewidget="true"][cui-areatype="toolbar"]
     list-style-image: var(--webextension-menupanel-image-dark, inherit);
   }
 
-  .webextension-page-action {
-    list-style-image: var(--webextension-urlbar-image, inherit);
-  }
-
   .webextension-menuitem {
     list-style-image: var(--webextension-menuitem-image, inherit);
   }
@@ -423,10 +419,6 @@ toolbarpaletteitem > toolbaritem[sdkstylewidget="true"][cui-areatype="toolbar"]
     list-style-image: var(--webextension-menupanel-image-2x-dark, inherit);
   }
 
-  .webextension-page-action {
-    list-style-image: var(--webextension-urlbar-image-2x, inherit);
-  }
-
   .webextension-menuitem {
     list-style-image: var(--webextension-menuitem-image-2x, inherit);
   }
diff --git a/browser/components/extensions/ExtensionPopups.jsm b/browser/components/extensions/ExtensionPopups.jsm
index cc02e48b32dd9..62bd1875aba71 100644
--- a/browser/components/extensions/ExtensionPopups.jsm
+++ b/browser/components/extensions/ExtensionPopups.jsm
@@ -374,9 +374,7 @@ class BasePopup {
 BasePopup.instances = new DefaultWeakMap(() => new WeakMap());
 
 class PanelPopup extends BasePopup {
-  constructor(extension, imageNode, popupURL, browserStyle) {
-    let document = imageNode.ownerDocument;
-
+  constructor(extension, document, popupURL, browserStyle) {
     let panel = document.createElement("panel");
     panel.setAttribute("id", makeWidgetId(extension.id) + "-panel");
     panel.setAttribute("class", "browser-extension-panel");
@@ -389,17 +387,15 @@ class PanelPopup extends BasePopup {
 
     document.getElementById("mainPopupSet").appendChild(panel);
 
-    super(extension, panel, popupURL, browserStyle);
-
-    this.contentReady.then(() => {
-      panel.openPopup(imageNode, "bottomcenter topright", 0, 0, false, false);
-
+    panel.addEventListener("popupshowing", () => {
       let event = new this.window.CustomEvent("WebExtPopupLoaded", {
         bubbles: true,
         detail: {extension},
       });
       this.browser.dispatchEvent(event);
-    });
+    }, {once: true});
+
+    super(extension, panel, popupURL, browserStyle);
   }
 
   get DESTROY_EVENT() {
diff --git a/browser/components/extensions/ext-pageAction.js b/browser/components/extensions/ext-pageAction.js
index b176f97e50957..1fec7875a4787 100644
--- a/browser/components/extensions/ext-pageAction.js
+++ b/browser/components/extensions/ext-pageAction.js
@@ -6,16 +6,14 @@
 /* import-globals-from ext-browserAction.js */
 /* import-globals-from ext-browser.js */
 
+XPCOMUtils.defineLazyModuleGetter(this, "PageActions",
+                                  "resource:///modules/PageActions.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PanelPopup",
                                   "resource:///modules/ExtensionPopups.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "TelemetryStopwatch",
                                   "resource://gre/modules/TelemetryStopwatch.jsm");
 
 
-var {
-  DefaultWeakMap,
-} = ExtensionUtils;
-
 Cu.import("resource://gre/modules/ExtensionParent.jsm");
 
 var {
@@ -37,9 +35,8 @@ this.pageAction = class extends ExtensionAPI {
     let {extension} = this;
     let options = extension.manifest.page_action;
 
-    this.iconData = new DefaultWeakMap(icons => this.getIconData(icons));
-
-    this.id = makeWidgetId(extension.id) + "-page-action";
+    let widgetId = makeWidgetId(extension.id);
+    this.id = widgetId + "-page-action";
 
     this.tabManager = extension.tabManager;
 
@@ -60,20 +57,24 @@ this.pageAction = class extends ExtensionAPI {
 
     this.tabContext.on("location-change", this.handleLocationChange.bind(this)); // eslint-disable-line mozilla/balanced-listeners
 
-    // WeakMap[ChromeWindow -> <xul:image>]
-    this.buttons = new WeakMap();
-
     pageActionMap.set(extension, this);
 
     this.defaults.icon = await StartupCache.get(
       extension, ["pageAction", "default_icon"],
       () => IconDetails.normalize({path: options.default_icon}, extension));
 
-    this.iconData.set(
-      this.defaults.icon,
-      await StartupCache.get(
-        extension, ["pageAction", "default_icon_data"],
-        () => this.getIconData(this.defaults.icon)));
+    if (!this.browserPageAction) {
+      this.browserPageAction = PageActions.addAction(new PageActions.Action({
+        id: widgetId,
+        title: this.defaults.title,
+        iconURL: this.defaults.icon,
+        shownInUrlbar: true,
+        disabled: true,
+        onCommand: (event, buttonNode) => {
+          this.handleClick(event.target.ownerGlobal);
+        },
+      }));
+    }
   }
 
   onShutdown(reason) {
@@ -81,11 +82,9 @@ this.pageAction = class extends ExtensionAPI {
 
     this.tabContext.shutdown();
 
-    for (let window of windowTracker.browserWindows()) {
-      if (this.buttons.has(window)) {
-        this.buttons.get(window).remove();
-        window.document.removeEventListener("popupshowing", this);
-      }
+    if (this.browserPageAction) {
+      this.browserPageAction.remove();
+      this.browserPageAction = null;
     }
   }
 
@@ -117,81 +116,24 @@ this.pageAction = class extends ExtensionAPI {
   //
   // Updates "tooltiptext" and "aria-label" to match "title" property.
   // Updates "image" to match the "icon" property.
-  // Shows or hides the icon, based on the "show" property.
+  // Enables or disables the icon, based on the "show" property.
   updateButton(window) {
     let tabData = this.tabContext.get(window.gBrowser.selectedTab);
-
-    if (!(tabData.show || this.buttons.has(window))) {
-      // Don't bother creating a button for a window until it actually
-      // needs to be shown.
-      return;
-    }
-
-    window.requestAnimationFrame(() => {
-      let button = this.getButton(window);
-
-      if (tabData.show) {
-        // Update the title and icon only if the button is visible.
-
-        let title = tabData.title || this.extension.name;
-        button.setAttribute("tooltiptext", title);
-        button.setAttribute("aria-label", title);
-        button.classList.add("webextension-page-action");
-
-        let {style} = this.iconData.get(tabData.icon);
-
-        button.setAttribute("style", style);
-      }
-
-      button.hidden = !tabData.show;
-    });
-  }
-
-  getIconData(icons) {
-    let getIcon = size => {
-      let {icon} = IconDetails.getPreferredIcon(icons, this.extension, size);
-      // TODO: implement theme based icon for pageAction (Bug 1398156)
-      return IconDetails.escapeUrl(icon);
-    };
-
-    let style = `
-      --webextension-urlbar-image: url("${getIcon(16)}");
-      --webextension-urlbar-image-2x: url("${getIcon(32)}");
-    `;
-
-    return {style};
-  }
-
-  // Create an |image| node and add it to the |page-action-buttons|
-  // container in the given window.
-  addButton(window) {
-    let document = window.document;
-
-    let button = document.createElement("image");
-    button.id = this.id;
-    button.setAttribute("class", "urlbar-icon");
-
-    button.addEventListener("click", this); // eslint-disable-line mozilla/balanced-listeners
-
-    if (this.extension.hasPermission("menus") ||
-        this.extension.hasPermission("contextMenus")) {
-      document.addEventListener("popupshowing", this);
-    }
-
-    document.getElementById("page-action-buttons").appendChild(button);
-
-    return button;
-  }
-
-  // Returns the page action button for the given window, creating it if
-  // it doesn't already exist.
-  getButton(window) {
-    if (!this.buttons.has(window)) {
-      let button = this.addButton(window);
-      this.buttons.set(window, button);
+    let title = tabData.title || this.extension.name;
+    this.browserPageAction.setTitle(title, window);
+    this.browserPageAction.setTooltip(title, window);
+    this.browserPageAction.setDisabled(!tabData.show, window);
+
+    let iconURL;
+    if (typeof(tabData.icon) == "string") {
+      iconURL = IconDetails.escapeUrl(tabData.icon);
+    } else {
+      iconURL = Object.entries(tabData.icon).reduce((memo, [size, url]) => {
+        memo[size] = IconDetails.escapeUrl(url);
+        return memo;
+      }, {});
     }
-
-    return this.buttons.get(window);
+    this.browserPageAction.setIconURL(iconURL, window);
   }
 
   /**
@@ -209,31 +151,6 @@ this.pageAction = class extends ExtensionAPI {
     }
   }
 
-  handleEvent(event) {
-    const window = event.target.ownerGlobal;
-
-    switch (event.type) {
-      case "click":
-        if (event.button === 0) {
-          this.handleClick(window);
-        }
-        break;
-
-      case "popupshowing":
-        const menu = event.target;
-        const trigger = menu.triggerNode;
-
-        if (menu.id === "toolbar-context-menu" && trigger && trigger.id === this.id) {
-          global.actionContextMenu({
-            extension: this.extension,
-            onPageAction: true,
-            menu: menu,
-          });
-        }
-        break;
-    }
-  }
-
   // Handles a click event on the page action button for the given
   // window.
   // If the page action has a |popup| property, a panel is opened to
@@ -251,9 +168,11 @@ this.pageAction = class extends ExtensionAPI {
     // If it has no popup URL defined, we dispatch a click event, but do not
     // open a popup.
     if (popupURL) {
-      let popup = new PanelPopup(this.extension, this.getButton(window),
-                                 popupURL, this.browserStyle);
+      let popup = new PanelPopup(this.extension, window.document, popupURL,
+                                 this.browserStyle);
       await popup.contentReady;
+      window.BrowserPageActions.togglePanelForAction(this.browserPageAction,
+                                                     popup.panel);
       TelemetryStopwatch.finish(popupOpenTimingHistogram, this);
     } else {
       TelemetryStopwatch.cancel(popupOpenTimingHistogram, this);
diff --git a/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js b/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js
index 5640c109bc537..2831fc64abd15 100644
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon.js
@@ -260,7 +260,7 @@ add_task(async function testDetailsObjects() {
 
   await extension.startup();
 
-  let pageActionId = `${makeWidgetId(extension.id)}-page-action`;
+  let pageActionId = BrowserPageActions.urlbarButtonNodeIDForActionID(makeWidgetId(extension.id));
   let browserActionWidget = getBrowserActionWidget(extension);
 
   let tests = await extension.awaitMessage("ready");
@@ -360,7 +360,7 @@ add_task(async function testPageActionIconLoadingOnBrowserActionThemedIcon() {
 
   await promiseAnimationFrame();
 
-  let pageActionId = `${makeWidgetId(extension.id)}-page-action`;
+  let pageActionId = BrowserPageActions.urlbarButtonNodeIDForActionID(makeWidgetId(extension.id));
   let pageActionImage = document.getElementById(pageActionId);
 
   const iconURL = new URL(getListStyleImage(pageActionImage));
diff --git a/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon_permissions.js b/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon_permissions.js
index a59706e49bc27..9282f67494cee 100644
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon_permissions.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon_permissions.js
@@ -102,7 +102,7 @@ add_task(async function testDefaultDetails() {
     await Promise.all([extension.startup(), extension.awaitMessage("ready")]);
 
     let browserActionId = makeWidgetId(extension.id) + "-browser-action";
-    let pageActionId = makeWidgetId(extension.id) + "-page-action";
+    let pageActionId = BrowserPageActions.urlbarButtonNodeIDForActionID(makeWidgetId(extension.id));
 
     await promiseAnimationFrame();
 
diff --git a/browser/components/extensions/test/browser/browser_ext_menus.js b/browser/components/extensions/test/browser/browser_ext_menus.js
index a9bb0d5b039d9..f283df4fb8e2f 100644
--- a/browser/components/extensions/test/browser/browser_ext_menus.js
+++ b/browser/components/extensions/test/browser/browser_ext_menus.js
@@ -68,7 +68,10 @@ add_task(async function test_actionContextMenus() {
   await extension.startup();
   const tabId = await extension.awaitMessage("ready");
 
-  for (const kind of ["page", "browser"]) {
+  // TODO bug 1412170: Allow WebExtensions to hook into the browser page action
+  // context menu.
+//   for (const kind of ["page", "browser"]) {
+  for (const kind of ["browser"]) {
     const menu = await openActionContextMenu(extension, kind);
     const [submenu, second, , , , last, separator] = menu.children;
 
diff --git a/browser/components/extensions/test/browser/head.js b/browser/components/extensions/test/browser/head.js
index 259a519d939d8..c4afa57e3450d 100644
--- a/browser/components/extensions/test/browser/head.js
+++ b/browser/components/extensions/test/browser/head.js
@@ -393,7 +393,10 @@ async function openActionContextMenu(extension, kind, win = window) {
   // See comment from clickPageAction below.
   SetPageProxyState("valid");
   await promiseAnimationFrame(win);
-  const id = `#${makeWidgetId(extension.id)}-${kind}-action`;
+  const id =
+    kind == "page" ?
+    `#${BrowserPageActions.urlbarButtonNodeIDForActionID(makeWidgetId(extension.id))}` :
+    `#${makeWidgetId(extension.id)}-${kind}-action`;
   return openChromeContextMenu("toolbar-context-menu", id, win);
 }
 
@@ -425,7 +428,8 @@ async function clickPageAction(extension, win = window) {
 
   await promiseAnimationFrame(win);
 
-  let pageActionId = makeWidgetId(extension.id) + "-page-action";
+  let pageActionId = BrowserPageActions.urlbarButtonNodeIDForActionID(makeWidgetId(extension.id));
+
   let elem = win.document.getElementById(pageActionId);
 
   EventUtils.synthesizeMouseAtCenter(elem, {}, win);
diff --git a/browser/components/extensions/test/browser/head_pageAction.js b/browser/components/extensions/test/browser/head_pageAction.js
index f42315e5e6412..bf569f7dd938e 100644
--- a/browser/components/extensions/test/browser/head_pageAction.js
+++ b/browser/components/extensions/test/browser/head_pageAction.js
@@ -105,7 +105,7 @@ async function runTests(options) {
   function checkDetails(details) {
     let image = currentWindow.document.getElementById(pageActionId);
     if (details == null) {
-      ok(image == null || image.hidden, "image is hidden");
+      ok(image == null || image.getAttribute("disabled") == "true", "image is disabled");
     } else {
       ok(image, "image exists");
 
@@ -123,7 +123,7 @@ async function runTests(options) {
   let awaitFinish = new Promise(resolve => {
     extension.onMessage("nextTest", async (expecting, testsRemaining) => {
       if (!pageActionId) {
-        pageActionId = `${makeWidgetId(extension.id)}-page-action`;
+        pageActionId = BrowserPageActions.urlbarButtonNodeIDForActionID(makeWidgetId(extension.id));
       }
 
       await promiseAnimationFrame(currentWindow);
-- 
GitLab