fix(dashboard): audit conditions — selection retention, dead-code removal, sleep failure UX, group-name color

Follow-up fixes from the helpers-unify audit plus user-confirmed UX decisions.

Product:
- Batch sleep/dedup keep the ORIGINAL selection on chips that could not be
  resolved to a numeric tab id (they were not part of the operation), so a
  batch action never silently deselects untouched rows.
- closeTabOutDupes now closes through closeTabsSafely: the shared
  window-last-tab protection applies, so closing duplicate Tab Harbor pages
  never closes a window whose only tab would be removed.
- Removed three dead delegating wrappers (closeTabsByUrls/closeTabsExact/
  closeDuplicateTabs) that no longer had callers.
- Sleep failure UX: the per-chip sleep button is hidden on already-discarded
  rows (!tab.discarded), and group/global sleep filter out discarded tabs —
  an all-slept group is a silent no-op instead of a misleading
  'Failed to sleep' toast. Real discard failures still surface.
- Chrome-group card names use the PURE native group color again so they
  visually match the group chip in the browser tab strip (accepted contrast
  tradeoff on light palettes); hover/focus feedback is now an underline so
  keyboard focus stays visible without relying on color contrast.

Tests (406 -> 409):
- Behavioral tests for closeTabsByUrlsSafely (exact/file:// exact/hostname
  exact, unparseable skipped), closeDuplicatesByUrls (keepOne keeps the
  active copy) and the REAL ensureWindowsKeepLastTab implementation (was
  only mocked before).
- Hardened the shouldImportChromeGroupsIntoSessionState assertion (unconditional
  false anchored with doesNotMatch return true) and the sleepAllTabsButton
  i18n pair; updated the chip-discard button and color assertions.
This commit is contained in:
Leexunhuan743
2026-08-16 12:59:15 +08:00
parent afed14218e
commit 0b145f587e
4 changed files with 150 additions and 63 deletions
+107 -7
View File
@@ -445,8 +445,12 @@ test('user-created Chrome groups become first-class cards; their tabs leave doma
assert.match(runtimeJs, /isChromeGroup: true,/);
assert.match(runtimeJs, /chromeGroupId: group\.id,/);
assert.match(runtimeJs, /if \(chromeCards\.length > 0\) \{\s*domainGroups = \[\.\.\.chromeCards, \.\.\.applyGroupOrder\(\[\.\.\.manualGroups, \.\.\.automaticGroups\], groupOrderState\)\];/);
// Native groups are recognized live; the import pipeline is retired.
assert.match(runtimeJs, /function shouldImportChromeGroupsIntoSessionState\(\) \{[\s\S]{0,400}return false;/);
// Native groups are recognized live; the import pipeline is retired. The
// function must ALWAYS return false — a conditional false (e.g. flag-gated)
// would re-enable the retired pipeline, so the unconditional form is the
// behavioral anchor.
assert.match(runtimeJs, /function shouldImportChromeGroupsIntoSessionState\(\) \{[\s\S]{0,400}return false;\s*\}/);
assert.doesNotMatch(runtimeJs, /function shouldImportChromeGroupsIntoSessionState\(\) \{[\s\S]{0,400}return true/);
});
test('dragging into a Chrome group card joins the native group; dragging out ungroups', () => {
@@ -552,11 +556,12 @@ test('chrome group cards tint their name and row drag handles with the native co
// No separate top bar: the group color lives on the card name and the tab
// rows' drag handles (hover/focus keep the same tint).
assert.doesNotMatch(css, /\.mission-card\.chrome-group-card::before/);
// The NAME mixes the group color toward --ink (readable text, AA contrast);
// the drag handles keep the pure group color (graphical icons). Hover/focus
// step up to the 55% mix so keyboard focus keeps a visible feedback.
assert.match(css, /\.mission-card\.chrome-group-card \.mission-name \{\s*color:\s*color-mix\(in srgb, var\(--chrome-group-color, var\(--ink\)\) 35%, var\(--ink\)\);/);
assert.match(css, /\.mission-card\.chrome-group-card \.mission-rename-trigger:hover \.mission-name,[\s\S]{0,400}color:\s*color-mix\(in srgb, var\(--chrome-group-color, var\(--ink\)\) 55%, var\(--ink\)\);/);
// The NAME uses the pure native group color so it visually matches the
// group chip in the browser tab strip; hover/focus feedback is an underline
// (keyboard focus stays visible without relying on color contrast). The
// drag handles keep the pure group color too.
assert.match(css, /\.mission-card\.chrome-group-card \.mission-name \{\s*color:\s*var\(--chrome-group-color, var\(--ink\)\);/);
assert.match(css, /\.mission-card\.chrome-group-card \.mission-rename-trigger:hover \.mission-name,[\s\S]{0,400}text-decoration:\s*underline;/);
assert.match(css, /\.mission-card\.chrome-group-card \.chip-reorder-handle \{\s*color:\s*var\(--chrome-group-color,/);
assert.match(css, /\.mission-card\.chrome-group-card \.chip-reorder-handle:hover,[\s\S]{0,300}color:\s*color-mix\(in srgb, var\(--chrome-group-color, var\(--ink\)\) 55%, var\(--ink\)\);/);
// The tint is exposed on the handle as a CSS variable (not an inline color)
@@ -1050,3 +1055,98 @@ test('chrome-group query failure gates the toast and the snapshot fallback (C6/C
// left a user group from every card.
assert.match(runtimeJs, /const chromeGroupsQueryFailed = typeof getChromeGroupsLastError === 'function' && Boolean\(getChromeGroupsLastError\(\)\);[\s\S]{0,60}if \(chromeGroupsQueryFailed\) \{/);
});
// ---------------------------------------------------------------------------
// Audit follow-up (2026-08): behavioral tests for the unified close/dedup
// helpers — the URL-matching branches and the keep-one selection were only
// mirrored by regex assertions before.
// ---------------------------------------------------------------------------
test('closeTabsByUrlsSafely matches hostnames, exact URLs and file:// exactly', async () => {
const fn = new Function(`${extractFn(runtimeJs, 'closeTabsByUrlsSafely')}\nreturn closeTabsByUrlsSafely;`)();
let captured = null;
globalThis.runtimeGetCanonicalTabUrl = (url) => url;
globalThis.getTabCanonicalUrl = (tab) => String(tab?.url || '');
globalThis.queryTabsForDashboardWindow = async () => [
{ id: 1, windowId: 1, url: 'https://example.com/a' },
{ id: 2, windowId: 1, url: 'https://example.com/b' },
{ id: 3, windowId: 1, url: 'https://other.org/x' },
{ id: 4, windowId: 1, url: 'file:///tmp/note.md' },
{ id: 5, windowId: 1, url: 'https://sub.example.com/c' },
];
globalThis.closeTabsSafely = async (ids) => {
captured = ids;
return { closedCount: ids.length, closedTabIds: new Set(ids) };
};
// Hostname match is exact (no subdomain folding): example.com covers the
// two example.com tabs; the subdomain and other hosts stay.
await fn(['https://example.com/'], { exact: false, playSound: false });
assert.deepEqual(captured.sort(), [1, 2]);
// Exact match: only the identical URL.
captured = null;
await fn(['https://example.com/a'], { exact: true, playSound: false });
assert.deepEqual(captured, [1]);
// file:// URLs are always matched exactly, never by hostname.
captured = null;
await fn(['file:///tmp/note.md'], { exact: false, playSound: false });
assert.deepEqual(captured, [4]);
// Unparseable input URLs are skipped, nothing closes.
captured = null;
await fn(['not a url'], { exact: false, playSound: false });
assert.deepEqual(captured, []);
delete globalThis.runtimeGetCanonicalTabUrl;
delete globalThis.getTabCanonicalUrl;
delete globalThis.queryTabsForDashboardWindow;
delete globalThis.closeTabsSafely;
});
test('closeDuplicatesByUrls keeps the active copy when keepOne is set', async () => {
const fn = new Function(`${extractFn(runtimeJs, 'closeDuplicatesByUrls')}\nreturn closeDuplicatesByUrls;`)();
let captured = null;
globalThis.runtimeGetCanonicalTabUrl = (url) => url;
globalThis.getTabCanonicalUrl = (tab) => String(tab?.url || '');
globalThis.queryTabsForDashboardWindow = async () => [
{ id: 1, windowId: 1, url: 'https://example.com/x', active: false },
{ id: 2, windowId: 1, url: 'https://example.com/x', active: true },
{ id: 3, windowId: 1, url: 'https://example.com/y', active: false },
];
globalThis.closeTabsSafely = async (ids) => {
captured = ids;
return { closedCount: ids.length, closedTabIds: new Set(ids) };
};
// keepOne: the active copy (2) survives.
await fn(['https://example.com/x'], { keepOne: true, playSound: false });
assert.deepEqual(captured, [1]);
// keepOne=false: every copy closes.
captured = null;
await fn(['https://example.com/x'], { keepOne: false, playSound: false });
assert.deepEqual(captured.sort(), [1, 2]);
// No matches → nothing closes.
captured = null;
await fn(['https://nowhere.example'], { keepOne: true, playSound: false });
assert.deepEqual(captured, []);
delete globalThis.runtimeGetCanonicalTabUrl;
delete globalThis.getTabCanonicalUrl;
delete globalThis.queryTabsForDashboardWindow;
delete globalThis.closeTabsSafely;
});
test('ensureWindowsKeepLastTab never empties a window (real implementation)', () => {
const fn = new Function(`${extractFn(runtimeJs, 'ensureWindowsKeepLastTab')}\nreturn ensureWindowsKeepLastTab;`)();
const allTabs = [
{ id: 1, windowId: 7, active: true },
{ id: 2, windowId: 7, active: false },
{ id: 3, windowId: 8, active: false },
];
// Window 7 would lose both tabs: the active one (1) is kept. Window 8's
// only tab (3) is kept too.
assert.deepEqual(fn(allTabs, [1, 2, 3]), [2]);
// A window with tabs outside the close set is not protected: 1 is closable.
assert.deepEqual(fn(allTabs, [1]), [1]);
// A single-tab window whose only tab is in the close set is protected.
assert.deepEqual(fn([{ id: 9, windowId: 9, active: false }], [9]), []);
});
+22 -46
View File
@@ -3215,32 +3215,6 @@ function ensureWindowsKeepLastTab(allTabs, toCloseIds) {
return [...toCloseSet];
}
/**
* closeTabsByUrls(urls)
*
* Closes all open tabs whose hostname matches any of the given URLs.
* After closing, re-fetches the tab list to keep our state accurate.
*
* Special case: file:// URLs are matched exactly (they have no hostname).
*/
async function closeTabsByUrls(urls) {
const result = await closeTabsByUrlsSafely(urls, { exact: false, playSound: false });
await refreshTabData();
return result;
}
/**
* closeTabsExact(urls)
*
* Closes tabs by exact URL match (not hostname). Used for landing pages
* so closing "Gmail inbox" doesn't also close individual email threads.
*/
async function closeTabsExact(urls) {
const result = await closeTabsByUrlsSafely(urls, { exact: true, playSound: false });
await refreshTabData();
return result;
}
/**
* focusTab(url, tabId = null)
*
@@ -3364,19 +3338,6 @@ async function runDefaultSearch(query) {
await navigateCurrentTabToUrl(fallbackUrl);
}
/**
* closeDuplicateTabs(urls, keepOne)
*
* Closes duplicate tabs for the given list of URLs.
* keepOne=true keep one copy of each, close the rest.
* keepOne=false close all copies.
*/
async function closeDuplicateTabs(urls, keepOne = true) {
const result = await closeDuplicatesByUrls(urls, { keepOne, playSound: false });
await refreshTabData();
return result;
}
/**
* groupTabsWithStaleRetry(tabIds)
*
@@ -3451,6 +3412,9 @@ async function ungroupTabsWithStaleRetry(tabIds) {
* closeTabOutDupes()
*
* Closes all duplicate Tab Harbor new-tab pages except the current one.
* The close itself goes through closeTabsSafely so the shared
* window-last-tab protection applies: a window whose ONLY tab would be
* closed keeps it instead of closing the window.
*/
async function closeTabOutDupes() {
const newtabUrl = window.location.href;
@@ -3470,7 +3434,7 @@ async function closeTabOutDupes() {
tabOutTabs.find(t => t.active) ||
tabOutTabs[0];
const toClose = tabOutTabs.filter(t => t.id !== keep.id).map(t => t.id);
if (toClose.length > 0) await chrome.tabs.remove(toClose);
if (toClose.length > 0) await closeTabsSafely(toClose, { playSound: false });
await fetchOpenTabs();
}
@@ -3599,7 +3563,7 @@ function buildPageChipHtml(tab, group, urlCounts = {}, collapsed = false) {
<span class="chip-favicon chip-favicon-fallback"${faviconUrl ? ' style="display:none"' : ''}>${fallbackLabel}</span>
<span class="chip-text">${safeLabel}</span>
<div class="chip-actions">
${sleepControlEnabled && !tab.active && !isPlaceholder ? `<button class="chip-action chip-discard" data-action="discard-tab" data-tab-id="${tab.id}" aria-label="${runtimeT ? runtimeT('discardTab') : 'Sleep tab'}" data-tooltip="${runtimeT ? runtimeT('discardTab') : 'Sleep tab'}">
${sleepControlEnabled && !tab.active && !tab.discarded && !isPlaceholder ? `<button class="chip-action chip-discard" data-action="discard-tab" data-tab-id="${tab.id}" aria-label="${runtimeT ? runtimeT('discardTab') : 'Sleep tab'}" data-tooltip="${runtimeT ? runtimeT('discardTab') : 'Sleep tab'}">
${ICONS.moon}
</button>` : ''}
${!isPlaceholder ? `<button class="chip-action chip-session-save" data-action="save-single-tab-session" data-tab-id="${tab.id}" data-tab-url="${safeUrl}" data-tab-title="${safeTitle}" aria-label="${runtimeT ? runtimeT('saveTabSession') : 'Save tab session'}" data-tooltip="${runtimeT ? runtimeT('saveTabSession') : 'Save tab session'}">
@@ -5066,9 +5030,14 @@ document.addEventListener('click', async (e) => {
.map(chipId => chipTabIds.get(String(chipId)))
.filter(tabId => tabId != null);
const { discarded, failed, skippedActive, stale, resultsByTabId } = await sleepTabsByIds(tabIds, { skipActive: true });
// Chips that could not be resolved to a numeric tab id did not take
// part in the sleep, so they were not affected — keep them selected.
// Chips whose tab was processed stay selected when the tab survived
// (stale/failed/skipped-active included); rows that are really gone
// are pruned by refreshPageChipSelectionClasses after the render.
const keptChipIds = [...selectedPageChipIds].filter(chipId => {
const tabId = chipTabIds.get(String(chipId));
return tabId != null && resultsByTabId.has(tabId);
return tabId == null || resultsByTabId.has(tabId);
});
await finishBatchAction({ keptChipIds });
if (discarded > 0) {
@@ -5099,10 +5068,12 @@ document.addEventListener('click', async (e) => {
const { closedCount, closedTabIds } = await closeDuplicatesInSelection(tabIds);
// Keep the ORIGINAL chip sort ids that survive dedup, so selection is
// preserved on every card even when a chip id is not a plain tab id.
// refreshPageChipSelectionClasses prunes any id that no longer has a row.
// Chips without a resolvable numeric tab id did not take part in the
// dedup — keep them selected. refreshPageChipSelectionClasses prunes
// any id that no longer has a row.
const keptChipIds = [...selectedPageChipIds].filter(chipId => {
const tabId = chipTabIds.get(String(chipId));
return tabId != null && !closedTabIds.has(tabId);
return tabId == null || !closedTabIds.has(tabId);
});
await finishBatchAction({ keptChipIds });
if (closedCount > 0) {
@@ -5285,7 +5256,10 @@ document.addEventListener('click', async (e) => {
const group = domainGroups.find(g => getStableGroupId(g.domain) === domainId);
if (!group) return;
const tabIds = getOrderedUniqueTabsForGroup(group).filter(t => !t.active).map(t => t.id);
// Already-discarded rows have no sleep button and cannot be discarded
// again; filter them out so "nothing left to sleep" is a silent no-op
// instead of a misleading failure toast.
const tabIds = getOrderedUniqueTabsForGroup(group).filter(t => !t.active && !t.discarded).map(t => t.id);
if (!tabIds.length) return;
window.__suppressAutoRefreshUntil = Date.now() + 2000;
@@ -5310,7 +5284,9 @@ document.addEventListener('click', async (e) => {
// ---- Sleep all open tabs ----
if (action === 'sleep-all-open-tabs') {
const tabIds = getRealTabs().filter(t => !t.active).map(t => t.id);
// Same as the per-group path: already-discarded tabs are not sleepable
// again, so an all-slept window is a silent no-op, not a false failure.
const tabIds = getRealTabs().filter(t => !t.active && !t.discarded).map(t => t.id);
if (!tabIds.length) return;
window.__suppressAutoRefreshUntil = Date.now() + 2000;
+12 -9
View File
@@ -1759,20 +1759,23 @@ body.group-dragging .group-nav-button {
/* User-created Chrome group cards wear their native group color on the card
name and the tab rows' drag handles — a quiet accent that follows the
group's real Chrome color without adding a separate bar. The NAME mixes
the group color toward --ink so text stays readable (AA ≥ 4.5:1 for the
built-in palette colors on every light theme); the drag handles are
graphical icons and keep the pure group color. */
group's real Chrome color without adding a separate bar. The NAME uses
the pure native group color so it visually matches the group chip in the
browser tab strip (light palettes may show lower contrast for very light
group colors — accepted tradeoff for visual consistency). Hover/focus
feedback uses an underline instead of a color shift so keyboard focus
stays visible without relying on contrast. The drag handles keep the
pure group color too. */
.mission-card.chrome-group-card .mission-name {
color: color-mix(in srgb, var(--chrome-group-color, var(--ink)) 35%, var(--ink));
color: var(--chrome-group-color, var(--ink));
}
.mission-card.chrome-group-card .mission-rename-trigger:hover .mission-name,
.mission-card.chrome-group-card .mission-rename-trigger:focus-visible .mission-name {
/* Hover/focus must be visibly different from the resting mix — step up to
the 55% mix (same pattern as the drag handles) so keyboard focus keeps
a visible, readable feedback in every theme. */
color: color-mix(in srgb, var(--chrome-group-color, var(--ink)) 55%, var(--ink));
color: var(--chrome-group-color, var(--ink));
text-decoration: underline;
text-underline-offset: 3px;
text-decoration-thickness: 1.5px;
}
.mission-card.chrome-group-card .chip-reorder-handle {
+9 -1
View File
@@ -139,7 +139,7 @@ test('desk settings separates appearance and feature controls', () => {
});
test('manual sleep control places per-tab moon action first', () => {
assert.match(runtimeJs, /function buildPageChipHtml\(tab, group, urlCounts = \{\}, collapsed = false\) \{[\s\S]*<div class="chip-actions">\s*\$\{sleepControlEnabled && !tab\.active && !isPlaceholder \? `<button class="chip-action chip-discard"[\s\S]*<button class="chip-action chip-session-save"/);
assert.match(runtimeJs, /function buildPageChipHtml\(tab, group, urlCounts = \{\}, collapsed = false\) \{[\s\S]*<div class="chip-actions">\s*\$\{sleepControlEnabled && !tab\.active && !tab\.discarded && !isPlaceholder \? `<button class="chip-action chip-discard"[\s\S]*<button class="chip-action chip-session-save"/);
assert.match(runtimeJs, /const pageChips = orderedTabs\.map\(\(tab, index\) => buildPageChipHtml\(tab, group, urlCounts, index >= 8 && !isOverflowExpanded\)\)\.join\(''\)[\s\S]*buildOverflowChips\(extraCount\)/);
});
@@ -1395,6 +1395,10 @@ test('discard tab supports per-chip, group-level, and global sleep-all with gate
assert.match(runtimeJs, /data-action="sleep-all-open-tabs"/);
assert.match(runtimeJs, /page-chip--discarded/);
assert.match(runtimeJs, /if \(action === 'discard-tab'\)\s*\{[\s\S]*await fetchOpenTabs\(\);[\s\S]*await renderDashboard\(\);/);
// Group/global sleep skip already-discarded tabs, so "nothing left to
// sleep" is a silent no-op instead of a misleading failure toast.
assert.match(runtimeJs, /getOrderedUniqueTabsForGroup\(group\)\.filter\(t => !t\.active && !t\.discarded\)/);
assert.match(runtimeJs, /getRealTabs\(\)\.filter\(t => !t\.active && !t\.discarded\)/);
assert.match(runtimeJs, /if \(action === 'sleep-domain-tabs'\)\s*\{[\s\S]*await fetchOpenTabs\(\);[\s\S]*await renderDashboard\(\);/);
// The section-header sleep-all uses its own "all tabs" label; the per-group
// card keeps the "in group" label.
@@ -1403,6 +1407,10 @@ test('discard tab supports per-chip, group-level, and global sleep-all with gate
const i18nJs = fs.readFileSync(path.join(__dirname, 'i18n.js'), 'utf8');
assert.match(i18nJs, /sleepAllOpenTabsButton: 'Sleep all tabs'/);
assert.match(i18nJs, /sleepAllOpenTabsButton: '休眠全部标签页'/);
// Both keys must stay distinct with their own wording — collapsing them to
// a shared string would silently rename one of the two buttons.
assert.match(i18nJs, /sleepAllTabsButton: 'Sleep all tabs in group'/);
assert.match(i18nJs, /sleepAllTabsButton: '休眠组内全部标签页'/);
const css = fs.readFileSync(path.join(__dirname, 'style.css'), 'utf8');
assert.match(css, /\.chip-discard:hover\s*\{[\s\S]*color:\s*var\(--muted\);/);