Bug 1412170 - Integrate WebExtension page action context menus with the Photon page action context menu: Photon page action changes. r=jaws

MozReview-Commit-ID: BOEhQEJbUNO --HG-- extra : rebase_source : ff88d19669530269486d3441fe9cbebe6c73090b
2017-10-30 17:47:54 -04:00 · 2017-10-30 17:47:54 -04:00 · 13280ee095
--- a/browser/base/content/browser-pageActions.js
+++ b/browser/base/content/browser-pageActions.js
@ -48,6 +48,24 @@ var BrowserPageActions = {
   */
  init() {
    this.placeAllActions();
+
+    // Add a click listener to #page-action-buttons for blocking clicks on
+    // disabled actions in the urlbar.  Normally we'd do this by setting
+    // `pointer-events: none` in the CSS, but that also blocks context menu
+    // events, and we want the context menu even on disabled actions so that
+    // they can be removed from the urlbar.
+    this.mainButtonNode.parentNode.addEventListener("click", event => {
+      if (event.button == 2) {
+        // Let context-clicks be handled normally.
+        return;
+      }
+      let node = event.originalTarget;
+      let action = this.actionForNode(node);
+      if (action && action.getDisabled(window)) {
+        event.preventDefault();
+        event.stopPropagation();
+      }
+    }, true);
  },

  /**
@ -128,6 +146,7 @@ var BrowserPageActions = {
      "subviewbutton-iconic",
      "pageAction-panel-button"
    );
+    buttonNode.setAttribute("actionid", action.id);
    if (action.nodeAttributes) {
      for (let name in action.nodeAttributes) {
        buttonNode.setAttribute(name, action.nodeAttributes[name]);
@ -391,8 +410,8 @@ var BrowserPageActions = {
  _makeUrlbarButtonNode(action) {
    let buttonNode = document.createElement("image");
    buttonNode.classList.add("urlbar-icon", "urlbar-page-action");
+    buttonNode.setAttribute("actionid", action.id);
    buttonNode.setAttribute("role", "button");
-    buttonNode.setAttribute("context", "pageActionPanelContextMenu");
    buttonNode.addEventListener("contextmenu", event => {
      BrowserPageActions.onContextMenu(event);
    });
@ -416,6 +435,7 @@ var BrowserPageActions = {
  removeAction(action) {
    this._removeActionFromPanel(action);
    this._removeActionFromUrlbar(action);
+    action.onRemovedFromWindow(window);
  },

  _removeActionFromPanel(action) {
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@ -1443,11 +1443,6 @@ toolbarpaletteitem[place="palette"][hidden] {
  }
 }

-.urlbar-page-action[disabled] {
-  pointer-events: none;
-  -moz-user-focus: ignore;
-}
-
 /* WebExtension Sidebars */
 #sidebar-box[sidebarcommand$="-sidebar-action"] > #sidebar-header > #sidebar-switcher-target > #sidebar-icon {
  list-style-image: var(--webextension-menuitem-image, inherit);
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@ -421,7 +421,7 @@
                            mainViewId="pageActionPanelMainView"
                            viewCacheId="appMenu-viewCache">
        <panelview id="pageActionPanelMainView"
-                   context="pageActionPanelContextMenu"
+                   context="pageActionContextMenu"
                   oncontextmenu="BrowserPageActions.onContextMenu(event);"
                   class="PanelUI-subView">
          <vbox class="panel-subview-body"/>
@ -444,9 +444,9 @@
      <label id="pageActionFeedbackMessage"/>
    </panel>

-    <menupopup id="pageActionPanelContextMenu"
+    <menupopup id="pageActionContextMenu"
               onpopupshowing="BrowserPageActions.onContextMenuShowing(event, this);">
-      <menuitem id="pageActionPanelContextMenu-toggleUrlbar"
+      <menuitem id="pageActionContextMenu-toggleUrlbar"
                add-label="&pageAction.addToUrlbar.label;"
                remove-label="&pageAction.removeFromUrlbar.label;"
                label="&pageAction.addToUrlbar.label;"
@ -859,7 +859,9 @@
                <label id="switchtab" class="urlbar-display urlbar-display-switchtab" value="&urlbar.switchToTab.label;"/>
                <label id="extension" class="urlbar-display urlbar-display-extension" value="&urlbar.extension.label;"/>
              </box>
-              <hbox id="page-action-buttons">
+              <hbox id="page-action-buttons"
+                    context="pageActionContextMenu"
+                    oncontextmenu="BrowserPageActions.onContextMenu(event);">
                <hbox id="userContext-icons" hidden="true">
                  <label id="userContext-label"/>
                  <image id="userContext-indicator"/>
@ -882,8 +884,6 @@
                <hbox id="star-button-box"
                      hidden="true"
                      class="urlbar-icon-wrapper urlbar-page-action"
-                      context="pageActionPanelContextMenu"
-                      oncontextmenu="BrowserPageActions.onContextMenu(event);"
                      onclick="BrowserPageActions.doCommandForAction(PageActions.actionForID('bookmark'), event, this);">
                  <image id="star-button"
                         class="urlbar-icon"
--- a/browser/base/content/test/urlbar/browser_page_action_menu.js
+++ b/browser/base/content/test/urlbar/browser_page_action_menu.js
@ -635,7 +635,7 @@ add_task(async function contextMenu() {
    // Open the panel and then open the context menu on the bookmark button.
    await promisePageActionPanelOpen();
    let bookmarkButton = document.getElementById("pageAction-panel-bookmark");
-    let contextMenuPromise = promisePanelShown("pageActionPanelContextMenu");
+    let contextMenuPromise = promisePanelShown("pageActionContextMenu");
    EventUtils.synthesizeMouseAtCenter(bookmarkButton, {
      type: "contextmenu",
      button: 2,
@ -643,12 +643,12 @@ add_task(async function contextMenu() {
    await contextMenuPromise;

    // The context menu should show "Remove from Address Bar".  Click it.
-    let contextMenuNode = document.getElementById("pageActionPanelContextMenu");
+    let contextMenuNode = document.getElementById("pageActionContextMenu");
    Assert.equal(contextMenuNode.childNodes.length, 1,
                 "Context menu has one child");
    Assert.equal(contextMenuNode.childNodes[0].label, "Remove from Address Bar",
                 "Context menu is in the 'remove' state");
-    contextMenuPromise = promisePanelHidden("pageActionPanelContextMenu");
+    contextMenuPromise = promisePanelHidden("pageActionContextMenu");
    EventUtils.synthesizeMouseAtCenter(contextMenuNode.childNodes[0], {});
    await contextMenuPromise;

@ -661,7 +661,7 @@ add_task(async function contextMenu() {

    // Open the context menu again on the bookmark button.  (The page action
    // panel remains open.)
-    contextMenuPromise = promisePanelShown("pageActionPanelContextMenu");
+    contextMenuPromise = promisePanelShown("pageActionContextMenu");
    EventUtils.synthesizeMouseAtCenter(bookmarkButton, {
      type: "contextmenu",
      button: 2,
@ -673,7 +673,7 @@ add_task(async function contextMenu() {
                 "Context menu has one child");
    Assert.equal(contextMenuNode.childNodes[0].label, "Add to Address Bar",
                 "Context menu is in the 'add' state");
-    contextMenuPromise = promisePanelHidden("pageActionPanelContextMenu");
+    contextMenuPromise = promisePanelHidden("pageActionContextMenu");
    EventUtils.synthesizeMouseAtCenter(contextMenuNode.childNodes[0], {});
    await contextMenuPromise;

@ -683,7 +683,7 @@ add_task(async function contextMenu() {
    }, "Waiting for star button to become unhidden");

    // Open the context menu on the bookmark star in the urlbar.
-    contextMenuPromise = promisePanelShown("pageActionPanelContextMenu");
+    contextMenuPromise = promisePanelShown("pageActionContextMenu");
    EventUtils.synthesizeMouseAtCenter(starButtonBox, {
      type: "contextmenu",
      button: 2,
@ -695,7 +695,7 @@ add_task(async function contextMenu() {
                 "Context menu has one child");
    Assert.equal(contextMenuNode.childNodes[0].label, "Remove from Address Bar",
                 "Context menu is in the 'remove' state");
-    contextMenuPromise = promisePanelHidden("pageActionPanelContextMenu");
+    contextMenuPromise = promisePanelHidden("pageActionContextMenu");
    EventUtils.synthesizeMouseAtCenter(contextMenuNode.childNodes[0], {});
    await contextMenuPromise;

@ -707,7 +707,7 @@ add_task(async function contextMenu() {
    // Finally, add the bookmark star back to the urlbar so that other tests
    // that rely on it are OK.
    await promisePageActionPanelOpen();
-    contextMenuPromise = promisePanelShown("pageActionPanelContextMenu");
+    contextMenuPromise = promisePanelShown("pageActionContextMenu");
    EventUtils.synthesizeMouseAtCenter(bookmarkButton, {
      type: "contextmenu",
      button: 2,
@ -717,7 +717,7 @@ add_task(async function contextMenu() {
                 "Context menu has one child");
    Assert.equal(contextMenuNode.childNodes[0].label, "Add to Address Bar",
                 "Context menu is in the 'add' state");
-    contextMenuPromise = promisePanelHidden("pageActionPanelContextMenu");
+    contextMenuPromise = promisePanelHidden("pageActionContextMenu");
    EventUtils.synthesizeMouseAtCenter(contextMenuNode.childNodes[0], {});
    await contextMenuPromise;
    await BrowserTestUtils.waitForCondition(() => {
--- a/browser/extensions/pocket/bootstrap.js
+++ b/browser/extensions/pocket/bootstrap.js
@ -110,10 +110,6 @@ var PocketPageAction = {
          let wrapper = doc.createElement("hbox");
          wrapper.id = "pocket-button-box";
          wrapper.classList.add("urlbar-icon-wrapper", "urlbar-page-action");
-          wrapper.setAttribute("context", "pageActionPanelContextMenu");
-          wrapper.addEventListener("contextmenu", event => {
-            window.BrowserPageActions.onContextMenu(event);
-          });
          let animatableBox = doc.createElement("hbox");
          animatableBox.id = "pocket-animatable-box";
          let animatableImage = doc.createElement("image");
--- a/browser/modules/PageActions.jsm
+++ b/browser/modules/PageActions.jsm
@ -533,6 +533,10 @@ this.PageActions = {
 *        Called when the action is added to the urlbar in a browser window:
 *        onPlacedInUrlbar(buttonNode)
 *        * buttonNode: The action's node in the urlbar.
+ * @param onRemovedFromWindow (function, optional)
+ *        Called after the action is removed from a browser window:
+ *        onRemovedFromWindow(browserWindow)
+ *        * browserWindow: The browser window that the action was removed from.
 * @param onShowingInPanel (function, optional)
 *        Called when a browser window's page action panel is showing:
 *        onShowingInPanel(buttonNode)
@ -571,6 +575,7 @@ function Action(options) {
    onLocationChange: false,
    onPlacedInPanel: false,
    onPlacedInUrlbar: false,
+    onRemovedFromWindow: false,
    onShowingInPanel: false,
    shownInUrlbar: false,
    subview: false,
@ -924,6 +929,19 @@ Action.prototype = {
    }
  },

+  /**
+   * Call this when the DOM nodes for the action are removed from a browser
+   * window.
+   *
+   * @param  browserWindow (DOM window, required)
+   *         The browser window the action was removed from.
+   */
+  onRemovedFromWindow(browserWindow) {
+    if (this._onRemovedFromWindow) {
+      this._onRemovedFromWindow(browserWindow);
+    }
+  },
+
  /**
   * Call this when the action's button is shown in the page action panel.
   *
--- a/browser/themes/shared/urlbar-searchbar.inc.css
+++ b/browser/themes/shared/urlbar-searchbar.inc.css
@ -210,14 +210,14 @@
  height: 30px;
 }

-.urlbar-icon:hover,
-.urlbar-icon-wrapper:hover {
+.urlbar-icon:not([disabled]):hover,
+.urlbar-icon-wrapper:not([disabled]):hover {
  background-color: hsla(0,0%,80%,.4);
 }

 .urlbar-icon[open],
 .urlbar-icon-wrapper[open],
-.urlbar-icon:hover:active,
+.urlbar-icon:not([disabled]):hover:active,
 .urlbar-icon-wrapper:hover:active {
  background-color: hsla(0,0%,80%,.6);
 }