From 668686ae0504450a8c93501d1eb115f201eb982d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Qu=C3=A8ze?= Date: Fri, 3 Jul 2020 14:28:17 +0000 Subject: [PATCH] Bug 1621015 - Profiler popup button should be split into a start/stop button, and a down arrow, r=julienw. Differential Revision: https://phabricator.services.mozilla.com/D77680 --- .../themes/shared/toolbarbutton-icons.inc.css | 17 ++- .../performance-new/popup/menu-button.jsm.js | 95 +++++++++++- .../performance-new/test/browser/browser.ini | 2 + .../browser/browser_split-toolbar-button.js | 135 ++++++++++++++++++ .../performance-new/test/browser/head.js | 4 +- 5 files changed, 246 insertions(+), 7 deletions(-) create mode 100644 devtools/client/performance-new/test/browser/browser_split-toolbar-button.js diff --git a/browser/themes/shared/toolbarbutton-icons.inc.css b/browser/themes/shared/toolbarbutton-icons.inc.css index 9514eb1d5338..52219e5ee886 100644 --- a/browser/themes/shared/toolbarbutton-icons.inc.css +++ b/browser/themes/shared/toolbarbutton-icons.inc.css @@ -256,7 +256,7 @@ toolbar[brighttext] { list-style-image: url("chrome://browser/skin/developer.svg"); } -#profiler-button { +#profiler-button > .toolbarbutton-icon { list-style-image: url("chrome://devtools/skin/images/tool-profiler.svg"); } @@ -269,18 +269,27 @@ toolbar[brighttext] { fill-opacity: 1; } -#profiler-button.profiler-active image { +#profiler-button.profiler-active > image { background-color: #0060df33; } -#profiler-button.profiler-active:hover image { +#profiler-button.profiler-active:hover > image { background-color: #0060df22; } -#profiler-button.profiler-paused image { +#profiler-button.profiler-paused > image { opacity: 0.4; } +#profiler-button > .toolbarbutton-menu-dropmarker { + -moz-context-properties: fill, fill-opacity; +} + +.widget-overflow-list #profiler-button > .toolbarbutton-menu-dropmarker, +#customization-palette #profiler-button > .toolbarbutton-menu-dropmarker { + display: none; +} + #preferences-button { list-style-image: url("chrome://global/skin/icons/settings.svg"); } diff --git a/devtools/client/performance-new/popup/menu-button.jsm.js b/devtools/client/performance-new/popup/menu-button.jsm.js index b8c9027f6b98..1acf6f6d9cc3 100644 --- a/devtools/client/performance-new/popup/menu-button.jsm.js +++ b/devtools/client/performance-new/popup/menu-button.jsm.js @@ -26,6 +26,10 @@ const lazy = createLazyLoaders({ ChromeUtils.import( "resource://devtools/client/performance-new/popup/panel.jsm.js" ), + Background: () => + ChromeUtils.import( + "resource://devtools/client/performance-new/popup/background.jsm.js" + ), }); const WIDGET_ID = "profiler-button"; @@ -75,7 +79,13 @@ function openPopup(document) { if (!button) { throw new Error("Could not find the profiler button."); } - button.click(); + + // Sending a click event anywhere on the button could start the profiler + // instead of opening the popup. Sending a command event on a view widget + // will make CustomizableUI show the view. + const cmdEvent = document.createEvent("xulcommandevent"); + cmdEvent.initCommandEvent("command", true, true, button.ownerGlobal); + button.dispatchEvent(cmdEvent); } /** @@ -233,6 +243,17 @@ function initialize(toggleProfilerKeyShortcuts) { // is in the overflow menu. buttonElement.classList.add("subviewbutton-nav"); + // Our buttons has 2 targets: the icon which starts the profiler or + // captures the profile, and the dropmarker which opens the popup. + // To have this behavior, we need to enable showing the dropmarker... + buttonElement.setAttribute("wantdropmarker", "true"); + // ... and to implement our custom click and keyboard event handling + // to decide which of the 2 behaviors should be triggered. + buttonElement.addEventListener("click", item); + // This would be better as a keypress handler, but CustomizableUI's + // keypress handler runs before keypress handlers we could add here. + buttonElement.addEventListener("keydown", item); + function setButtonActive() { buttonElement.setAttribute( "tooltiptext", @@ -273,8 +294,80 @@ function initialize(toggleProfilerKeyShortcuts) { Services.obs.removeObserver(setButtonActive, "profiler-started"); Services.obs.removeObserver(setButtonInactive, "profiler-stopped"); Services.obs.removeObserver(setButtonPaused, "profiler-paused"); + buttonElement.removeEventListener("click", item); + buttonElement.removeEventListener("keydown", item); }); }, + + handleEvent: event => { + function startOrCapture() { + if (Services.profiler.IsPaused()) { + // A profile is already being captured, ignore this event. + return; + } + const { startProfiler, captureProfile } = lazy.Background(); + if (Services.profiler.IsActive()) { + captureProfile("aboutprofiling"); + } else { + startProfiler("aboutprofiling"); + } + } + + if (event.type == "click") { + // Only handle clicks on the left button. + if (event.button != 0) { + return; + } + + const button = event.target; + + // Ignore the click event while in the overflow menu + if (button.getAttribute("cui-anchorid") == "nav-bar-overflow-button") { + return; + } + + // If we are within the area of the icon, handle the event. + // Otherwise we are on the dropmarker and CustomizableUI will take + // care of opening the panel. + const win = button.ownerGlobal; + const iconBounds = win.windowUtils.getBoundsWithoutFlushing( + button.icon + ); + if ( + win.RTL_UI ? event.x >= iconBounds.left : event.x <= iconBounds.right + ) { + startOrCapture(); + event.stopPropagation(); + event.preventDefault(); + } + } else if (event.type == "keydown") { + if (event.key == " " || event.key == "Enter") { + startOrCapture(); + event.stopPropagation(); + event.preventDefault(); + return; + } + if (event.key == "ArrowDown") { + // Trigger a "command" event that CustomizableUI will handle. + const button = event.target; + const cmdEvent = button.ownerDocument.createEvent("xulcommandevent"); + cmdEvent.initCommandEvent( + "command", + true, + true, + button.ownerGlobal, + 0, + event.ctrlKey, + event.altKey, + event.shiftKey, + event.metaKey, + null, + event.mozInputSource + ); + event.currentTarget.dispatchEvent(cmdEvent); + } + } + }, }; CustomizableUI.createWidget(item); diff --git a/devtools/client/performance-new/test/browser/browser.ini b/devtools/client/performance-new/test/browser/browser.ini index d7b4ff46fa42..ae03357cf478 100644 --- a/devtools/client/performance-new/test/browser/browser.ini +++ b/devtools/client/performance-new/test/browser/browser.ini @@ -36,3 +36,5 @@ support-files = # Bug 941073. Also see: 1178420, 1115757, 1401049, 1269392 [browser_popup-private-browsing.js] skip-if = os == 'linux' + +[browser_split-toolbar-button.js] diff --git a/devtools/client/performance-new/test/browser/browser_split-toolbar-button.js b/devtools/client/performance-new/test/browser/browser_split-toolbar-button.js new file mode 100644 index 000000000000..3bbd7f2692f9 --- /dev/null +++ b/devtools/client/performance-new/test/browser/browser_split-toolbar-button.js @@ -0,0 +1,135 @@ +/* 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"; + +function isActive() { + return Services.profiler.IsActive(); +} + +/** + * Force focus to an element that isn't focusable. + * Toolbar buttons aren't focusable because if they were, clicking them would + * focus them, which is undesirable. Therefore, they're only made focusable + * when a user is navigating with the keyboard. This function forces focus as + * is done during toolbar keyboard navigation. + */ +function forceFocus(elem) { + elem.setAttribute("tabindex", "-1"); + elem.focus(); + elem.removeAttribute("tabindex"); +} + +async function waitForProfileAndCloseTab() { + await waitUntil( + () => !button.classList.contains("profiler-paused"), + "Waiting until the profiler is no longer paused" + ); + + await checkTabLoadedProfile({ + initialTitle: "Waiting on the profile", + successTitle: "Profile received", + errorTitle: "Error", + }); +} +var button; + +add_task(async function setup() { + info( + "Add the profiler button to the toolbar and ensure capturing a profile loads a local url." + ); + await setProfilerFrontendUrl( + "http://example.com/browser/devtools/client/performance-new/test/browser/fake-frontend.html" + ); + await makeSureProfilerPopupIsEnabled(); + button = document.getElementById("profiler-button"); +}); + +add_task(async function click_icon() { + info("Test that the profiler icon starts and captures a profile."); + + ok(!button.hasAttribute("open"), "should start with the panel closed"); + ok(!isActive(), "should start with the profiler inactive"); + + await EventUtils.synthesizeMouseAtCenter(button.icon, {}); + ok(isActive(), "should have started the profiler"); + + await EventUtils.synthesizeMouseAtCenter(button.icon, {}); + await waitForProfileAndCloseTab(); +}); + +add_task(async function click_dropmarker() { + info("Test that the profiler icon dropmarker opens the panel."); + + ok(!button.hasAttribute("open"), "should start with the panel closed"); + ok(!isActive(), "should start with the profiler inactive"); + + const popupShownPromise = waitForProfilerPopupEvent("popupshown"); + await EventUtils.synthesizeMouseAtCenter(button.dropmarker, {}); + await popupShownPromise; + + info("Ensure the panel is open and the profiler still inactive."); + ok(button.getAttribute("open") == "true", "panel should be open"); + ok(!isActive(), "profiler should still be inactive"); + await getElementByLabel(document, "Start Recording"); + + info("Press Escape to close the panel."); + const popupHiddenPromise = waitForProfilerPopupEvent("popuphidden"); + EventUtils.synthesizeKey("KEY_Escape"); + await popupHiddenPromise; + ok(!button.hasAttribute("open"), "panel should be closed"); +}); + +add_task(async function space_key() { + info("Test that the Space key starts and captures a profile."); + + ok(!button.hasAttribute("open"), "should start with the panel closed"); + ok(!isActive(), "should start with the profiler inactive"); + forceFocus(button); + + info("Press Space to start the profiler."); + EventUtils.synthesizeKey(" "); + ok(isActive(), "should have started the profiler"); + + info("Press Space again to capture the profile."); + EventUtils.synthesizeKey(" "); + await waitForProfileAndCloseTab(); +}); + +add_task(async function enter_key() { + info("Test that the Enter key starts and captures a profile."); + + ok(!button.hasAttribute("open"), "should start with the panel closed"); + ok(!isActive(), "should start with the profiler inactive"); + forceFocus(button); + + info("Pressing Enter should start the profiler."); + EventUtils.synthesizeKey("KEY_Enter"); + ok(isActive(), "should have started the profiler"); + + info("Pressing Enter again to capture the profile."); + EventUtils.synthesizeKey("KEY_Enter"); + await waitForProfileAndCloseTab(); +}); + +add_task(async function arrowDown_key() { + info("Test that ArrowDown key dropmarker opens the panel."); + + ok(!button.hasAttribute("open"), "should start with the panel closed"); + ok(!isActive(), "should start with the profiler inactive"); + forceFocus(button); + + info("Pressing the down arrow should open the panel."); + const popupShownPromise = waitForProfilerPopupEvent("popupshown"); + EventUtils.synthesizeKey("KEY_ArrowDown"); + await popupShownPromise; + ok(!isActive(), "profiler should still be inactive"); + ok(button.getAttribute("open") == "true", "panel should be open"); + + info("Press Escape to close the panel."); + const popupHiddenPromise = waitForProfilerPopupEvent("popuphidden"); + EventUtils.synthesizeKey("KEY_Escape"); + await popupHiddenPromise; + ok(!button.hasAttribute("open"), "panel should be closed"); +}); diff --git a/devtools/client/performance-new/test/browser/head.js b/devtools/client/performance-new/test/browser/head.js index d481e5c8b595..83a44270fd87 100644 --- a/devtools/client/performance-new/test/browser/head.js +++ b/devtools/client/performance-new/test/browser/head.js @@ -239,8 +239,8 @@ async function _toggleOpenProfilerPopup(window) { const popupShown = waitForProfilerPopupEvent("popupshown"); - info("> Trigger a click on the profiler menu button."); - profilerButton.click(); + info("> Trigger a click on the profiler button dropmarker."); + await EventUtils.synthesizeMouseAtCenter(profilerButton.dropmarker, {}); if (profilerButton.getAttribute("open") !== "true") { throw new Error(