corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1905944: Fix intermittents in browser_tab_preview.js r=sthompson,tabbrowser-reviewers, a=test-only

Browse Source 
This patch fixes a number of issues that clear up a few different
intermittents in browser_tab_preview.js.

1. In browser_tab_groups_tab_interactions_telemetry.js, a call to "close
   duplicate tabs" would automatically open the confirmation hint panel.
   If not manually closed, it seems to persist in a half-way open state
   where the `animating` attribute is always `true`. This is now fixed.

2. There is a test somewhere that seems to apply the
   `sidebar.verticalTabs` pref and doesn't turn it off. I would like to
   figure out which test is causing this, but as a last defence I have
   made sure this is always properly set within this test suite.

3. Some tests check that THP is disabled if another panel is open. These
   used to work by opening the `appMenu-popup` menu (the hamburger menu
   in the top right of the browser). The problem with this approach is
   that this panel (and basically every other panel) are very complex
   and are prone to change. This was causing an intermittent failure
   where attempting to close this panel threw an error in console that
   seemed to be specifically related to this panel. I fixed this by
   creating a fake minimal panel that is guaranteed to not change as
   application code changes. (I borrowed this technique from
   https://searchfox.org/mozilla-central/source/browser/base/content/test/popupNotifications/browser_popupNotification_queue.js).

4. There is a suite of tests that checks that THP is disabled in the
   event that a drag operation is performed. The problem is that
   dragging a tab by 100px sometimes detaches the tab and creates a new
   window, breaking the test run. Reducing the drag amount seems to fix
   this.

5. As a last line of defence, I have added a state reset function at the
   start of each test to ensure that all panels are closed and that the
   mouse is not hovering over the tab strip. (The latter was being done at
   the end of some tests already, but this makes sure it is consistently
   applied everywhere.)

Closes bug1898099, bug1905944, bug1933597.

Differential Revision: https://phabricator.services.mozilla.com/D258976
This commit is contained in:
Jeremy Swinarton
2025-07-28 21:03:21 +00:00
committed by rvandermeulen@mozilla.com
parent 2ce3f80962
commit 520c64e4d9
2 changed files with 106 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
browser/components/tabbrowser/test/browser/tabs/browser_tab_groups_tab_interactions_telemetry.js
View File

@@ -295,6 +295,13 @@ add_task(async function test_tabInteractionsCloseViaAnotherTabContext() {
);
await assertMetricFoundFor("close_tab_other", 4);
// Dismiss the confirmation panel that appears after closing duplicate tabs.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {});
await BrowserTestUtils.waitForCondition(
() => !window.ConfirmationHint._panel.getAttribute("animating"),
"Ensure the confirmation panel is closed"
);
window.gBrowser.removeAllTabsBut(initialTab);
await resetTelemetry();
});

194
browser/components/tabbrowser/test/browser/tabs/browser_tab_preview.js
View File

@@ -42,15 +42,53 @@ async function closePreviews(win = window) {
return previewHidden;
}
function getOpenPanels() {
return document.querySelectorAll(
"panel[panelopen=true],panel[animating=true],menupopup[open=true]"
);
}
async function resetState() {
for (let panel of getOpenPanels()) {
let hiddenEvent = BrowserTestUtils.waitForPopupEvent(panel, "hidden");
panel.hidePopup();
await hiddenEvent;
}
let openPanels = getOpenPanels();
Assert.ok(!openPanels.length, `sanity check: no panels open`);
// Ensure the mouse is not hovering over the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
}
function createFakePanel(win = window) {
let panel = win.document.createXULElement("panel");
// Necessary to get the panel open, animating, etc. elements to appear.
panel.setAttribute("type", "arrow");
win.document.documentElement.appendChild(panel);
return panel;
}
add_setup(async function () {
await SpecialPowers.pushPrefEnv({
set: [
["browser.tabs.hoverPreview.enabled", true],
["browser.tabs.hoverPreview.showThumbnails", false],
["browser.tabs.tooltipsShowPidAndActiveness", false],
["sidebar.revamp", false],
["sidebar.verticalTabs", false],
["ui.tooltip.delay_ms", 0],
],
});
await resetState();
registerCleanupFunction(async function () {
await resetState();
});
});
/**
@@ -88,10 +126,7 @@ add_task(async function hoverTests() {
BrowserTestUtils.removeTab(tab1);
BrowserTestUtils.removeTab(tab2);
// Move the mouse outside of the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
await resetState();
});
// Bug 1897475 - don't show tab previews in background windows
@@ -140,11 +175,8 @@ add_task(async function noTabPreviewInBackgroundWindowTests() {
BrowserTestUtils.removeTab(bgTab);
// Move the mouse outside of the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
sinon.restore();
await resetState();
});
/**
@@ -179,10 +211,7 @@ add_task(async function focusTests() {
BrowserTestUtils.removeTab(tab1);
BrowserTestUtils.removeTab(tab2);
// Move the mouse outside of the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
await resetState();
});
/**
@@ -208,13 +237,8 @@ add_task(async function pidAndActivenessHiddenByDefaultTests() {
);
await closePreviews();
BrowserTestUtils.removeTab(tab1);
// Move the mouse outside of the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
await resetState();
});
add_task(async function pidAndActivenessTests() {
@@ -271,11 +295,7 @@ add_task(async function pidAndActivenessTests() {
BrowserTestUtils.removeTab(tab1);
BrowserTestUtils.removeTab(tab2);
await SpecialPowers.popPrefEnv();
// Move the mouse outside of the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
await resetState();
});
/**
@@ -334,11 +354,7 @@ add_task(async function thumbnailTests() {
// Removing the tab should close the preview.
await previewHidden;
// Move the mouse outside of the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
await resetState();
});
/**
@@ -396,11 +412,7 @@ add_task(async function wireframeTests() {
// Removing the tab should close the preview.
await previewHidden;
// Move the mouse outside of the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
await resetState();
});
/**
@@ -447,6 +459,7 @@ add_task(async function delayTests() {
BrowserTestUtils.removeTab(tab1);
BrowserTestUtils.removeTab(tab2);
sinon.restore();
await resetState();
});
/**
@@ -474,7 +487,7 @@ add_task(async function dragTests() {
EventUtils.synthesizePlainDragAndDrop({
srcElement: tab1,
destElement: null,
stepX: 100,
stepX: 5,
stepY: 0,
});
@@ -489,12 +502,8 @@ add_task(async function dragTests() {
BrowserTestUtils.removeTab(tab1);
sinon.restore();
// Move the mouse outside of the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
await SpecialPowers.popPrefEnv();
await resetState();
});
/**
@@ -537,11 +546,7 @@ add_task(async function panelSuppressionOnContextMenuTests() {
contentAreaContextMenu.hidePopup();
BrowserTestUtils.removeTab(tab);
sinon.restore();
// Move the mouse outside of the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
await resetState();
});
/**
@@ -559,11 +564,13 @@ add_task(async function panelSuppressionOnPanelTests() {
const previewComponent = gBrowser.tabContainer.previewPanel;
sinon.spy(previewComponent, "activate");
// The `openPopup` API appears to not be working for this panel,
// but it can be triggered by firing a click event on the associated button.
const appMenuButton = document.getElementById("PanelUI-menu-button");
const appMenuPopup = document.getElementById("appMenu-popup");
appMenuButton.click();
let fakePanel = createFakePanel();
const popupShownEvent = BrowserTestUtils.waitForPopupEvent(
fakePanel,
"shown"
);
fakePanel.openPopup();
await popupShownEvent;
EventUtils.synthesizeMouseAtCenter(tab, { type: "mouseover" }, window);
@@ -584,12 +591,12 @@ add_task(async function panelSuppressionOnPanelTests() {
window
);
const popupHidingEvent = BrowserTestUtils.waitForEvent(
appMenuPopup,
"popuphiding"
const popupHiddenEvent = BrowserTestUtils.waitForPopupEvent(
fakePanel,
"hidden"
);
appMenuPopup.hidePopup();
await popupHidingEvent;
fakePanel.hidePopup();
await popupHiddenEvent;
// Attempt to open the tab preview immediately after the popup hiding event
await openPreview(tab);
@@ -597,11 +604,8 @@ add_task(async function panelSuppressionOnPanelTests() {
BrowserTestUtils.removeTab(tab);
sinon.restore();
// Move the mouse outside of the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
fakePanel.remove();
await resetState();
});
/**
@@ -614,11 +618,13 @@ add_task(async function panelSuppressionOnPanelLazyLoadTests() {
let fgWindow = await BrowserTestUtils.openNewBrowserWindow();
let fgTab = fgWindow.gBrowser.tabs[0];
// The `openPopup` API appears to not be working for this panel,
// but it can be triggered by firing a click event on the associated button.
const appMenuButton = fgWindow.document.getElementById("PanelUI-menu-button");
const appMenuPopup = fgWindow.document.getElementById("appMenu-popup");
appMenuButton.click();
let fakePanel = createFakePanel(fgWindow);
const popupShownEvent = BrowserTestUtils.waitForPopupEvent(
fakePanel,
"shown"
);
fakePanel.openPopup();
await popupShownEvent;
EventUtils.synthesizeMouseAtCenter(fgTab, { type: "mouseover" }, fgWindow);
@@ -626,8 +632,8 @@ add_task(async function panelSuppressionOnPanelLazyLoadTests() {
// Sometimes the tests run slower than the test browser -- it's not always possible
// to catch the panel in its opening state, so we have to check for both states.
return (
(appMenuPopup.getAttribute("animating") === "true" ||
appMenuPopup.getAttribute("panelopen") === "true") &&
(fakePanel.getAttribute("animating") === "true" ||
fakePanel.getAttribute("panelopen") === "true") &&
fgWindow.gBrowser.tabContainer.previewPanel !== null
);
});
@@ -635,7 +641,7 @@ add_task(async function panelSuppressionOnPanelLazyLoadTests() {
// We can't spy on the previewComponent and check for calls to `activate` like in other tests,
// since we can't guarantee that the spy will be set up before the call is made.
// Therefore the only realiable way to test that the popup isn't open is to reach in and check
// Therefore the only reliable way to test that the popup isn't open is to reach in and check
// that it is in a disabled state.
Assert.equal(previewComponent._isDisabled(), true, "");
@@ -651,20 +657,17 @@ add_task(async function panelSuppressionOnPanelLazyLoadTests() {
fgWindow
);
const popupHidingEvent = BrowserTestUtils.waitForEvent(
appMenuPopup,
"popuphiding"
const popupHiddenEvent = BrowserTestUtils.waitForPopupEvent(
fakePanel,
"hidden"
);
appMenuPopup.hidePopup();
await popupHidingEvent;
fakePanel.hidePopup();
await popupHiddenEvent;
BrowserTestUtils.removeTab(fgTab);
// Move the mouse outside of the tab strip.
fakePanel.remove();
await BrowserTestUtils.closeWindow(fgWindow);
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
await resetState();
});
/**
@@ -712,12 +715,13 @@ add_task(async function otherPanelOpenTests() {
Assert.ok(previewComponent._panelOpener._timer, "Timer is set");
// Open the popup before the timer finishes...
const appMenuButton = document.getElementById("PanelUI-menu-button");
const appMenuPopup = document.getElementById("appMenu-popup");
appMenuButton.click();
await BrowserTestUtils.waitForEvent(appMenuPopup, "popupshown");
let fakePanel = createFakePanel();
const popupShownEvent = BrowserTestUtils.waitForPopupEvent(
fakePanel,
"shown"
);
fakePanel.openPopup();
await popupShownEvent;
// Wait for timer to finish...
await BrowserTestUtils.waitForCondition(
@@ -738,16 +742,17 @@ add_task(async function otherPanelOpenTests() {
type: "mouseout",
});
const popupHidingEvent = BrowserTestUtils.waitForEvent(
appMenuPopup,
"popuphiding"
const popupHiddenEvent = BrowserTestUtils.waitForPopupEvent(
fakePanel,
"hidden"
);
appMenuPopup.hidePopup();
await popupHidingEvent;
fakePanel.hidePopup();
await popupHiddenEvent;
fakePanel.remove();
sinon.restore();
await SpecialPowers.popPrefEnv();
await resetState();
});
/**
@@ -782,6 +787,7 @@ add_task(async function urlBarInputTests() {
Assert.equal(previewElement.state, "closed", "Preview is closed");
BrowserTestUtils.removeTab(tab1);
await resetState();
});
/**
@@ -812,8 +818,8 @@ add_task(async function zeroDelayTests() {
await closePreviews();
BrowserTestUtils.removeTab(tab);
await SpecialPowers.popPrefEnv();
await resetState();
});
/**
@@ -852,11 +858,7 @@ add_task(async function wheelTests() {
while (gBrowser.tabs.length > 1) {
BrowserTestUtils.removeTab(gBrowser.tabs[0]);
}
// Move the mouse outside of the tab strip.
EventUtils.synthesizeMouseAtCenter(document.documentElement, {
type: "mouseover",
});
await resetState();
});
add_task(async function appearsAsTooltipToAccessibilityToolsTests() {
@@ -903,6 +905,7 @@ add_task(async function tabContentChangeTests() {
await closePreviews();
BrowserTestUtils.removeTab(tab);
await resetState();
});
/**
@@ -934,4 +937,5 @@ add_task(async function tabPreview_verticalTabsPositioning() {
await closePreviews();
BrowserTestUtils.removeTab(tab);
await resetState();
});