Skip to content

Commit a0b5f7d

Browse files
JohnMcLearclaude
andcommitted
fix: rewrite page down/up to use pixel-based line counting
The previous approach tried to scroll the outerWin iframe element directly which didn't work. Reverted to the original cursor-movement approach but calculates lines-to-skip using viewport pixel height divided by actual rendered line heights. This correctly handles long wrapped lines that consume multiple visual rows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent df5102c commit a0b5f7d

File tree

1 file changed

+44
-33
lines changed

1 file changed

+44
-33
lines changed

src/static/js/ace2_inner.ts

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2883,43 +2883,54 @@ function Ace2Inner(editorInfo, cssManagers) {
28832883
const isPageDown = evt.which === 34;
28842884
const isPageUp = evt.which === 33;
28852885

2886-
if ((isPageDown && padShortcutEnabled.pageDown) ||
2887-
(isPageUp && padShortcutEnabled.pageUp)) {
2888-
// Scroll by actual viewport height in pixels, not by line count.
2889-
// This fixes the case where very long wrapped lines consume the
2890-
// entire viewport, making line-count-based scrolling useless.
2891-
const viewportHeight = outerDoc.documentElement.clientHeight;
2892-
// Keep a small overlap so the user doesn't lose context
2893-
const scrollAmount = viewportHeight - 40;
2894-
const currentScrollY = scroll.getScrollY();
2895-
2896-
if (isPageDown) {
2897-
scroll.setScrollY(currentScrollY + scrollAmount);
2898-
} else {
2899-
scroll.setScrollY(Math.max(0, currentScrollY - scrollAmount));
2900-
}
2886+
const oldVisibleLineRange = scroll.getVisibleLineRange(rep);
2887+
let topOffset = rep.selStart[0] - oldVisibleLineRange[0];
2888+
if (topOffset < 0) topOffset = 0;
29012889

2902-
// Move cursor into the new visible area
2903-
scheduler.setTimeout(() => {
2904-
const linesCount = rep.lines.length();
2905-
const newVisibleRange = scroll.getVisibleLineRange(rep);
2890+
scheduler.setTimeout(() => {
2891+
const newVisibleLineRange = scroll.getVisibleLineRange(rep);
2892+
const linesCount = rep.lines.length();
29062893

2907-
if (isPageDown) {
2908-
// Place cursor at the first line of the new viewport
2909-
rep.selStart[0] = newVisibleRange[0];
2910-
rep.selEnd[0] = newVisibleRange[0];
2911-
} else {
2912-
// Place cursor at the last line of the new viewport
2913-
rep.selEnd[0] = newVisibleRange[1];
2914-
rep.selStart[0] = newVisibleRange[1];
2894+
// Calculate lines to skip based on viewport pixel height divided by
2895+
// the average rendered line height. This correctly handles long wrapped
2896+
// lines that consume multiple visual rows (fixes #4562).
2897+
const viewportHeight = outerDoc.documentElement.clientHeight;
2898+
const visibleStart = newVisibleLineRange[0];
2899+
const visibleEnd = newVisibleLineRange[1];
2900+
let totalPixelHeight = 0;
2901+
for (let i = visibleStart; i <= Math.min(visibleEnd, linesCount - 1); i++) {
2902+
const entry = rep.lines.atIndex(i);
2903+
if (entry && entry.lineNode) {
2904+
totalPixelHeight += entry.lineNode.offsetHeight;
29152905
}
2906+
}
2907+
const visibleLogicalLines = visibleEnd - visibleStart;
2908+
// Use pixel-based count: how many logical lines fit in one viewport
2909+
const numberOfLinesInViewport = visibleLogicalLines > 0 && totalPixelHeight > 0
2910+
? Math.max(1, Math.round(visibleLogicalLines * viewportHeight / totalPixelHeight))
2911+
: Math.max(1, visibleLogicalLines);
2912+
2913+
if (isPageUp && padShortcutEnabled.pageUp) {
2914+
rep.selStart[0] -= numberOfLinesInViewport;
2915+
rep.selEnd[0] -= numberOfLinesInViewport;
2916+
}
29162917

2917-
// clamp to valid line range
2918-
rep.selStart[0] = Math.max(0, Math.min(rep.selStart[0], linesCount - 1));
2919-
rep.selEnd[0] = Math.max(0, Math.min(rep.selEnd[0], linesCount - 1));
2920-
updateBrowserSelectionFromRep();
2921-
}, 0);
2922-
}
2918+
if (isPageDown && padShortcutEnabled.pageDown) {
2919+
rep.selStart[0] += numberOfLinesInViewport;
2920+
rep.selEnd[0] += numberOfLinesInViewport;
2921+
}
2922+
2923+
// clamp to valid line range
2924+
rep.selStart[0] = Math.max(0, Math.min(rep.selStart[0], linesCount - 1));
2925+
rep.selEnd[0] = Math.max(0, Math.min(rep.selEnd[0], linesCount - 1));
2926+
updateBrowserSelectionFromRep();
2927+
// scroll to the caret position
2928+
const myselection = targetDoc.getSelection();
2929+
let caretOffsetTop = myselection.focusNode.parentNode.offsetTop ||
2930+
myselection.focusNode.offsetTop;
2931+
if (caretOffsetTop === -1) caretOffsetTop = myselection.focusNode.offsetTop;
2932+
scroll.setScrollY(caretOffsetTop);
2933+
}, 200);
29232934
}
29242935
}
29252936

0 commit comments

Comments
 (0)