fix: PageDown now advances caret by a full page of lines (#7437)

PageDown was broken — it moved the caret to the last visible line of
the current viewport instead of advancing by one page. This caused it
to get "stuck" at the bottom of the viewport.

The old code set the caret to oldVisibleLineRange[1] - 1 (the last
visible line), which was essentially a no-op for scrolling. The fix
mirrors the PageUp logic: advance/retreat by numberOfLinesInViewport.

Also simplified the clamping logic for both selStart and selEnd.

Fixes: https://github.com/ether/etherpad-lite/issues/6710

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
John McLear 2026-04-03 07:15:58 +01:00 committed by GitHub
parent 4ce2a1acd1
commit 712bfe6ac4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 129 additions and 23 deletions

View file

@ -2849,35 +2849,18 @@ function Ace2Inner(editorInfo, cssManagers) {
const numberOfLinesInViewport = newVisibleLineRange[1] - newVisibleLineRange[0];
if (isPageUp && padShortcutEnabled.pageUp) {
// move to the bottom line +1 in the viewport (essentially skipping over a page)
rep.selEnd[0] -= numberOfLinesInViewport;
// move to the bottom line +1 in the viewport (essentially skipping over a page)
rep.selStart[0] -= numberOfLinesInViewport;
rep.selEnd[0] -= numberOfLinesInViewport;
}
// if we hit page down
if (isPageDown && padShortcutEnabled.pageDown) {
// If the new viewpoint position is actually further than where we are right now
if (rep.selEnd[0] >= oldVisibleLineRange[0]) {
// dont go further in the page down than what's visible IE go from 0 to 50
// if 50 is visible on screen but dont go below that else we miss content
rep.selStart[0] = oldVisibleLineRange[1] - 1;
// dont go further in the page down than what's visible IE go from 0 to 50
// if 50 is visible on screen but dont go below that else we miss content
rep.selEnd[0] = oldVisibleLineRange[1] - 1;
}
rep.selStart[0] += numberOfLinesInViewport;
rep.selEnd[0] += numberOfLinesInViewport;
}
// ensure min and max
if (rep.selEnd[0] < 0) {
rep.selEnd[0] = 0;
}
if (rep.selStart[0] < 0) {
rep.selStart[0] = 0;
}
if (rep.selEnd[0] >= linesCount) {
rep.selEnd[0] = linesCount - 1;
}
// clamp to valid line range
rep.selStart[0] = Math.max(0, Math.min(rep.selStart[0], linesCount - 1));
rep.selEnd[0] = Math.max(0, Math.min(rep.selEnd[0], linesCount - 1));
updateBrowserSelectionFromRep();
// get the current caret selection, can't use rep. here because that only gives
// us the start position not the current

View file

@ -0,0 +1,123 @@
import {expect, test} from "@playwright/test";
import {clearPadContent, getPadBody, goToNewPad, writeToPad} from "../helper/padHelper";
test.beforeEach(async ({page}) => {
await goToNewPad(page);
});
// Regression test for https://github.com/ether/etherpad-lite/issues/6710
test.describe('Page Up / Page Down', function () {
test.describe.configure({retries: 2});
test('PageDown moves caret forward by a page of lines', async function ({page}) {
const padBody = await getPadBody(page);
await clearPadContent(page);
// Create enough lines to require scrolling (more than a viewport)
for (let i = 0; i < 60; i++) {
await writeToPad(page, `line ${i + 1}`);
await page.keyboard.press('Enter');
}
// Move caret to the first line
await page.keyboard.down('Control');
await page.keyboard.press('Home');
await page.keyboard.up('Control');
await page.waitForTimeout(200);
// Press PageDown — the handler uses a 200ms setTimeout internally
await page.keyboard.press('PageDown');
await page.waitForTimeout(1000);
// The caret should have moved significantly forward (not stuck at the bottom of first viewport)
// Get the current line by checking which div has the caret
const innerFrame = page.frame('ace_inner')!;
const selection = await innerFrame.evaluate(() => {
const sel = document.getSelection();
if (!sel || !sel.focusNode) return 0;
// Walk up to find the div
let node = sel.focusNode as HTMLElement;
while (node && node.tagName !== 'DIV') node = node.parentElement!;
if (!node) return 0;
// Find the index of this div
const divs = Array.from(document.getElementById('innerdocbody')!.children);
return divs.indexOf(node);
});
// The caret should have advanced (viewport may be small in headless mode)
expect(selection).toBeGreaterThan(2);
});
test('PageUp moves caret backward by a page of lines', async function ({page}) {
const padBody = await getPadBody(page);
await clearPadContent(page);
// Create enough lines
for (let i = 0; i < 60; i++) {
await writeToPad(page, `line ${i + 1}`);
await page.keyboard.press('Enter');
}
// Move caret to the last line
await page.keyboard.down('Control');
await page.keyboard.press('End');
await page.keyboard.up('Control');
await page.waitForTimeout(200);
// Press PageUp
await page.keyboard.press('PageUp');
await page.waitForTimeout(500);
// The caret should have moved significantly backward
const innerFrame = page.frame('ace_inner')!;
const selection = await innerFrame.evaluate(() => {
const sel = document.getSelection();
if (!sel || !sel.focusNode) return 999;
let node = sel.focusNode as HTMLElement;
while (node && node.tagName !== 'DIV') node = node.parentElement!;
if (!node) return 999;
const divs = Array.from(document.getElementById('innerdocbody')!.children);
return divs.indexOf(node);
});
// The caret should be well before the last line (at least 10 lines from end)
expect(selection).toBeLessThan(50);
});
test('PageDown then PageUp returns to approximately same position', async function ({page}) {
const padBody = await getPadBody(page);
await clearPadContent(page);
for (let i = 0; i < 60; i++) {
await writeToPad(page, `line ${i + 1}`);
await page.keyboard.press('Enter');
}
// Start at top
await page.keyboard.down('Control');
await page.keyboard.press('Home');
await page.keyboard.up('Control');
await page.waitForTimeout(200);
// PageDown then PageUp
await page.keyboard.press('PageDown');
await page.waitForTimeout(1000);
await page.keyboard.press('PageUp');
await page.waitForTimeout(1000);
// Should be back near the top
const innerFrame = page.frame('ace_inner')!;
const selection = await innerFrame.evaluate(() => {
const sel = document.getSelection();
if (!sel || !sel.focusNode) return 999;
let node = sel.focusNode as HTMLElement;
while (node && node.tagName !== 'DIV') node = node.parentElement!;
if (!node) return 999;
const divs = Array.from(document.getElementById('innerdocbody')!.children);
return divs.indexOf(node);
});
// Should be back near the start (allow some drift due to viewport calculations)
expect(selection).toBeLessThan(8);
});
});