diff --git a/src/static/js/ace2_inner.ts b/src/static/js/ace2_inner.ts index 3ca880a9484..bf61e8cb88f 100644 --- a/src/static/js/ace2_inner.ts +++ b/src/static/js/ace2_inner.ts @@ -2879,22 +2879,36 @@ function Ace2Inner(editorInfo, cssManagers) { // This is required, browsers will try to do normal default behavior on // page up / down and the default behavior SUCKS evt.preventDefault(); - const oldVisibleLineRange = scroll.getVisibleLineRange(rep); - let topOffset = rep.selStart[0] - oldVisibleLineRange[0]; - if (topOffset < 0) { - topOffset = 0; - } const isPageDown = evt.which === 34; const isPageUp = evt.which === 33; + const oldVisibleLineRange = scroll.getVisibleLineRange(rep); + let topOffset = rep.selStart[0] - oldVisibleLineRange[0]; + if (topOffset < 0) topOffset = 0; + scheduler.setTimeout(() => { - // the visible lines IE 1,10 const newVisibleLineRange = scroll.getVisibleLineRange(rep); - // total count of lines in pad IE 10 const linesCount = rep.lines.length(); - // How many lines are in the viewport right now? - const numberOfLinesInViewport = newVisibleLineRange[1] - newVisibleLineRange[0]; + + // Calculate lines to skip based on viewport pixel height divided by + // the average rendered line height. This correctly handles long wrapped + // lines that consume multiple visual rows (fixes #4562). + const viewportHeight = getInnerHeight(); + const visibleStart = newVisibleLineRange[0]; + const visibleEnd = newVisibleLineRange[1]; + let totalPixelHeight = 0; + for (let i = visibleStart; i <= Math.min(visibleEnd, linesCount - 1); i++) { + const entry = rep.lines.atIndex(i); + if (entry && entry.lineNode) { + totalPixelHeight += entry.lineNode.offsetHeight; + } + } + const visibleLogicalLines = visibleEnd - visibleStart + 1; + // Use pixel-based count: how many logical lines fit in one viewport + const numberOfLinesInViewport = visibleLogicalLines > 0 && totalPixelHeight > 0 + ? Math.max(1, Math.round(visibleLogicalLines * viewportHeight / totalPixelHeight)) + : Math.max(1, visibleLogicalLines); if (isPageUp && padShortcutEnabled.pageUp) { rep.selStart[0] -= numberOfLinesInViewport; @@ -2910,18 +2924,11 @@ function Ace2Inner(editorInfo, cssManagers) { 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 + // scroll to the caret position const myselection = targetDoc.getSelection(); - // get the carets selection offset in px IE 214 let caretOffsetTop = myselection.focusNode.parentNode.offsetTop || myselection.focusNode.offsetTop; - - // sometimes the first selection is -1 which causes problems - // (Especially with ep_page_view) - // so use focusNode.offsetTop value. if (caretOffsetTop === -1) caretOffsetTop = myselection.focusNode.offsetTop; - // set the scrollY offset of the viewport on the document scroll.setScrollY(caretOffsetTop); }, 200); } diff --git a/src/tests/frontend-new/specs/page_up_down.spec.ts b/src/tests/frontend-new/specs/page_up_down.spec.ts index d1f395bb99d..640792b82c2 100644 --- a/src/tests/frontend-new/specs/page_up_down.spec.ts +++ b/src/tests/frontend-new/specs/page_up_down.spec.ts @@ -84,6 +84,65 @@ test.describe('Page Up / Page Down', function () { expect(selection).toBeLessThan(50); }); + // Regression test for #4562: consecutive very long wrapped lines should not + // cause PageDown/PageUp to skip too many or too few logical lines. The + // pixel-based calculation must account for lines that occupy far more visual + // rows than the viewport height. + test('PageDown with consecutive long wrapped lines moves by correct amount (#4562)', async function ({page}) { + const padBody = await getPadBody(page); + await clearPadContent(page); + + // Build a pad with long lines interspersed with short ones via the inner + // document directly to avoid slow keyboard.type on Firefox. + const longLine = 'word '.repeat(300); + const innerFrame = page.frame('ace_inner')!; + await innerFrame.evaluate((text: string) => { + const body = document.getElementById('innerdocbody')!; + body.innerHTML = ''; + for (let i = 0; i < 6; i++) { + const longDiv = document.createElement('div'); + longDiv.textContent = text; + body.appendChild(longDiv); + const shortDiv = document.createElement('div'); + shortDiv.textContent = `short ${i}`; + body.appendChild(shortDiv); + } + }, longLine); + // Wait for Etherpad to process the DOM changes + await page.waitForTimeout(2000); + + // Move caret to the very top + await page.keyboard.down('Control'); + await page.keyboard.press('Home'); + await page.keyboard.up('Control'); + await page.waitForTimeout(200); + + // Press PageDown twice and verify caret advances each time + const getCaretLine = async () => { + return innerFrame.evaluate(() => { + const sel = document.getSelection(); + if (!sel || !sel.focusNode) return -1; + let node = sel.focusNode as HTMLElement; + while (node && node.tagName !== 'DIV') node = node.parentElement!; + if (!node) return -1; + const divs = Array.from(document.getElementById('innerdocbody')!.children); + return divs.indexOf(node); + }); + }; + + const lineBefore = await getCaretLine(); + + await page.keyboard.press('PageDown'); + await page.waitForTimeout(1000); + const lineAfterFirst = await getCaretLine(); + expect(lineAfterFirst).toBeGreaterThan(lineBefore); + + await page.keyboard.press('PageDown'); + await page.waitForTimeout(1000); + const lineAfterSecond = await getCaretLine(); + expect(lineAfterSecond).toBeGreaterThan(lineAfterFirst); + }); + test('PageDown then PageUp returns to approximately same position', async function ({page}) { const padBody = await getPadBody(page); await clearPadContent(page);