Skip to content

Commit de449b7

Browse files
JohnMcLearclaude
andcommitted
fix: anchor reapply loop cancels on user interaction
Addresses Qodo review: the 10s reapply loop could fight the user when they tried to scroll or click away from the anchored line. Listen for wheel/touchmove/keydown/mousedown on both ace_outer and ace_inner documents in capture phase and tear down the interval on first signal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 65076b8 commit de449b7

File tree

2 files changed

+62
-4
lines changed

2 files changed

+62
-4
lines changed

src/static/js/pad_editor.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,8 +255,8 @@ const focusOnHashedLine = (ace, lineNumberInt) => {
255255
exports.focusOnLine = (ace) => {
256256
const lineNumberInt = getHashedLineNumber();
257257
if (lineNumberInt == null) return;
258+
const $aceOuter = $('iframe[name="ace_outer"]');
258259
const getCurrentTargetOffset = () => {
259-
const $aceOuter = $('iframe[name="ace_outer"]');
260260
const $inner = $aceOuter.contents().find('iframe').contents().find('#innerdocbody');
261261
const line = $inner.find(`div:nth-child(${lineNumberInt})`);
262262
if (line.length === 0) return null;
@@ -266,20 +266,44 @@ exports.focusOnLine = (ace) => {
266266
const maxSettleDuration = 10000;
267267
const settleInterval = 250;
268268
const startTime = Date.now();
269-
let intervalId = null;
269+
let intervalId: number | null = null;
270+
271+
const userEventNames = ['wheel', 'touchmove', 'keydown', 'mousedown'];
272+
const docs: Document[] = [];
273+
const stop = () => {
274+
if (intervalId != null) {
275+
window.clearInterval(intervalId);
276+
intervalId = null;
277+
}
278+
for (const doc of docs) {
279+
for (const name of userEventNames) doc.removeEventListener(name, stop, true);
280+
}
281+
docs.length = 0;
282+
};
270283

271284
const focusUntilStable = () => {
272285
if (Date.now() - startTime >= maxSettleDuration) {
273-
window.clearInterval(intervalId);
286+
stop();
274287
return;
275288
}
276289
const currentOffsetTop = getCurrentTargetOffset();
277290
if (currentOffsetTop == null) return;
278-
279291
focusOnHashedLine(ace, lineNumberInt);
280292
};
281293

282294
focusUntilStable();
283295
intervalId = window.setInterval(focusUntilStable, settleInterval);
296+
// Stop fighting the user: any deliberate scroll, tap, click, or keystroke cancels the
297+
// reapply loop so late layout corrections do not steal focus once the user takes over.
298+
// Listen on both the ace_outer and ace_inner documents in capture phase so we see the
299+
// user's intent even if inner handlers stopPropagation().
300+
const outerDoc = ($aceOuter.contents()[0] as any) as Document | undefined;
301+
const innerIframe = $aceOuter.contents().find('iframe')[0] as HTMLIFrameElement | undefined;
302+
const innerDoc = innerIframe?.contentDocument;
303+
for (const doc of [outerDoc, innerDoc]) {
304+
if (!doc) continue;
305+
docs.push(doc);
306+
for (const name of userEventNames) doc.addEventListener(name, stop, true);
307+
}
284308
// End of setSelection / set Y position of editor
285309
};

src/tests/frontend-new/specs/anchor_scroll.spec.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,38 @@ test.describe('anchor scrolling', () => {
4747
return Math.abs(currentViewportTop - initialViewportTop);
4848
}).toBeLessThanOrEqual(80);
4949
});
50+
51+
test('user scroll cancels the reapply loop so navigation is not locked', async ({page}) => {
52+
await goToNewPad(page);
53+
const padUrl = page.url();
54+
await clearPadContent(page);
55+
await writeToPad(page, Array.from({length: 30}, (_v, i) => `Line ${i + 1}`).join('\n'));
56+
await page.waitForTimeout(1000);
57+
58+
await page.goto('about:blank');
59+
await page.goto(`${padUrl}#L20`);
60+
await page.waitForSelector('iframe[name="ace_outer"]');
61+
await page.waitForSelector('#editorcontainer.initialized');
62+
63+
const outerDoc = page.frameLocator('iframe[name="ace_outer"]').locator('#outerdocbody');
64+
const getScrollTop = async () => await outerDoc.evaluate(
65+
(el) => el.parentElement?.scrollTop || 0);
66+
67+
await expect.poll(getScrollTop).toBeGreaterThan(10);
68+
69+
// User interacts with the pad. The anchor-scroll handler listens for
70+
// wheel/mousedown/keydown/touchmove on the outer iframe document and must cancel
71+
// its reapply loop. We dispatch a mousedown on the outer document, then reset
72+
// scrollTop to 0 and verify it stays there.
73+
await outerDoc.evaluate((el) => {
74+
const doc = el.ownerDocument;
75+
doc.dispatchEvent(new MouseEvent('mousedown', {bubbles: true}));
76+
if (el.parentElement) el.parentElement.scrollTop = 0;
77+
});
78+
79+
// Give the reapply loop several ticks to attempt a re-scroll. If cancellation worked,
80+
// scrollTop stays near 0 instead of snapping back to the anchor.
81+
await page.waitForTimeout(1500);
82+
expect(await getScrollTop()).toBeLessThanOrEqual(20);
83+
});
5084
});

0 commit comments

Comments
 (0)