commit 8e267f9570961ef588d8a98f9d48c63e4288cc3f
parent f865dca7a78c94e8855626bd69ea53802b17875d
Author: Sam Foster <sfoster@mozilla.com>
Date: Mon, 24 Nov 2025 17:18:26 +0000
Bug 1905212 - Allow less flickering in the performance tests, don't render the sidebar launcher when hidden. r=nsharpley
* Turns out we end up flickering *less* with sidebar.revamp
* Avoid loading images we don't show by skipping rendering sidebar-main until its container is visible
* Ensure the customizable UI test shows the launcher before making assertions on it
* Make the a11y test less racy and pass on purpose
Differential Revision: https://phabricator.services.mozilla.com/D273508
Diffstat:
7 files changed, 74 insertions(+), 27 deletions(-)
diff --git a/browser/base/content/test/performance/browser.toml b/browser/base/content/test/performance/browser.toml
@@ -16,8 +16,6 @@ prefs = [
# For perfomance tests do not enable the remote control cue, which gets set
# when Marionette is enabled, but users normally don't see.
"browser.chrome.disableRemoteControlCueForTests=true",
- # Need to keep the revamped sidebar off until Bug 1905212 is done
- "sidebar.revamp=false",
]
support-files = ["head.js"]
diff --git a/browser/base/content/test/performance/head.js b/browser/base/content/test/performance/head.js
@@ -1016,7 +1016,7 @@ async function checkLoadedScripts({
// window for some reason. See bug 1445161. This function allows to deal with
// that in a central place.
function isLikelyFocusChange(rects, frame) {
- if (rects.length > 3 && rects.every(r => r.y2 < 100)) {
+ if (rects.length >= 3 && rects.every(r => r.y2 < 100)) {
// There are at least 4 areas that changed near the top of the screen.
// Note that we need a bit more leeway than the titlebar height, because on
// OSX other toolbarbuttons in the navigation toolbar also get disabled
diff --git a/browser/components/customizableui/test/browser_sidebar_toggle.js b/browser/components/customizableui/test/browser_sidebar_toggle.js
@@ -13,6 +13,19 @@ registerCleanupFunction(async function () {
}
});
+async function ensureSidebarLauncherIsVisible() {
+ await TestUtils.waitForTick();
+ // Show the sidebar launcher if its hidden
+ if (SidebarController.sidebarContainer.hidden) {
+ document.getElementById("sidebar-button").doCommand();
+ }
+ await TestUtils.waitForTick();
+ Assert.ok(
+ BrowserTestUtils.isVisible(SidebarController.sidebarMain),
+ "Sidebar launcher is visible"
+ );
+}
+
var showSidebar = async function (win = window) {
let button = win.document.getElementById("sidebar-button");
let sidebarFocusedPromise = BrowserTestUtils.waitForEvent(
@@ -68,7 +81,7 @@ add_task(async function () {
await hideSidebar();
} else {
const sidebar = document.querySelector("sidebar-main");
- ok(sidebar, "Sidebar is shown.");
+ await ensureSidebarLauncherIsVisible();
for (const [index, toolButton] of sidebar.toolButtons.entries()) {
await SidebarController.toggle(toolButton.getAttribute("view"));
is(
diff --git a/browser/components/sidebar/SidebarState.sys.mjs b/browser/components/sidebar/SidebarState.sys.mjs
@@ -468,6 +468,7 @@ export class SidebarState {
this.#launcherEverVisible = true;
}
this.#launcherContainerEl.hidden = !visible;
+ this.#launcherEl.requestUpdate();
this.#updateTabbrowser(visible);
this.#sidebarBoxEl.style.paddingInlineStart =
this.panelOpen && !visible ? "var(--space-small)" : "unset";
diff --git a/browser/components/sidebar/sidebar-main.mjs b/browser/components/sidebar/sidebar-main.mjs
@@ -776,6 +776,11 @@ export default class SidebarMain extends MozLitElement {
);
}
+ shouldUpdate() {
+ const container = window.SidebarController.sidebarContainer;
+ return container && !container.hidden;
+ }
+
render() {
/* Add 1 to tools and extensions count for "Customize sidebar" option */
let enabledToolsAndExtensionsCount =
diff --git a/browser/components/sidebar/tests/browser/browser_a11y_sidebar.js b/browser/components/sidebar/tests/browser/browser_a11y_sidebar.js
@@ -55,6 +55,7 @@ function isActiveElement(el) {
add_task(async function test_keyboard_navigation() {
const sidebar = document.querySelector("sidebar-main");
+ let promisePanelFocused;
info("Waiting for tool buttons to be present");
await BrowserTestUtils.waitForMutationCondition(
sidebar,
@@ -85,55 +86,80 @@ add_task(async function test_keyboard_navigation() {
);
ok(isActiveElement(toolButtons[1]), "Second tool button is focused.");
- info("Press Arrow Up key.");
- EventUtils.synthesizeKey("KEY_ArrowUp", {});
- ok(isActiveElement(toolButtons[0]), "First tool button is focused.");
-
+ // 2nd tool is synced tabs which is a less-moving target than the chat panel
info("Press Enter key.");
+ promisePanelFocused = BrowserTestUtils.waitForEvent(window, "SidebarFocused");
EventUtils.synthesizeKey("KEY_Enter", {});
- await sidebar.updateComplete;
+ await promisePanelFocused;
+ await SidebarController.waitUntilStable();
+
ok(sidebar.open, "Sidebar is open.");
+ ok(
+ isActiveElement(SidebarController.browser),
+ "The focus moved to the sidebar panel browser"
+ );
+
+ info("selectedView is:" + sidebar.selectedView);
is(
sidebar.selectedView,
- toolButtons[0].getAttribute("view"),
- "Sidebar is showing the first tool."
+ toolButtons[1].getAttribute("view"),
+ "Sidebar is showing the 2nd tool."
);
// Moz-button is passing an "aria-pressed" attribute to the actual buttonEl:
is(
- toolButtons[0].buttonEl.getAttribute("aria-pressed"),
+ toolButtons[1].buttonEl.getAttribute("aria-pressed"),
"true",
"aria-pressed is true for the active tool button."
);
is(
- toolButtons[1].buttonEl.getAttribute("aria-pressed"),
+ toolButtons[0].buttonEl.getAttribute("aria-pressed"),
"false",
"aria-pressed is false for the inactive tool button."
);
- info("Press Enter key again.");
+ info("Press Shift+tab to move focus to the close button in the panel");
+ EventUtils.synthesizeKey("KEY_Tab", { shiftKey: true }, window);
+
+ info("Press Enter key to click the panel close button.");
+ let panelClosedPromise = BrowserTestUtils.waitForEvent(
+ SidebarController._box,
+ "sidebar-hide"
+ );
EventUtils.synthesizeKey("KEY_Enter", {});
- await sidebar.updateComplete;
- ok(!sidebar.open, "Sidebar is closed.");
+ await panelClosedPromise;
+ await SidebarController.waitUntilStable();
+ ok(
+ isActiveElement(gBrowser.selectedBrowser),
+ "The focus moved to the selected browser when the panel closed"
+ );
+
+ ok(!sidebar.open, "Sidebar panel is closed.");
is(
- toolButtons[0].buttonEl.getAttribute("aria-pressed"),
+ toolButtons[1].buttonEl.getAttribute("aria-pressed"),
"false",
"Tool is no longer active, aria-pressed becomes false."
);
- const customizeButton = sidebar.customizeButton;
+ // We seem to need to wait here before re-focusing the tool button
+ await waitForRepaint();
+ info("Re-focus the first tool button");
+ sidebar.buttonGroup.activeChild = toolButtons[0];
toolButtons[0].focus();
+ await SidebarController.waitUntilStable();
- info("Press Tab key.");
+ const customizeButton = sidebar.customizeButton;
+
+ info(
+ "Press Tab key to the next control group - which should be the customize button"
+ );
EventUtils.synthesizeKey("KEY_Tab", {});
ok(isActiveElement(customizeButton), "Customize button is focused.");
- info("Press Enter key again.");
- const promiseFocused = BrowserTestUtils.waitForEvent(
- window,
- "SidebarFocused"
- );
+
+ info("Press Enter key to open the customize panel");
+ promisePanelFocused = BrowserTestUtils.waitForEvent(window, "SidebarFocused");
EventUtils.synthesizeKey("KEY_Enter", {});
- await promiseFocused;
- await sidebar.updateComplete;
+ await promisePanelFocused;
+ await SidebarController.waitUntilStable();
ok(sidebar.open, "Sidebar is open.");
let customizeDocument = SidebarController.browser.contentDocument;
diff --git a/browser/components/sidebar/tests/unit/test_sidebar_state.js b/browser/components/sidebar/tests/unit/test_sidebar_state.js
@@ -18,6 +18,10 @@ const mockElement = {
style: { width: "200px" },
toggleAttribute: sinon.stub(),
};
+const mockLitElement = Object.assign(mockElement, {
+ requestUpdate: sinon.stub(),
+});
+
const mockGlobal = {
document: { getElementById: () => mockElement },
gBrowser: { tabContainer: mockElement },
@@ -27,7 +31,7 @@ const mockController = {
hide: sinon.stub(),
showInitially: sinon.stub(),
sidebarContainer: { ownerGlobal: mockGlobal },
- sidebarMain: mockElement,
+ sidebarMain: mockLitElement,
sidebarRevampEnabled: true,
sidebarRevampVisibility: "always-show",
sidebars: new Set(["viewBookmarksSidebar"]),
