From b3522226e65da173000f836d28897dce10ffe7c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 2 Nov 2021 11:03:51 +0000 Subject: [PATCH] Bug 1738265 - Explicitly close some devtools menus in some tests. r=nchevobbe,devtools-reviewers With the negative margin, some context menus that remain open start getting events that would otherwise go to the content underneath (and which the affected tests need to pass). Close the context menus explicitly in the affected tests (this is what would usually happen if a user clicked a context menu item anyways). Differential Revision: https://phabricator.services.mozilla.com/D129867 --- .../browser_accessibility_print_to_json.js | 13 +++++-- .../test/mochitest/browser_dbg-watchpoints.js | 10 +++--- .../client/debugger/test/mochitest/helpers.js | 2 ++ devtools/client/framework/menu.js | 34 +++++++++++++------ .../test/browser_changes_copy_declaration.js | 3 ++ ...onsole_context_menu_copy_entire_message.js | 5 ++- ..._copy_message_with_framework_stacktrace.js | 5 ++- ...sole_context_menu_export_console_output.js | 4 +++ 8 files changed, 58 insertions(+), 18 deletions(-) diff --git a/devtools/client/accessibility/test/browser/browser_accessibility_print_to_json.js b/devtools/client/accessibility/test/browser/browser_accessibility_print_to_json.js index 5b91684cc671..df26ab1ae734 100644 --- a/devtools/client/accessibility/test/browser/browser_accessibility_print_to_json.js +++ b/devtools/client/accessibility/test/browser/browser_accessibility_print_to_json.js @@ -8,7 +8,10 @@ const TEST_URI = "

Top level header

"; function getMenuItems(toolbox) { const menuDoc = toolbox.doc.defaultView.windowRoot.ownerGlobal.document; const menu = menuDoc.getElementById("accessibility-row-contextmenu"); - return [...menu.getElementsByTagName("menuitem")]; + return { + menu, + items: [...menu.getElementsByTagName("menuitem")], + }; } async function newTabSelected(tab) { @@ -35,9 +38,15 @@ async function checkJSONSnapshotForRow({ doc, tab, toolbox }, index, expected) { ); info(`Triggering "Print To JSON" menu item for row ${index}.`); - const [printToJSON] = getMenuItems(toolbox); + const { + menu, + items: [printToJSON], + } = getMenuItems(toolbox); printToJSON.click(); + // Note that click() above doesn't close the menupopup. + menu.hidePopup(); + const jsonViewTab = await newTabSelected(tab); Assert.deepEqual( parseSnapshotFromTabURI(jsonViewTab), diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-watchpoints.js b/devtools/client/debugger/test/mochitest/browser_dbg-watchpoints.js index 8d886aebca23..259a35ca2da0 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-watchpoints.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-watchpoints.js @@ -21,11 +21,12 @@ add_task(async function() { await toggleScopeNode(dbg, 3); const addedWatchpoint = waitForDispatch(dbg.store, "SET_WATCHPOINT"); await rightClickScopeNode(dbg, 5); - await waitForContextMenu(dbg); + let popup = await waitForContextMenu(dbg); let submenu = await openContextMenuSubmenu(dbg, selectors.watchpointsSubmenu); const getWatchpointItem = document.querySelector(selectors.addGetWatchpoint); submenu.activateItem(getWatchpointItem); await addedWatchpoint; + popup.hidePopup(); await resume(dbg); await waitForPaused(dbg); @@ -61,11 +62,12 @@ add_task(async function() { await toggleScopeNode(dbg, 4); const addedWatchpoint2 = waitForDispatch(dbg.store, "SET_WATCHPOINT"); await rightClickScopeNode(dbg, 6); - await waitForContextMenu(dbg); + popup = await waitForContextMenu(dbg); submenu = await openContextMenuSubmenu(dbg, selectors.watchpointsSubmenu); const getWatchpointItem2 = document.querySelector(selectors.addGetWatchpoint); submenu.activateItem(getWatchpointItem2); await addedWatchpoint2; + popup.hidePopup(); info("Resume and wait to pause at the access to b in getB"); await resume(dbg); @@ -81,7 +83,6 @@ add_task(async function() { info("Remove the get watchpoint on b"); const removedWatchpoint2 = waitForDispatch(dbg.store, "REMOVE_WATCHPOINT"); await toggleScopeNode(dbg, 3); - await rightClickScopeNode(dbg, 5); const el2 = await waitForElementWithSelector(dbg, ".remove-get-watchpoint"); el2.scrollIntoView(); clickElementWithSelector(dbg, ".remove-get-watchpoint"); @@ -90,11 +91,12 @@ add_task(async function() { info("Add back the get watchpoint on b"); const addedWatchpoint3 = waitForDispatch(dbg.store, "SET_WATCHPOINT"); await rightClickScopeNode(dbg, 5); - await waitForContextMenu(dbg); + popup = await waitForContextMenu(dbg); submenu = await openContextMenuSubmenu(dbg, selectors.watchpointsSubmenu); const getWatchpointItem3 = document.querySelector(selectors.addGetWatchpoint); submenu.activateItem(getWatchpointItem3); await addedWatchpoint3; + popup.hidePopup(); info("Resume and wait to pause on the final `obj.b;`"); await resume(dbg); diff --git a/devtools/client/debugger/test/mochitest/helpers.js b/devtools/client/debugger/test/mochitest/helpers.js index 6f929a917ac6..20457801678b 100644 --- a/devtools/client/debugger/test/mochitest/helpers.js +++ b/devtools/client/debugger/test/mochitest/helpers.js @@ -1582,6 +1582,8 @@ async function waitForContextMenu(dbg) { await new Promise(resolve => { popup.addEventListener("popupshown", () => resolve(), { once: true }); }); + + return popup; } function selectContextMenuItem(dbg, selector) { diff --git a/devtools/client/framework/menu.js b/devtools/client/framework/menu.js index 184b36099cda..887a9ac26094 100644 --- a/devtools/client/framework/menu.js +++ b/devtools/client/framework/menu.js @@ -76,6 +76,22 @@ Menu.prototype.popupAtTarget = function(target, doc) { this.popup(x * zoom, y * zoom, doc); }; +/** + * Hide an existing menu, if there's any. + * + * @param {Document} doc + * The document that should own the context menu. + */ +Menu.prototype.hide = function(doc) { + const win = doc.defaultView; + doc = DevToolsUtils.getTopWindow(win).document; + const popup = doc.querySelector('popupset menupopup[menu-api="true"]'); + if (!popup) { + return; + } + popup.hidePopup(); +}; + /** * Show the Menu at a specified location on the screen * @@ -89,27 +105,25 @@ Menu.prototype.popupAtTarget = function(target, doc) { * The document that should own the context menu. */ Menu.prototype.popup = function(screenX, screenY, doc) { + // 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 + // opening a new one. + this.hide(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; + doc = DevToolsUtils.getTopWindow(win).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 - // opening a new one. - let popup = popupset.querySelector('menupopup[menu-api="true"]'); - if (popup) { - popup.hidePopup(); - } - popup = doc.createXULElement("menupopup"); + const popup = doc.createXULElement("menupopup"); popup.setAttribute("menu-api", "true"); popup.setAttribute("consumeoutsideclicks", "false"); popup.setAttribute("incontentshell", "false"); diff --git a/devtools/client/inspector/changes/test/browser_changes_copy_declaration.js b/devtools/client/inspector/changes/test/browser_changes_copy_declaration.js index 993c6744c91d..d59840a4e70e 100644 --- a/devtools/client/inspector/changes/test/browser_changes_copy_declaration.js +++ b/devtools/client/inspector/changes/test/browser_changes_copy_declaration.js @@ -45,6 +45,9 @@ add_task(async function() { () => checkClipboardData(EXPECTED_CLIPBOARD_REMOVED) ); + info("Hiding menu"); + menu.hide(document); + info( "Click the Copy Declaration context menu item for the added declaration" ); diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_entire_message.js b/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_entire_message.js index 132acff8f588..2ad03fb9580e 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_entire_message.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_entire_message.js @@ -203,10 +203,13 @@ async function copyMessageContent(hud, messageEl) { const copyMenuItem = menuPopup.querySelector("#console-menu-copy"); ok(copyMenuItem, "copy menu item is enabled"); - return waitForClipboardPromise( + const text = await waitForClipboardPromise( () => copyMenuItem.click(), data => data ); + + menuPopup.hidePopup(); + return text; } /** diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_message_with_framework_stacktrace.js b/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_message_with_framework_stacktrace.js index 64d86fa4ddd8..e7ce96ae6046 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_message_with_framework_stacktrace.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_message_with_framework_stacktrace.js @@ -122,8 +122,11 @@ async function copyMessageContent(hud, messageEl) { const copyMenuItem = menuPopup.querySelector("#console-menu-copy"); ok(copyMenuItem, "copy menu item is enabled"); - return waitForClipboardPromise( + const text = await waitForClipboardPromise( () => copyMenuItem.click(), data => data ); + + menuPopup.hidePopup(); + return text; } diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_export_console_output.js b/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_export_console_output.js index cc4831a83f56..5e2ba8955b06 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_export_console_output.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_export_console_output.js @@ -155,6 +155,8 @@ async function exportAllToFile(hud, message) { exportFile.click(); info("Exporting to file"); + menuPopup.hidePopup(); + // The file may not be ready yet. await waitFor(() => OS.File.exists(nsiFile.path)); const buffer = await OS.File.read(nsiFile.path); @@ -180,6 +182,8 @@ async function exportAllToClipboard(hud, message) { return data; } ); + + menuPopup.hidePopup(); return clipboardText; }