From f800d550cb583fcf5e61a0a29728be79e9116716 Mon Sep 17 00:00:00 2001 From: "mr. m" <91018726+mr-cheffy@users.noreply.github.com> Date: Thu, 17 Sep 2026 14:53:29 +0200 Subject: [PATCH] gh-15422: Fixed broken folder animations (gh-15436) --- src/zen/folders/ZenFolders.mjs | 154 ++++++++++------ src/zen/spaces/ZenGradientGenerator.mjs | 4 +- .../folders/browser_folder_issue_9981.js | 5 +- .../folders/browser_folder_reset_button.js | 5 +- .../tests/folders/browser_folder_subfolder.js | 172 ++++++++++++++++++ src/zen/tests/folders/head.js | 14 ++ .../mochitests/tabMediaIndicator/browser.toml | 4 +- 7 files changed, 293 insertions(+), 65 deletions(-) diff --git a/src/zen/folders/ZenFolders.mjs b/src/zen/folders/ZenFolders.mjs index 22119f054..c33f21114 100644 --- a/src/zen/folders/ZenFolders.mjs +++ b/src/zen/folders/ZenFolders.mjs @@ -404,13 +404,18 @@ class nsZenFolders extends nsZenDOMOperatedFeature { return; } - const collapsedRoot = group.rootMostCollapsedFolder; + let collapsedRoot = group.rootMostCollapsedFolder; if (!collapsedRoot) { return; } - collapsedRoot.setAttribute("has-active", "true"); - await this.animateSelect(collapsedRoot); + // If the folder got expanded instead, the tab may still be + // inside of another collapsed subfolder. + while (collapsedRoot) { + collapsedRoot.setAttribute("has-active", "true"); + const expanded = await this.animateSelect(collapsedRoot); + collapsedRoot = expanded ? group.rootMostCollapsedFolder : null; + } gBrowser.tabContainer._invalidateCachedTabs(); } @@ -1505,22 +1510,28 @@ class nsZenFolders extends nsZenDOMOperatedFeature { #createAnimation(items, targetState, opts, callback = () => {}) { items = Array.isArray(items) ? items : [items]; return items.map(item => - this.#animateItem(item, targetState, opts).then(callback) + this.#animateItem(item, targetState, opts).then( + finished => finished && callback() + ) ); } + #itemAnimations = new WeakMap(); + /** * Animates an element to the given target state. An array value is a * [from, to] pair, "auto" endpoints are resolved by measuring and an * empty string animates back to the element's natural value. The final - * values are left applied as inline styles. + * values are left applied as inline styles. A running animation on the + * same element is replaced, continuing from its current values. * * @param {Element} item - The element to animate. * @param {object} targetState - Property to value (or [from, to]) map. * @param {object} opts - The animation options. * @param {number} opts.duration - The duration in seconds. * @param {string} opts.ease - The easing name. - * @returns {Promise} Resolves when the animation has finished. + * @returns {Promise} Resolves when the animation has finished, + * false if it got replaced by another one before that. */ async #animateItem(item, targetState, { duration = 0.18, ease } = {}) { const computed = window.getComputedStyle(item); @@ -1539,13 +1550,18 @@ class nsZenFolders extends nsZenDOMOperatedFeature { const to = {}; const finalStyles = new Map(); for (const [prop, value] of Object.entries(targetState)) { - let [start, end] = Array.isArray(value) ? value : [undefined, value]; + const start = Array.isArray(value) ? value[0] : undefined; + from[prop] = + start === undefined || start === "auto" + ? measure(prop) + : toCssValue(prop, start); + } + // The start values are taken, the natural values need to be measured + // without the previous animation applying. + this.#itemAnimations.get(item)?.cancel(); + for (const [prop, value] of Object.entries(targetState)) { + let end = Array.isArray(value) ? value[1] : value; const cssProp = prop.replace(/[A-Z]/g, c => `-${c.toLowerCase()}`); - if (start === undefined || start === "auto") { - start = measure(prop); - } else { - start = toCssValue(prop, start); - } if (end === "" || end === "auto") { // Resolve the natural value by clearing any inline override. item.style.removeProperty(cssProp); @@ -1555,18 +1571,18 @@ class nsZenFolders extends nsZenDOMOperatedFeature { end = toCssValue(prop, end); finalStyles.set(cssProp, end); } - from[prop] = start; to[prop] = end; } const animation = item.animate([from, to], { duration: duration * 1000, easing: ease === "easeInOut" ? "ease-in-out" : "ease", }); + this.#itemAnimations.set(item, animation); try { await animation.finished; } catch (e) { - // The animation was cancelled, leave the element as-is. - return; + // The animation was replaced, leave the element to the new one. + return false; } for (const [cssProp, value] of finalStyles) { if (value === null) { @@ -1575,6 +1591,7 @@ class nsZenFolders extends nsZenDOMOperatedFeature { item.style.setProperty(cssProp, value); } } + return true; } #calculateHeightShift(tabsContainer, selectedTabs) { @@ -1757,49 +1774,53 @@ class nsZenFolders extends nsZenDOMOperatedFeature { } } - const afterMarginTop = () => { - tabsContainer.style.overflowY = ""; - if (group.hasAttribute("has-active")) { - const activeTabs = group.activeTabs; - const folders = new Map(); - group.removeAttribute("has-active"); - for (let tab of activeTabs) { - const tabGroup = tab?.group?.hasAttribute("split-view-group") - ? tab?.group?.group - : tab?.group; - if (!folders.has(tabGroup?.id)) { - folders.set(tabGroup?.id, tabGroup?.activeGroups?.at(-1)); - } - let activeGroup = folders.get(tabGroup?.id); - if (activeGroup) { - this.setFolderIndentation( - [tab], - activeGroup, - /* for collapse = */ true - ); + // Update the active state and indentation right away instead of when + // the animation ends, it would otherwise be lost if the folder gets + // collapsed again before that. + if (group.hasAttribute("has-active")) { + const activeTabs = group.activeTabs; + group.removeAttribute("has-active"); + for (const tab of activeTabs) { + const tabGroup = tab?.group?.hasAttribute("split-view-group") + ? tab?.group?.group + : tab?.group; + const activeGroup = tabGroup?.activeGroups?.at(-1); + if (activeGroup) { + this.setFolderIndentation( + [tab], + activeGroup, + /* for collapse = */ true + ); + } else { + // Since the folder is now expanded, we should remove active attribute + // to the tab that was previously visible + tab.removeAttribute("folder-active"); + if (tab.group?.hasAttribute("split-view-group")) { + tab.group.style.removeProperty("--zen-folder-indent"); } else { - // Since the folder is now expanded, we should remove active attribute - // to the tab that was previously visible - tab.removeAttribute("folder-active"); - if (tab.group?.hasAttribute("split-view-group")) { - tab.group.style.removeProperty("--zen-folder-indent"); - } else { - tab.style.removeProperty("--zen-folder-indent"); - } + tab.style.removeProperty("--zen-folder-indent"); } } - folders.clear(); } - // Folder has been expanded and has no active tabs - group.activeTabs = []; + } + // Folder has been expanded and has no active tabs + group.activeTabs = []; + + const afterMarginTop = () => { + tabsContainer.style.overflowY = ""; }; let duration = this.#folderAnimationDuration; + // Items of collapsed subfolders must stay hidden, don't animate them twice. + const itemsToReveal = itemsToShow.filter( + item => !itemsToHide.includes(item) + ); + animations.push( ...this.#createAnimation( - itemsToShow, - { opacity: "", height: "" }, + itemsToReveal, + { opacity: "", height: "", minHeight: "" }, { duration, ease: "easeInOut" } ), ...this.#createAnimation( @@ -1821,10 +1842,6 @@ class nsZenFolders extends nsZenDOMOperatedFeature { this.#animationCount += 1; await Promise.all(animations); this.#animationCount -= 1; - - // Cleanup - this.styleCleanup(itemsToShow); - this.styleCleanup(itemsToHide); } async animateUnloadAll(group) { @@ -1969,9 +1986,16 @@ class nsZenFolders extends nsZenDOMOperatedFeature { gBrowser.tabContainer._invalidateCachedTabs(); } + /** + * Shows the selected tabs of a collapsed folder. + * + * @param {MozTabbrowserTabGroup} group - The collapsed folder. + * @returns {Promise} True if the folder got expanded instead, + * since all of its tabs are active. + */ async animateSelect(group) { if (!group?.isZenFolder) { - return; + return false; } this.cancelPopupTimer(); @@ -1989,7 +2013,24 @@ class nsZenFolders extends nsZenDOMOperatedFeature { if (group.collapsed && selectedTabs.length) { const active = new Set([...(group.activeTabs ?? []), ...selectedTabs]); - const tabs = group.tabs.filter(tab => !tab.hasAttribute("zen-empty-tab")); + // Tabs inside of collapsed subfolders would stay hidden if the folder + // gets expanded, so they don't need to be active. + const isInsideCollapsedSubfolder = tab => { + let folder = tab.group?.hasAttribute("split-view-group") + ? tab.group.group + : tab.group; + while (folder && folder !== group) { + if (folder.collapsed) { + return true; + } + folder = folder.group; + } + return false; + }; + const tabs = group.tabs.filter( + tab => + !tab.hasAttribute("zen-empty-tab") && !isInsideCollapsedSubfolder(tab) + ); if ( tabs.length && tabs.every( @@ -2001,7 +2042,7 @@ class nsZenFolders extends nsZenDOMOperatedFeature { ) ) { group.collapsed = false; - return; + return true; } } @@ -2160,11 +2201,12 @@ class nsZenFolders extends nsZenDOMOperatedFeature { await Promise.all(animations); this.#animationCount -= 1; if (this.#animationCount) { - return; + return false; } // Cleanup this.styleCleanup(selectedTabs); + return false; } animateGroupMove(group, expand = false) { diff --git a/src/zen/spaces/ZenGradientGenerator.mjs b/src/zen/spaces/ZenGradientGenerator.mjs index ce42663b2..7ca02341a 100644 --- a/src/zen/spaces/ZenGradientGenerator.mjs +++ b/src/zen/spaces/ZenGradientGenerator.mjs @@ -1367,8 +1367,8 @@ export class nsZenThemePicker extends nsZenMultiWindowFeature { if (themedColors.length === 2) { if (!forToolbar) { return [ - `linear-gradient(${rotation}deg, ${this.#getSingleRGBColor(themedColors[1], forToolbar)} 20%, transparent 100%)`, - `linear-gradient(${rotation + 180}deg, ${this.#getSingleRGBColor(themedColors[0], forToolbar)} 20%, transparent 100%)`, + `linear-gradient(${rotation}deg, ${this.#getSingleRGBColor(themedColors[1], forToolbar)} 30%, transparent 120%)`, + `linear-gradient(${rotation + 180}deg, ${this.#getSingleRGBColor(themedColors[0], forToolbar)} 30%, transparent 120%)`, ] .reverse() .join(", "); diff --git a/src/zen/tests/folders/browser_folder_issue_9981.js b/src/zen/tests/folders/browser_folder_issue_9981.js index 13ec5da87..6b6b80614 100644 --- a/src/zen/tests/folders/browser_folder_issue_9981.js +++ b/src/zen/tests/folders/browser_folder_issue_9981.js @@ -44,10 +44,7 @@ add_task(async function test_Issue_9981() { const tab2ResetButton = tab2.querySelector(".tab-reset-button"); tab2ResetButton.style.display = "flex"; - await TestUtils.waitForCondition( - () => tab2ResetButton.getBoundingClientRect().width != 0 - ); - EventUtils.synthesizeMouseAtCenter(tab2ResetButton, {}); + await clickWhenReady(tab2ResetButton); await TestUtils.waitForCondition( () => !tab2.hasAttribute("folder-active"), diff --git a/src/zen/tests/folders/browser_folder_reset_button.js b/src/zen/tests/folders/browser_folder_reset_button.js index 5e45fce9a..2df1526ae 100644 --- a/src/zen/tests/folders/browser_folder_reset_button.js +++ b/src/zen/tests/folders/browser_folder_reset_button.js @@ -42,7 +42,10 @@ add_task(async function test_Issue_() { "Tab 2 should be in the active folder" ); - EventUtils.synthesizeMouseAtCenter(folder.resetButton, {}); + EventUtils.synthesizeMouseAtCenter(folder.labelElement, { + type: "mousemove", + }); + await clickWhenReady(folder.resetButton); await new Promise(resolve => /* eslint-disable mozilla/no-arbitrary-setTimeout */ diff --git a/src/zen/tests/folders/browser_folder_subfolder.js b/src/zen/tests/folders/browser_folder_subfolder.js index 330773cf2..86508adbc 100644 --- a/src/zen/tests/folders/browser_folder_subfolder.js +++ b/src/zen/tests/folders/browser_folder_subfolder.js @@ -26,3 +26,175 @@ add_task(async function test_Create_Folder() { await removeFolder(subfolder); await removeFolder(parent); }); + +add_task(async function test_Collapsed_Subfolder_Stays_Collapsed() { + const originalTab = gBrowser.selectedTab; + const activeTab = BrowserTestUtils.addTab(gBrowser, "data:text/html,tab1"); + const hiddenTab = BrowserTestUtils.addTab(gBrowser, "data:text/html,tab2"); + const parentTab = BrowserTestUtils.addTab(gBrowser, "data:text/html,tab3"); + const subfolder = await gZenFolders.createFolder([activeTab, hiddenTab], { + renameFolder: false, + label: "subfolder", + }); + const parent = await gZenFolders.createFolder([parentTab], { + renameFolder: false, + label: "parent", + }); + parentTab.after(subfolder); + gBrowser.selectedTab = activeTab; + await new Promise(resolve => setTimeout(resolve, 0)); + + const height = tab => tab.getBoundingClientRect().height; + + subfolder.collapsed = true; + await TestUtils.waitForCondition( + () => !height(hiddenTab), + "Unselected tab is hidden when the subfolder collapses" + ); + + parent.collapsed = true; + await TestUtils.waitForCondition( + () => !height(parentTab), + "Parent tab is hidden when the parent folder collapses" + ); + + parent.collapsed = false; + await TestUtils.waitForCondition( + () => height(parentTab) && !parent.hasAttribute("has-active"), + "Parent folder is expanded again" + ); + await TestUtils.waitForTick(); + + ok(subfolder.collapsed, "Subfolder is still collapsed"); + ok(height(activeTab), "Active tab of the subfolder is still visible"); + Assert.equal( + height(hiddenTab), + 0, + "Unselected tab of the collapsed subfolder stays hidden" + ); + + gBrowser.selectedTab = originalTab; + await removeFolder(subfolder); + await removeFolder(parent); +}); + +add_task(async function test_Spam_Toggle_Parent_Folder() { + const originalTab = gBrowser.selectedTab; + const activeTab = BrowserTestUtils.addTab(gBrowser, "data:text/html,tab1"); + const hiddenTab = BrowserTestUtils.addTab(gBrowser, "data:text/html,tab2"); + const parentTab = BrowserTestUtils.addTab(gBrowser, "data:text/html,tab3"); + const subfolder = await gZenFolders.createFolder([activeTab, hiddenTab], { + renameFolder: false, + label: "subfolder", + }); + const parent = await gZenFolders.createFolder([parentTab], { + renameFolder: false, + label: "parent", + }); + parentTab.after(subfolder); + gBrowser.selectedTab = activeTab; + // createFolder sets the initial collapsed state on a timeout. + await new Promise(resolve => setTimeout(resolve, 0)); + + const height = tab => tab.getBoundingClientRect().height; + const indent = tab => tab.style.getPropertyValue("--zen-folder-indent"); + + subfolder.collapsed = true; + await TestUtils.waitForCondition( + () => !height(hiddenTab), + "Unselected tab is hidden when the subfolder collapses" + ); + + for (let i = 0; i < 3; i++) { + parent.collapsed = !parent.collapsed; + await new Promise(resolve => requestAnimationFrame(resolve)); + } + await TestUtils.waitForCondition( + () => !height(parentTab), + "Parent tab is hidden when the parent folder ends up collapsed" + ); + + ok(parent.collapsed, "Parent folder is collapsed"); + ok(parent.hasAttribute("has-active"), "Parent folder is still active"); + Assert.deepEqual(parent.activeTabs, [activeTab], "Active tab is kept"); + ok(height(activeTab), "Active tab is still visible"); + Assert.equal(indent(activeTab), "0px", "Active tab lost its indentation"); + Assert.equal(height(hiddenTab), 0, "Unselected tab stays hidden"); + + for (let i = 0; i < 3; i++) { + parent.collapsed = !parent.collapsed; + await new Promise(resolve => requestAnimationFrame(resolve)); + } + await TestUtils.waitForCondition( + () => height(parentTab), + "Parent tab is visible when the parent folder ends up expanded" + ); + + ok(!parent.hasAttribute("has-active"), "Parent folder is not active"); + ok(subfolder.hasAttribute("has-active"), "Subfolder is still active"); + Assert.equal(indent(activeTab), "14px", "Active tab is indented again"); + Assert.equal(height(hiddenTab), 0, "Unselected tab stays hidden"); + + gBrowser.selectedTab = originalTab; + await removeFolder(subfolder); + await removeFolder(parent); +}); + +add_task(async function test_Select_All_Tabs_Expands_Subfolder() { + const originalTab = gBrowser.selectedTab; + const tab1 = BrowserTestUtils.addTab(gBrowser, "data:text/html,tab1"); + const tab2 = BrowserTestUtils.addTab(gBrowser, "data:text/html,tab2"); + const tab3 = BrowserTestUtils.addTab(gBrowser, "data:text/html,tab3"); + const subfolder = await gZenFolders.createFolder([tab2, tab3], { + renameFolder: false, + label: "subfolder", + }); + const parent = await gZenFolders.createFolder([tab1], { + renameFolder: false, + label: "parent", + }); + tab1.after(subfolder); + await new Promise(resolve => setTimeout(resolve, 0)); + + const height = tab => tab.getBoundingClientRect().height; + + subfolder.collapsed = true; + gBrowser.selectedTab = tab1; + parent.collapsed = true; + await TestUtils.waitForCondition( + () => !height(tab2) && !height(tab3), + "Subfolder tabs are hidden when the parent folder collapses" + ); + + gBrowser.selectedTab = tab3; + await TestUtils.waitForCondition( + () => height(tab3) && !parent.collapsed, + "Parent folder is expanded when tab 3 is selected" + ); + await TestUtils.waitForCondition( + () => !height(tab2), + "Tab 2 stays hidden inside of the collapsed subfolder" + ); + ok(!parent.hasAttribute("has-active"), "Parent folder is not active"); + ok(subfolder.collapsed, "Subfolder is still collapsed"); + ok(subfolder.hasAttribute("has-active"), "Subfolder is active"); + Assert.deepEqual(subfolder.activeTabs, [tab3], "Tab 3 is the active tab"); + ok(!tab1.hasAttribute("folder-active"), "Tab 1 is not folder-active"); + + gBrowser.selectedTab = tab2; + await TestUtils.waitForCondition( + () => height(tab2), + "Tab 2 is visible when selected" + ); + ok(!parent.collapsed, "Parent folder is expanded"); + ok(!subfolder.collapsed, "Subfolder is expanded"); + ok(!parent.hasAttribute("has-active"), "Parent folder is not active"); + ok(!subfolder.hasAttribute("has-active"), "Subfolder is not active"); + for (const tab of [tab1, tab2, tab3]) { + ok(!tab.hasAttribute("folder-active"), "Tab is not folder-active"); + } + + gBrowser.selectedTab = originalTab; + await removeFolder(subfolder); + await removeFolder(parent); +}); diff --git a/src/zen/tests/folders/head.js b/src/zen/tests/folders/head.js index e3d14953d..5e4a19e85 100644 --- a/src/zen/tests/folders/head.js +++ b/src/zen/tests/folders/head.js @@ -32,3 +32,17 @@ async function addTabTo( await BrowserTestUtils.browserLoaded(browser); return tab; } + +async function clickWhenReady(button) { + await TestUtils.waitForCondition(() => { + const rect = button.getBoundingClientRect(); + return ( + rect.width && + document.elementFromPoint( + rect.x + rect.width / 2, + rect.y + rect.height / 2 + ) === button + ); + }, "Button should be clickable"); + EventUtils.synthesizeMouseAtCenter(button, {}); +} diff --git a/src/zen/tests/mochitests/tabMediaIndicator/browser.toml b/src/zen/tests/mochitests/tabMediaIndicator/browser.toml index 41ec23ef4..4b221ad11 100644 --- a/src/zen/tests/mochitests/tabMediaIndicator/browser.toml +++ b/src/zen/tests/mochitests/tabMediaIndicator/browser.toml @@ -38,12 +38,12 @@ https_first_disabled = true ["browser_mute.js"] +["browser_mute2.js"] + ["browser_mute_persist_navigation.js"] ["browser_mute_restore_closed_audible_tab.js"] -["browser_mute2.js"] - ["browser_mute_webAudio.js"] ["browser_sound_indicator_silent_video.js"]