From 6864a46430d7718d02892d8f393a730d3710fee7 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Thu, 22 Oct 2026 09:41:01 -0700 Subject: [PATCH] fix(history): keep a marker tap when the timeline re-renders mid-press The recorded history timeline rewrote every marker's textContent on each render, so a scroll-triggered re-render replaced the text node under the pointer. When that frame landed between the press and the release of a tap, Chromium touch dropped the click and the Recorded run dialog never opened. Text is now written only when it changes. Adds a deterministic regression test that re-renders the timeline between mouse down and mouse up. --- tests/browser/workspace-history.spec.mjs | 13 +++++++++++++ web/js/workspace/history.mjs | 14 ++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/browser/workspace-history.spec.mjs b/tests/browser/workspace-history.spec.mjs index 405718c..ac7c336 100644 --- a/tests/browser/workspace-history.spec.mjs +++ b/tests/browser/workspace-history.spec.mjs @@ -317,6 +317,19 @@ test('marker labels disclose exact observed dequeue and outcome including times } }); +// A timeline scroll re-renders the markers on the next frame. On a slow runner that frame lands between the press and +// the release of a marker tap (Playwright's own scroll-into-view scrolls the timeline first); the re-render must leave +// the pressed marker's nodes alone, or Chromium touch drops the click. The scroll event is dispatched so the marker +// stays under the pointer, and the two frames are the re-render itself. +test('a timeline re-render between press and release keeps the marker tap',async({page})=>{ + const f=page.fixture;await open(page,f.detail_id); + const target=marker(page,f.runs.shared,'z');await expect(target).toHaveAttribute('aria-disabled','false');await target.scrollIntoViewIfNeeded(); + const box=await target.boundingBox();await page.mouse.move(box.x+box.width/2,box.y+box.height/2);await page.mouse.down(); + await recorded(page).locator('.history-timeline').evaluate(timeline=>{timeline.dispatchEvent(new Event('scroll'));return new Promise(resolve=>requestAnimationFrame(()=>requestAnimationFrame(resolve)));}); + await page.mouse.up(); + await expect(dialog(page).locator('[data-receipt]')).toHaveCount(5); +}); + test('same-epoch refresh retains recorded context mode and exact receipt after fresh candidate reads',async({page})=>{ const f=page.fixture;await open(page,f.detail_id);await marker(page,f.runs.shared,'z').click();await expect(dialog(page).locator('[data-receipt]')).toHaveCount(5); await dialog(page).getByRole('button',{name:'Effective',exact:true}).click(); diff --git a/web/js/workspace/history.mjs b/web/js/workspace/history.mjs index d7e65c0..992b895 100644 --- a/web/js/workspace/history.mjs +++ b/web/js/workspace/history.mjs @@ -22,6 +22,12 @@ function observedLabel(value) { return `Observed dequeue ${value} ms (${Number.isSafeInteger(time) && time <= 8640000000000000 ? new Date(time).toISOString() : 'outside display range'})`; } +// Writing textContent replaces the text node even when the text is unchanged. A marker re-rendered between the press and +// the release of a tap would lose the node under the pointer, and Chromium touch then drops the click. +function setText(node, text) { + if (node.textContent !== text) node.textContent = text; +} + // New nodes use ordinary insertion; connected moves preserve browser state where supported. function place(parent, node, before) { if (node === before || (node.parentNode === parent && node.nextSibling === before)) return; @@ -115,7 +121,7 @@ export function mountRecordedHistory(container, {api, positiveId, parentScope, s const {node, buttons} = column; node.style.insetInlineStart = `${38 + index * 64}px`;node.value = index + 1; node.setAttribute('aria-posinset', String(index + 1));node.setAttribute('aria-setsize', String(items.length)); - column.position.textContent = String(index + 1); + setText(column.position, String(index + 1)); const anchors = new Map();for (const anchor of run.matching_anchors) if (!anchors.has(anchor.context_id)) anchors.set(anchor.context_id, anchor); for (const anchor of anchors.values()) { let button = buttons.get(anchor.context_id); @@ -126,7 +132,7 @@ export function mountRecordedHistory(container, {api, positiveId, parentScope, s const context = contexts.get(anchor.context_id), first = context.first === index; button.classList.toggle('history-context-anchor', first); button.style.insetBlockStart = `${80 + context.row * 57}px`; - button.textContent = first ? '−' : ''; + setText(button, first ? '−' : ''); const label = `Recorded run ${index + 1} of ${items.length} loaded · ${observedLabel(run.observed_dequeue_at)} · Outcome ${run.outcome} · ${preview(context.summary)} · context ${anchor.context_id.slice(0, 8)} · anchor ${anchor.anchor_id}`; button.setAttribute('aria-label', label);button.title = label; button.setAttribute('aria-pressed', String(state.selection.run_id === run.run_id && state.selection.context_id === anchor.context_id)); @@ -157,7 +163,7 @@ export function mountRecordedHistory(container, {api, positiveId, parentScope, s let branch = branchNodes.get(id); if (!branch) {branch = el('div', undefined, 'history-context-branch');branch.dataset.contextBranch = id;branch.append(el('span', '', 'history-context-label'));branchNodes.set(id, branch);} branch.style.insetBlockStart = `${80 + context.row * 57}px`;branch.style.insetInlineStart = `${38 + context.first * 64}px`; - branch.firstChild.textContent = `${preview(context.summary)} · ${id.slice(0, 8)} · ${distinction(context.summary)}`; + setText(branch.firstChild, `${preview(context.summary)} · ${id.slice(0, 8)} · ${distinction(context.summary)}`); place(branches, branch, before);before = branch.nextSibling; } if (anchor) { @@ -176,7 +182,7 @@ export function mountRecordedHistory(container, {api, positiveId, parentScope, s selected.hidden = !outside;selected.disabled = next.validating || window.loading || !!next.refreshRequired; earlier.hidden = !window.has_more;earlier.disabled = window.loading || !!next.refreshRequired;earlier.setAttribute('aria-disabled', String(earlier.disabled)); refresh.disabled = window.loading || next.validating;retry.hidden = !window.error || !!next.refreshRequired; - reload.hidden = !next.refreshRequired;reload.textContent = next.refreshLabel || 'Reload recorded history'; + reload.hidden = !next.refreshRequired;setText(reload, next.refreshLabel || 'Reload recorded history'); timeline.hidden = !items.length; let initialAnchor = null; if (!initialized && items.length && !window.loading && !next.validating && !next.refreshRequired) {