Commit 6c24dca7 authored by Julian Descottes's avatar Julian Descottes
Browse files

Bug 1543940 - menu.popup() should take a document argument instead of toolbox r=ochameau 

Depends on D27693

Menu::popup and popupAtZoom are expecting a toolbox argument as last argument.
However, half of the callsites do not have access to the toolbox and just pass
a { doc } object. This is misleading when trying to work on menu.js because you
cannot rely on toolbox APIs.

Differential Revision: https://phabricator.services.mozilla.com/D28036

--HG--
extra : moz-landing-system : lando
parent 2f080ac3

devtools/client/accessibility/components/AccessibilityRow.js

+1 −1
Original line number Original line Diff line number Diff line
@@ -203,7 +203,7 @@ class AccessibilityRow extends Component { 
      }));

      }));

    }

    }





    menu.popup(e.screenX, e.screenY, gToolbox);

    menu.popup(e.screenX, e.screenY, gToolbox.doc);





    if (gTelemetry) {

    if (gTelemetry) {

      gTelemetry.scalarAdd(TELEMETRY_ACCESSIBLE_CONTEXT_MENU_OPENED, 1);

      gTelemetry.scalarAdd(TELEMETRY_ACCESSIBLE_CONTEXT_MENU_OPENED, 1);

devtools/client/framework/menu.js

+7 −9
Original line number Original line Diff line number Diff line
@@ -59,11 +59,11 @@ Menu.prototype.insert = function(pos, menuItem) { 
 *

 *

 * @param {int} x

 * @param {int} x

 * @param {int} y

 * @param {int} y

 * @param Toolbox toolbox

 * @param {Document} doc

 */

 */

Menu.prototype.popupWithZoom = function(x, y, toolbox) {

Menu.prototype.popupWithZoom = function(x, y, doc) {

  const zoom = getCurrentZoom(toolbox.doc);

  const zoom = getCurrentZoom(doc);

  this.popup(x * zoom, y * zoom, toolbox);

  this.popup(x * zoom, y * zoom, doc);

};

};





/**

/**
@@ -75,12 +75,10 @@ Menu.prototype.popupWithZoom = function(x, y, toolbox) { 
 *

 *

 * @param {int} screenX

 * @param {int} screenX

 * @param {int} screenY

 * @param {int} screenY

 * @param Toolbox toolbox (non standard)

 * @param {Document} doc

 *        Needed so we in which window to inject XUL

 *        The document that should own the context menu.

 */

 */

Menu.prototype.popup = function(screenX, screenY, toolbox) {

Menu.prototype.popup = function(screenX, screenY, doc) {

  const doc = toolbox.doc;



  let popupset = doc.querySelector("popupset");

  let popupset = doc.querySelector("popupset");

  if (!popupset) {

  if (!popupset) {

    popupset = doc.createXULElement("popupset");

    popupset = doc.createXULElement("popupset");

devtools/client/framework/test/browser_menu_api.js

+2 −2
Original line number Original line Diff line number Diff line
@@ -80,7 +80,7 @@ async function testMenuPopup(toolbox) { 
    visible: false,

    visible: false,

  }));

  }));





  menu.popup(0, 0, toolbox);

  menu.popup(0, 0, toolbox.doc);





  ok(toolbox.doc.querySelector("#menu-popup"), "A popup is in the DOM");

  ok(toolbox.doc.querySelector("#menu-popup"), "A popup is in the DOM");
@@ -143,7 +143,7 @@ async function testSubmenu(toolbox) { 
    disabled: true,

    disabled: true,

  }));

  }));





  menu.popup(0, 0, toolbox);

  menu.popup(0, 0, toolbox.doc);

  ok(toolbox.doc.querySelector("#menu-popup"), "A popup is in the DOM");

  ok(toolbox.doc.querySelector("#menu-popup"), "A popup is in the DOM");

  is(toolbox.doc.querySelectorAll("#menu-popup > menuitem").length, 0,

  is(toolbox.doc.querySelectorAll("#menu-popup > menuitem").length, 0,

    "No menuitem children");

    "No menuitem children");

devtools/client/framework/toolbox.js

+1 −1
Original line number Original line Diff line number Diff line
@@ -3253,7 +3253,7 @@ Toolbox.prototype = { 
    menu.once("open", () => this.emit("menu-open"));

    menu.once("open", () => this.emit("menu-open"));

    menu.once("close", () => this.emit("menu-close"));

    menu.once("close", () => this.emit("menu-close"));





    menu.popup(x, y, { doc: this.doc });

    menu.popup(x, y, this.doc);

  },

  },





  /**

  /**

devtools/client/inspector/changes/ChangesContextMenu.js

+1 −1
Original line number Original line Diff line number Diff line
@@ -87,7 +87,7 @@ class ChangesContextMenu { 
    });

    });

    menu.append(menuitemSelectAll);

    menu.append(menuitemSelectAll);





    menu.popup(screenX, screenY, this.inspector.toolbox);

    menu.popup(screenX, screenY, this.inspector.toolbox.doc);

    return menu;

    return menu;

  }

  }