Bug 1543940 - Use the toolbox top window for context menus r=ochameau
☠☠ backed out by a51c08b24de3 ☠ ☠
authorJulian Descottes <jdescottes@mozilla.com>
Wed, 08 May 2019 15:14:05 +0000
changeset 473079 a66967f1704278ebfee2dfd560c2ccba84953395
parent 473078 b1e6e932873c26533fe57f4d87e4c99f45f4097b
child 473080 42e2136f684fd55780c7a8e63227c0bc64e4b6ef
push id113065
push useropoprus@mozilla.com
push dateThu, 09 May 2019 03:46:59 +0000
treeherdermozilla-inbound@34a824c75b7b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1543940
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1543940 - Use the toolbox top window for context menus r=ochameau Depends on D28036 If a context menu is opened in the toolbox document when running in a frame with type=content, keyboard navigation will not move to the context menu when it's opened. Differential Revision: https://phabricator.services.mozilla.com/D27695
devtools/client/framework/menu.js
devtools/client/framework/toolbox.js
devtools/client/inspector/markup/markup-context-menu.js
devtools/client/shared/components/menu/utils.js
--- a/devtools/client/framework/menu.js
+++ b/devtools/client/framework/menu.js
@@ -1,16 +1,17 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
+const DevToolsUtils = require("devtools/shared/DevToolsUtils");
 const EventEmitter = require("devtools/shared/event-emitter");
 const { getCurrentZoom } = require("devtools/shared/layout/utils");
 
 /**
  * A partial implementation of the Menu API provided by electron:
  * https://github.com/electron/electron/blob/master/docs/api/menu.md.
  *
  * Extra features:
@@ -74,16 +75,22 @@ Menu.prototype.popupWithZoom = function(
  *   - positioningItem Number - (optional) OS X
  *
  * @param {int} screenX
  * @param {int} screenY
  * @param {Document} doc
  *        The document that should own the context menu.
  */
 Menu.prototype.popup = function(screenX, screenY, doc) {
+  // The context-menu will be created in the topmost window to preserve keyboard
+  // navigation (see Bug 1543940).
+  // Keep a reference on the window owning the menu to hide the popup on unload.
+  const win = doc.defaultView;
+  doc = DevToolsUtils.getTopWindow(doc.defaultView).document;
+
   let popupset = doc.querySelector("popupset");
   if (!popupset) {
     popupset = doc.createXULElement("popupset");
     doc.documentElement.appendChild(popupset);
   }
   // See bug 1285229, on Windows, opening the same popup multiple times in a
   // row ends up duplicating the popup. The newly inserted popup doesn't
   // dismiss the old one. So remove any previously displayed popup before
@@ -98,19 +105,25 @@ Menu.prototype.popup = function(screenX,
   popup.setAttribute("consumeoutsideclicks", "false");
   popup.setAttribute("incontentshell", "false");
 
   if (this.id) {
     popup.id = this.id;
   }
   this._createMenuItems(popup);
 
+  // The context menu will be created in the topmost chrome window. Hide it manually when
+  // the owner document is unloaded.
+  const onWindowUnload = () => popup.hidePopup();
+  win.addEventListener("unload", onWindowUnload);
+
   // Remove the menu from the DOM once it's hidden.
   popup.addEventListener("popuphidden", (e) => {
     if (e.target === popup) {
+      win.removeEventListener("unload", onWindowUnload);
       popup.remove();
       this.emit("close");
     }
   });
 
   popup.addEventListener("popupshown", (e) => {
     if (e.target === popup) {
       this.emit("open");
@@ -152,16 +165,21 @@ Menu.prototype._createMenuItems = functi
         item.hover();
       });
 
       parent.appendChild(menuitem);
     }
   });
 };
 
+Menu.getMenuElementById = function(id, doc) {
+  const menuDoc = DevToolsUtils.getTopWindow(doc.defaultView).document;
+  return menuDoc.getElementById(id);
+};
+
 Menu.setApplicationMenu = () => {
   throw Error("Not implemented");
 };
 
 Menu.sendActionToFirstResponder = () => {
   throw Error("Not implemented");
 };
 
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -386,16 +386,20 @@ Toolbox.prototype = {
    * When the toolbox is loaded in a frame with type="content", win.parent will not return
    * the parent Chrome window. This getter should return the parent Chrome window
    * regardless of the frame type. See Bug 1539979.
    */
   get topWindow() {
     return DevToolsUtils.getTopWindow(this.win);
   },
 
+  get topDoc() {
+    return this.topWindow.document;
+  },
+
   /**
    * Shortcut to the document containing the toolbox UI
    */
   get doc() {
     return this.win.document;
   },
 
   /**
@@ -3242,17 +3246,17 @@ Toolbox.prototype = {
   /**
    * Open the textbox context menu at given coordinates.
    * Panels in the toolbox can call this on contextmenu events with event.screenX/Y
    * instead of having to implement their own copy/paste/selectAll menu.
    * @param {Number} x
    * @param {Number} y
    */
   openTextBoxContextMenu: function(x, y) {
-    const menu = createEditContextMenu(this.win, "toolbox-menu");
+    const menu = createEditContextMenu(this.topWindow, "toolbox-menu");
 
     // Fire event for tests
     menu.once("open", () => this.emit("menu-open"));
     menu.once("close", () => this.emit("menu-close"));
 
     menu.popup(x, y, this.doc);
   },
 
--- a/devtools/client/inspector/markup/markup-context-menu.js
+++ b/devtools/client/inspector/markup/markup-context-menu.js
@@ -763,16 +763,17 @@ class MarkupContextMenu {
         error => console.warn(error));
     if (!hasMethod) {
       return;
     }
 
     const hasA11YProps = await this.walker.hasAccessibilityProperties(
       this.selection.nodeFront);
     if (hasA11YProps) {
-      this.toolbox.doc.getElementById(menuItem.id).disabled = menuItem.disabled = false;
+      const menuItemEl = Menu.getMenuElementById(menuItem.id, this.toolbox.doc);
+      menuItemEl.disabled = menuItem.disabled = false;
     }
 
     this.inspector.emit("node-menu-updated");
   }
 }
 
 module.exports = MarkupContextMenu;
--- a/devtools/client/shared/components/menu/utils.js
+++ b/devtools/client/shared/components/menu/utils.js
@@ -63,17 +63,17 @@ function showMenu(items, options) {
     screenX = rect.left + defaultView.mozInnerScreenX;
     screenY = rect.bottom + defaultView.mozInnerScreenY;
   }
 
   let doc;
   if (options.useTopLevelWindow) {
     doc = getTopLevelWindow(window).document;
   } else {
-    doc = window.parent.document;
+    doc = window.document;
   }
 
   menu.popup(screenX, screenY, doc);
 }
 
 module.exports = {
   showMenu,
 };