fix(images): keep the Filters label node across a listing reload
Every listing reload reports the active refinement count, and setActiveFilters rewrote the button's textContent even when the label was unchanged, replacing its text node. WebKit drops a click whose mousedown hit a node that was detached before mouseup, so a Safari user who types a search and clicks Filters inside the 250 ms debounce lost the click, and the webkit page workflows timed out in the source-change recovery tests (1 in ~30 runs of the pinned image; an instrumented probe showed pointerdown/mousedown, a DOM mutation, pointerup/mouseup and no click event). The label is rewritten only when it changes. The toolbar unit test counts textContent writes (1 for an unchanged count before, 0 after), and a workspace-images test pins the text node identity across a search reload: 0/6 before the fix on chromium and webkit, 6/6 after, with the recovery and Refine tests 36/36 in the pinned CI-like run.
This commit is contained in:
@@ -1512,6 +1512,25 @@ test('Refine opened before discovery settles lists the discovered sources and fo
|
||||
});
|
||||
|
||||
// #249 final review m1: as in the old field row, a Source offers only its own folders.
|
||||
test('a search reload keeps the Filters label node, so a click landing during the reload is not lost', async ({ page, request }) => {
|
||||
// WebKit drops a click whose mousedown hit a text node that was replaced before mouseup; the reload reports the
|
||||
// (unchanged) active count, so the label must keep its node.
|
||||
await expect(await request.post('/__test__/reset', { data: { extra_images: 80 } })).toBeOK();
|
||||
await page.goto(workspace);
|
||||
const button = filtersButton(page);
|
||||
await button.click();await expect(refineDialog(page).getByLabel('Folder', { exact: true }).locator('option[value="bulk"]')).toHaveCount(1);await page.keyboard.press('Escape');
|
||||
await refine(page, { Source: '0', Folder: 'bulk' });
|
||||
await expect(button).toHaveText('Filters · 2');
|
||||
const label = await button.evaluateHandle(node => node.firstChild);
|
||||
const reloaded = page.waitForResponse(response => {
|
||||
const url = new URL(response.url());return imagePages(url) && url.searchParams.get('q') === 'png';
|
||||
});
|
||||
await search(page).fill('png');
|
||||
await reloaded;
|
||||
await expect(button).toHaveText('Filters · 2');
|
||||
expect(await button.evaluate((node, text) => node.firstChild === text && text.isConnected, label)).toBe(true);
|
||||
});
|
||||
|
||||
test('Refine offers only the chosen source folders', async ({ page, request }) => {
|
||||
await expect(await request.post('/__test__/add-nested-image')).toBeOK();
|
||||
await page.goto(workspace);
|
||||
|
||||
@@ -765,6 +765,20 @@ test('the Filters button shows the active refinement count', async () => {
|
||||
assert.equal(filtersButton.textContent, 'Filters');
|
||||
for (const bad of [-1, 1.5, '1', null, Number.NaN]) assert.throws(() => toolbar.setActiveFilters(bad), TypeError);
|
||||
assert.equal(filtersButton.textContent, 'Filters');
|
||||
// An unchanged count leaves the label's text node alone: every listing reload reports the count, and WebKit
|
||||
// drops a click whose mousedown hit a text node that was replaced before mouseup.
|
||||
let writes = 0;
|
||||
const proto = Object.getPrototypeOf(filtersButton);
|
||||
const descriptor = Object.getOwnPropertyDescriptor(proto, 'textContent');
|
||||
Object.defineProperty(filtersButton, 'textContent', {
|
||||
get() { return descriptor.get.call(this); },
|
||||
set(text) { writes += 1; descriptor.set.call(this, text); },
|
||||
configurable: true,
|
||||
});
|
||||
toolbar.setActiveFilters(0);
|
||||
assert.equal(writes, 0);
|
||||
toolbar.setActiveFilters(3);
|
||||
assert.deepEqual([writes, filtersButton.textContent], [1, 'Filters · 3']);
|
||||
assert.throws(() => mountToolbar(new dom.Element('div'), { scope, categories: [], sorts: [['newest', 'Recently added']],
|
||||
activeFilters: -1, onChange() {}, onFilters() {}, onSaveView() {} }), TypeError);
|
||||
} finally {
|
||||
|
||||
@@ -107,7 +107,10 @@ export function mountToolbar(container, { scope, categories = [], sorts, value =
|
||||
category.value = [...category.options].some(node => node.value === selected) ? selected : '';
|
||||
}
|
||||
function setActiveFilters(count) {
|
||||
filters.textContent = filtersLabel(count);
|
||||
const next = filtersLabel(count);
|
||||
// Every listing reload reports the count; only a changed label rewrites the text node, because WebKit drops a
|
||||
// click whose mousedown hit a text node that was replaced before mouseup.
|
||||
if (filters.textContent !== next) filters.textContent = next;
|
||||
}
|
||||
setValue(value);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user