Bug 1412170 - Integrate WebExtension page action context menus with the Photon page action context menu: Photon page action changes. r?jaws draft
authorDrew Willcoxon <adw@mozilla.com>
Mon, 30 Oct 2017 17:47:54 -0400
changeset 688896 1ee007964539bef3ce8d5541bc075601bb04ed5f
parent 687856 018c7ef91d4c714f67ac60a427ef02d16bccc9da
child 688897 3eabcc1c9e60c9a5ceec19e364423d377b06ed58
push id86885
push userdwillcoxon@mozilla.com
push dateMon, 30 Oct 2017 21:55:06 +0000
reviewersjaws
bugs1412170
milestone58.0a1
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
browser/base/content/browser-pageActions.js
browser/base/content/browser.css
browser/base/content/browser.xul
browser/base/content/test/urlbar/browser_page_action_menu.js
browser/extensions/pocket/bootstrap.js
browser/modules/PageActions.jsm
browser/themes/shared/urlbar-searchbar.inc.css
--- a/browser/base/content/browser-pageActions.js
+++ b/browser/base/content/browser-pageActions.js
@@ -43,16 +43,34 @@ var BrowserPageActions = {
     return this.mainViewBodyNode = this.mainViewNode.querySelector(".panel-subview-body");
   },
 
   /**
    * Inits.  Call to init.
    */
   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);
   },
 
   /**
    * Places all registered actions.
    */
   placeAllActions() {
     // Place actions in the panel.  Notify of onBeforePlacedInWindow too.
     for (let action of PageActions.actions) {
@@ -123,16 +141,17 @@ var BrowserPageActions = {
     }
 
     let buttonNode = document.createElement("toolbarbutton");
     buttonNode.classList.add(
       "subviewbutton",
       "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]);
       }
     }
     let panelViewNode = null;
     if (action.subview) {
       buttonNode.classList.add("subviewbutton-nav");
@@ -386,18 +405,18 @@ 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);
     });
     if (action.nodeAttributes) {
       for (let name in action.nodeAttributes) {
         buttonNode.setAttribute(name, action.nodeAttributes[name]);
       }
     }
@@ -411,16 +430,17 @@ var BrowserPageActions = {
    * Removes all the DOM nodes of the given action.
    *
    * @param  action (PageActions.Action, required)
    *         The action to remove.
    */
   removeAction(action) {
     this._removeActionFromPanel(action);
     this._removeActionFromUrlbar(action);
+    action.onRemovedFromWindow(window);
   },
 
   _removeActionFromPanel(action) {
     let id = this.panelButtonNodeIDForActionID(action.id);
     let node = document.getElementById(id);
     if (node) {
       node.remove();
     }
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -1437,21 +1437,16 @@ toolbarpaletteitem[place="palette"][hidd
   .pageAction-panel-button > .toolbarbutton-icon {
     list-style-image: var(--pageAction-image-32px, inherit);
   }
   .urlbar-page-action {
     list-style-image: var(--pageAction-image-32px, inherit);
   }
 }
 
-.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);
   -moz-context-properties: fill;
   fill: currentColor;
   width: 16px;
   height: 16px;
 }
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -416,17 +416,17 @@
            copyURL-title="&pageAction.copyLink.label;"
            emailLink-title="&emailPageCmd.label;"
            sendToDevice-title="&pageAction.sendTabToDevice.label;"
            sendToDevice-notReadyTitle="&sendToDevice.syncNotReady.label;">
       <photonpanelmultiview id="pageActionPanelMultiView"
                             mainViewId="pageActionPanelMainView"
                             viewCacheId="appMenu-viewCache">
         <panelview id="pageActionPanelMainView"
-                   context="pageActionPanelContextMenu"
+                   context="pageActionContextMenu"
                    oncontextmenu="BrowserPageActions.onContextMenu(event);"
                    class="PanelUI-subView">
           <vbox class="panel-subview-body"/>
         </panelview>
       </photonpanelmultiview>
     </panel>
     <panel id="pageActionFeedback"
            role="alert"
@@ -439,19 +439,19 @@
            copyURLFeedback="&copyURLFeedback.label;"
            sendToDeviceFeedback="&sendToDeviceFeedback.label;">
       <hbox id="pageActionFeedbackAnimatableBox">
         <image id="pageActionFeedbackAnimatableImage"/>
       </hbox>
       <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;"
                 oncommand="BrowserPageActions.toggleShownInUrlbarForContextAction();"/>
     </menupopup>
 
     <!-- Bookmarks and history tooltip -->
     <tooltip id="bhTooltip"/>
@@ -856,17 +856,19 @@
                   <label id="identity-icon-label" class="plain" flex="1"/>
                   <label id="identity-icon-country-label" class="plain"/>
                 </hbox>
               </box>
               <box id="urlbar-display-box" align="center">
                 <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"/>
                 </hbox>
                 <image id="reader-mode-button"
                        class="urlbar-icon urlbar-page-action"
                        role="button"
                        hidden="true"
@@ -879,18 +881,16 @@
                 <image id="pageActionButton"
                        class="urlbar-icon urlbar-page-action"
                        role="button"
                        tooltiptext="&pageActionButton.tooltip;"
                        onmousedown="BrowserPageActions.mainButtonClicked(event);"/>
                 <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"
                          role="button"
                          observes="bookmarkThisPageBroadcaster"/>
                   <hbox id="star-button-animatable-box">
                     <image id="star-button-animatable-image"
                            role="presentation"
--- a/browser/base/content/test/urlbar/browser_page_action_menu.js
+++ b/browser/base/content/test/urlbar/browser_page_action_menu.js
@@ -630,99 +630,99 @@ add_task(async function sendToDevice_inU
 
 add_task(async function contextMenu() {
   // Open an actionable page so that the main page action button appears.
   let url = "http://example.com/";
   await BrowserTestUtils.withNewTab(url, async () => {
     // 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,
     });
     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;
 
     // The action should be removed from the urlbar.  In this case, the bookmark
     // star, the node in the urlbar should be hidden.
     let starButtonBox = document.getElementById("star-button-box");
     await BrowserTestUtils.waitForCondition(() => {
       return starButtonBox.hidden;
     }, "Waiting for star button to become hidden");
 
     // 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,
     });
     await contextMenuPromise;
 
     // The context menu should show "Add to Address Bar".  Click it.
     Assert.equal(contextMenuNode.childNodes.length, 1,
                  "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;
 
     // The action should be added to the urlbar.
     await BrowserTestUtils.waitForCondition(() => {
       return !starButtonBox.hidden;
     }, "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,
     });
     await contextMenuPromise;
 
     // The context menu should show "Remove from Address Bar".  Click it.
     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;
 
     // The action should be removed from the urlbar.
     await BrowserTestUtils.waitForCondition(() => {
       return starButtonBox.hidden;
     }, "Waiting for star button to become hidden");
 
     // 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,
     });
     await contextMenuPromise;
     Assert.equal(contextMenuNode.childNodes.length, 1,
                  "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(() => {
       return !starButtonBox.hidden;
     }, "Waiting for star button to become unhidden");
   });
 
   // urlbar tests that run after this one can break if the mouse is left over
--- a/browser/extensions/pocket/bootstrap.js
+++ b/browser/extensions/pocket/bootstrap.js
@@ -105,20 +105,16 @@ var PocketPageAction = {
 
           if (doc.getElementById("pocket-button-box")) {
             return;
           }
 
           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");
           animatableImage.id = "pocket-animatable-image";
           animatableImage.setAttribute("role", "presentation");
           let tooltip =
             gPocketBundle.GetStringFromName("pocket-button.tooltiptext");
           animatableImage.setAttribute("tooltiptext", tooltip);
--- a/browser/modules/PageActions.jsm
+++ b/browser/modules/PageActions.jsm
@@ -528,16 +528,20 @@ this.PageActions = {
  *        Called when the action is added to the page action panel in a browser
  *        window:
  *        onPlacedInPanel(buttonNode)
  *        * buttonNode: The action's node in the page action panel.
  * @param onPlacedInUrlbar (function, optional)
  *        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)
  *        * buttonNode: The action's node in the page action panel.
  * @param shownInUrlbar (bool, optional)
  *        Pass true to show the action in the urlbar, false otherwise.  False by
  *        default.
  * @param subview (object, optional)
@@ -566,16 +570,17 @@ function Action(options) {
     onBeforePlacedInWindow: false,
     onCommand: false,
     onIframeHiding: false,
     onIframeHidden: false,
     onIframeShown: false,
     onLocationChange: false,
     onPlacedInPanel: false,
     onPlacedInUrlbar: false,
+    onRemovedFromWindow: false,
     onShowingInPanel: false,
     shownInUrlbar: false,
     subview: false,
     tooltip: false,
     urlbarIDOverride: false,
     wantsIframe: false,
 
     // private
@@ -920,16 +925,29 @@ Action.prototype = {
    */
   onPlacedInUrlbar(buttonNode) {
     if (this._onPlacedInUrlbar) {
       this._onPlacedInUrlbar(buttonNode);
     }
   },
 
   /**
+   * 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.
    *
    * @param  buttonNode (DOM node, required)
    *         The action's panel button node.
    */
   onShowingInPanel(buttonNode) {
     if (this._onShowingInPanel) {
       this._onShowingInPanel(buttonNode);
--- a/browser/themes/shared/urlbar-searchbar.inc.css
+++ b/browser/themes/shared/urlbar-searchbar.inc.css
@@ -205,24 +205,24 @@
   height: 24px;
 }
 
 :root[uidensity=touch] .urlbar-icon {
   width: 30px;
   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);
 }
 
 .urlbar-icon-wrapper[open] > .urlbar-icon,
 .urlbar-icon-wrapper > .urlbar-icon:hover,
 .urlbar-icon-wrapper > .urlbar-icon:hover:active {
   background-color: transparent;