Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/static/js/ace2_inner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2994,6 +2994,25 @@ function Ace2Inner(editorInfo, cssManagers) {
lineAndColumnFromChar(selectionInfo.selStart),
lineAndColumnFromChar(selectionInfo.selEnd),
selectionInfo.selFocusAtStart);
// Issue #7007: bring the caret's line into view after
// undo/redo so the user can actually see the change that
// just got reverted. The outer inCallStack's finally-block
// scroll path is fragile on large pads — in particular
// `scrollNodeVerticallyIntoView`'s caret-below-viewport
// branch intentionally scrolls to a fixed offset to keep
// the Enter-on-last-line experience smooth (see PR #4639),
// which leaves undo/redo pointed at the wrong spot
// whenever the caret jumps to a mid-document line. Using
// Element.scrollIntoView with block:"center" is native,
// framework-agnostic, and matches the behavior other
// editors (gedit, libreoffice) use.
const focusPoint = selectionInfo.selFocusAtStart
? lineAndColumnFromChar(selectionInfo.selStart)
: lineAndColumnFromChar(selectionInfo.selEnd);
const caretLineNode = rep.lines.atIndex(focusPoint[0])?.lineNode;
if (caretLineNode && typeof caretLineNode.scrollIntoView === 'function') {
caretLineNode.scrollIntoView({block: 'center', behavior: 'auto'});
}
}
const oldEvent = currentCallStack.startNewEvent(oldEventType, true);
return oldEvent;
Expand Down
104 changes: 104 additions & 0 deletions src/tests/frontend-new/specs/undo_redo_scroll.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import {expect, test} from "@playwright/test";
import {clearPadContent, getPadBody, goToNewPad} from "../helper/padHelper";

test.beforeEach(async ({page}) => {
await goToNewPad(page);
});

// Regression test for https://github.com/ether/etherpad/issues/7007
//
// Pre-fix: after undo on a large pad, the viewport did not scroll to
// follow the caret. When the caret landed below the current viewport,
// src/static/js/scroll.ts's caretIsBelowOfViewport branch ran
// `outer.scrollTo(0, outer[0].innerHeight)` — a fixed offset, not the
// caret position — so the user couldn't see what had just been
// modified. That special-case was intended for "Enter at the very end
// of the pad" (PR #4639); it misbehaved whenever undo/redo or another
// path moved the caret to an arbitrary line below the viewport.
test.describe('Undo scroll-to-caret (#7007)', function () {
test.describe.configure({retries: 2});

// Use the Etherpad keyboard path so the undo module has real
// changesets to replay. 45 lines is enough to push the pad well past
// a typical CI headless viewport (~900px × ~20px per line).
const LINE_COUNT = 45;

test('Ctrl+Z scrolls viewport up when the caret lands above the view', async function ({page}) {
await (await getPadBody(page)).click();
await clearPadContent(page);

// Type LINE_COUNT short lines through the real editor (so every line
// lands in a changeset the undo module can reverse).
for (let i = 0; i < LINE_COUNT; i++) {
await page.keyboard.type(`line ${i + 1}`);
await page.keyboard.press('Enter');
}
await page.waitForTimeout(300);

// Move caret to the top, insert a single edit the undo will reverse.
await page.keyboard.down('Control');
await page.keyboard.press('Home');
await page.keyboard.up('Control');
await page.keyboard.type('X');
await page.waitForTimeout(300);

// Scroll the outer frame all the way down so the edit is out of view.
const outerFrame = page.frame('ace_outer')!;
await outerFrame.evaluate(() => {
window.scrollTo(0, document.body.scrollHeight);
});
await page.waitForTimeout(300);

const scrollBefore = await outerFrame.evaluate(
() => window.scrollY || document.scrollingElement?.scrollTop || 0);
expect(scrollBefore).toBeGreaterThan(0); // sanity: viewport actually scrolled

// Undo — caret returns to the top, viewport should follow.
await page.keyboard.press('Control+Z');
// scrollNodeVerticallyIntoView's caret-below branch uses a 150ms
// setTimeout; give it a generous budget for CI.
await page.waitForTimeout(800);

const scrollAfter = await outerFrame.evaluate(
() => window.scrollY || document.scrollingElement?.scrollTop || 0);

// Pre-fix: scrollAfter ≈ scrollBefore (no scroll).
// Fixed: scrollAfter < scrollBefore (viewport moved up toward the caret).
expect(scrollAfter).toBeLessThan(scrollBefore);
});

test('Ctrl+Z scrolls viewport down when the caret lands below the view', async function ({page}) {
await (await getPadBody(page)).click();
await clearPadContent(page);

for (let i = 0; i < LINE_COUNT; i++) {
await page.keyboard.type(`line ${i + 1}`);
await page.keyboard.press('Enter');
}
await page.waitForTimeout(300);

// Caret is already at the bottom (after the last Enter). Type an
// edit there, then scroll to top.
await page.keyboard.type('Y');
await page.waitForTimeout(300);

const outerFrame = page.frame('ace_outer')!;
await outerFrame.evaluate(() => window.scrollTo(0, 0));
await page.waitForTimeout(300);

const scrollBefore = await outerFrame.evaluate(
() => window.scrollY || document.scrollingElement?.scrollTop || 0);
expect(scrollBefore).toBe(0);

await page.keyboard.press('Control+Z');
await page.waitForTimeout(800);

const scrollAfter = await outerFrame.evaluate(
() => window.scrollY || document.scrollingElement?.scrollTop || 0);

// Pre-fix: scrollAfter was pinned to outer[0].innerHeight (a fixed
// offset) or stayed at 0; either way it was not the caret location.
// Fixed: the viewport scrolls down toward the caret at the bottom.
expect(scrollAfter).toBeGreaterThan(0);
});
});
Loading