Bug 1543940 - Use the toolbox top window for context menus r=ochameau
authorJulian Descottes <jdescottes@mozilla.com>
Wed, 08 May 2019 21:36:04 +0000
changeset 531951 a0768b78ff32581c457122430db68e11a0a5c479
parent 531950 388f61b1b134177011dcfe55f52faff072a50052
child 531952 6907e6f93141299a40d3824c8cd8de8b00dcc765
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [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,
 };