From 8cfe3492f968f13a073246480600fafa253e6c8f Mon Sep 17 00:00:00 2001 From: AshtonYoon Date: Mon, 12 Jan 2026 12:39:02 +0900 Subject: [PATCH 1/2] markdown: fix janky scrolling in preview with code blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #278930 This commit fixes the scroll synchronization issues between the markdown editor and preview when scrolling through code blocks: 1. **Code block line tracking**: Added endLine calculation for fenced code blocks by counting newlines, enabling accurate line-to-position mapping within multi-line code blocks. 2. **Padding-aware scrolling**: Implemented getContentBounds() to calculate scroll positions excluding padding, preventing jumps when scrolling through code blocks with padding. 3. **Scroll loop prevention**: Changed scroll disable flags from counters to timer-based booleans (50ms) to prevent infinite sync loops while maintaining smooth bidirectional synchronization. The fix ensures smooth scrolling through code blocks without jumping or stuttering in both directions (editor ↔ preview). --- .../preview-src/index.ts | 21 ++- .../preview-src/scroll-sync.ts | 139 ++++++++++++++++-- .../src/preview/preview.ts | 8 +- 3 files changed, 149 insertions(+), 19 deletions(-) diff --git a/extensions/markdown-language-features/preview-src/index.ts b/extensions/markdown-language-features/preview-src/index.ts index b6200b8ceb9..80c88186d36 100644 --- a/extensions/markdown-language-features/preview-src/index.ts +++ b/extensions/markdown-language-features/preview-src/index.ts @@ -14,6 +14,7 @@ import type { ToWebviewMessage } from '../types/previewMessaging'; import { isOfScheme, Schemes } from '../src/util/schemes'; let scrollDisabledCount = 0; +let scrollDisabledTimer: number | undefined; const marker = new ActiveLineMarker(); const settings = new SettingsManager(); @@ -125,7 +126,13 @@ onceDocumentLoaded(() => { const onUpdateView = (() => { const doScroll = throttle((line: number) => { - scrollDisabledCount += 1; + scrollDisabledCount = 1; + if (scrollDisabledTimer) { + clearTimeout(scrollDisabledTimer); + } + scrollDisabledTimer = window.setTimeout(() => { + scrollDisabledCount = 0; + }, 50); doAfterImagesLoaded(() => scrollToRevealSourceLine(line, documentVersion, settings)); }, 50); @@ -366,12 +373,12 @@ window.addEventListener('scroll', throttle(() => { updateScrollProgress(); if (scrollDisabledCount > 0) { - scrollDisabledCount -= 1; - } else { - const line = getEditorLineNumberForPageOffset(window.scrollY, documentVersion); - if (typeof line === 'number' && !isNaN(line)) { - messaging.postMessage('revealLine', { line }); - } + return; + } + + const line = getEditorLineNumberForPageOffset(window.scrollY, documentVersion); + if (typeof line === 'number' && !isNaN(line)) { + messaging.postMessage('revealLine', { line }); } }, 50)); diff --git a/extensions/markdown-language-features/preview-src/scroll-sync.ts b/extensions/markdown-language-features/preview-src/scroll-sync.ts index 33d81094cb5..5846a1b8bcb 100644 --- a/extensions/markdown-language-features/preview-src/scroll-sync.ts +++ b/extensions/markdown-language-features/preview-src/scroll-sync.ts @@ -15,6 +15,7 @@ export class CodeLineElement { readonly element: HTMLElement, readonly line: number, readonly codeElement?: HTMLElement, + readonly endLine?: number, ) { this._detailParentElements = Array.from(getParentsWithTagName(element, 'DETAILS')); } @@ -56,10 +57,19 @@ const getCodeLineElements = (() => { } + + if (element.tagName === 'CODE' && element.parentElement && element.parentElement.tagName === 'PRE') { // Fenced code blocks are a special case since the `code-line` can only be marked on // the `` element and not the parent `
` element.
-					cachedElements.push(new CodeLineElement(element.parentElement, line, element));
+					// Calculate the end line by counting newlines in the code block
+					const text = element.textContent || '';
+					const lineCount = (text.match(/\n/g) || []).length + 1;
+					const endLine = line + lineCount - 1;
+					cachedElements.push(new CodeLineElement(element.parentElement, line, element, endLine));
+				} else if (element.tagName === 'PRE') {
+					// Skip PRE elements as they will be handled via their CODE children
+					// This prevents duplicate entries for the same line number
 				} else if (element.tagName === 'UL' || element.tagName === 'OL') {
 					// Skip adding list elements since the first child has the same code line (and should be preferred)
 				} else {
@@ -112,6 +122,7 @@ export function getLineElementsAtPageOffset(offset: number, documentVersion: num
 	}
 	const hiElement = lines[hi];
 	const hiBounds = getElementBounds(hiElement);
+
 	if (hi >= 1 && hiBounds.top > position) {
 		const loElement = lines[lo];
 		return { previous: loElement, next: hiElement };
@@ -122,9 +133,16 @@ export function getLineElementsAtPageOffset(offset: number, documentVersion: num
 	return { previous: hiElement };
 }
 
-function getElementBounds({ element }: CodeLineElement): { top: number; height: number } {
+function getElementBounds(codeLineElement: CodeLineElement): { top: number; height: number } {
+	const { element, codeElement } = codeLineElement;
 	const myBounds = element.getBoundingClientRect();
 
+	// For fenced code blocks (PRE elements containing CODE), use the full height
+	// Don't look for children as the CODE element itself would be found as a child
+	if (codeElement) {
+		return myBounds;
+	}
+
 	// Some code line elements may contain other code line elements.
 	// In those cases, only take the height up to that child.
 	const codeLineChild = element.querySelector(`.${codeLineClass}`);
@@ -140,6 +158,43 @@ function getElementBounds({ element }: CodeLineElement): { top: number; height:
 	return myBounds;
 }
 
+/**
+ * Get the content bounds for a code line element, accounting for padding.
+ * For code blocks, returns the bounds of the content area (excluding padding).
+ * For other elements, returns the same as getElementBounds.
+ */
+function getContentBounds(codeLineElement: CodeLineElement): {
+	top: number;
+	height: number;
+	paddingTop: number;
+	paddingBottom: number;
+} {
+	const { element, codeElement } = codeLineElement;
+	const bounds = getElementBounds(codeLineElement);
+
+	// For code blocks (PRE elements), account for padding
+	if (codeElement) {
+		const computedStyle = window.getComputedStyle(element);
+		const paddingTop = parseFloat(computedStyle.paddingTop) || 0;
+		const paddingBottom = parseFloat(computedStyle.paddingBottom) || 0;
+
+		return {
+			top: bounds.top + paddingTop,
+			height: bounds.height - paddingTop - paddingBottom,
+			paddingTop,
+			paddingBottom
+		};
+	}
+
+	// For non-code elements, no padding adjustment needed
+	return {
+		top: bounds.top,
+		height: bounds.height,
+		paddingTop: 0,
+		paddingBottom: 0
+	};
+}
+
 /**
  * Attempt to reveal the element for a source line in the editor.
  */
@@ -160,17 +215,46 @@ export function scrollToRevealSourceLine(line: number, documentVersion: number,
 	let scrollTo = 0;
 	const rect = getElementBounds(previous);
 	const previousTop = rect.top;
-	if (next && next.line !== previous.line) {
-		// Between two elements. Go to percentage offset between them.
+
+
+	// Check if previous is a multi-line code block
+	if (previous.endLine && previous.endLine > previous.line) {
+		if (line < previous.endLine) {
+			// We're inside the code block - scroll proportionally through its content height (excluding padding)
+			const contentBounds = getContentBounds(previous);
+			const progressInCodeBlock = (line - previous.line) / (previous.endLine - previous.line);
+
+
+			// Calculate absolute position to content area
+			const contentAbsoluteTop = window.scrollY + contentBounds.top;
+			const targetAbsoluteY = contentAbsoluteTop + (contentBounds.height * progressInCodeBlock);
+			scrollTo = targetAbsoluteY;
+
+		} else if (next && next.line !== previous.line) {
+			// We're after the code block but before the next element
+			const betweenProgress = (line - previous.endLine) / (next.line - previous.endLine);
+			const elementAbsoluteEnd = window.scrollY + previousTop + rect.height;
+			const nextAbsoluteTop = window.scrollY + next.element.getBoundingClientRect().top;
+			const betweenHeight = nextAbsoluteTop - elementAbsoluteEnd;
+			scrollTo = elementAbsoluteEnd + betweenProgress * betweenHeight;
+		} else {
+			// Shouldn't happen, but fall back to end of element
+			scrollTo = window.scrollY + previousTop + rect.height;
+		}
+	} else if (next && next.line !== previous.line) {
+		// Original logic: Between two elements. Go to percentage offset between them.
 		const betweenProgress = (line - previous.line) / (next.line - previous.line);
-		const previousEnd = previousTop + rect.height;
-		const betweenHeight = next.element.getBoundingClientRect().top - previousEnd;
-		scrollTo = previousEnd + betweenProgress * betweenHeight;
+		const elementAbsoluteEnd = window.scrollY + previousTop + rect.height;
+		const nextAbsoluteTop = window.scrollY + next.element.getBoundingClientRect().top;
+		const betweenHeight = nextAbsoluteTop - elementAbsoluteEnd;
+		scrollTo = elementAbsoluteEnd + betweenProgress * betweenHeight;
 	} else {
 		const progressInElement = line - Math.floor(line);
-		scrollTo = previousTop + (rect.height * progressInElement);
+		scrollTo = window.scrollY + previousTop + (rect.height * progressInElement);
 	}
-	window.scroll(window.scrollX, Math.max(1, window.scrollY + scrollTo));
+
+
+	window.scroll(window.scrollX, Math.max(1, scrollTo));
 }
 
 export function getEditorLineNumberForPageOffset(offset: number, documentVersion: number): number | null {
@@ -181,12 +265,44 @@ export function getEditorLineNumberForPageOffset(offset: number, documentVersion
 		}
 		const previousBounds = getElementBounds(previous);
 		const offsetFromPrevious = (offset - window.scrollY - previousBounds.top);
+
+
+		// Check if previous is a multi-line code block
+		if (previous.endLine && previous.endLine > previous.line) {
+			// Use content bounds to exclude padding from the calculation
+			const contentBounds = getContentBounds(previous);
+			const offsetFromContent = offset - window.scrollY - contentBounds.top;
+
+
+			// Check if we're within the code block's content area (excluding padding)
+			if (offsetFromContent >= 0 && offsetFromContent <= contentBounds.height) {
+				const progressWithinCodeBlock = offsetFromContent / contentBounds.height;
+				const calculatedLine = previous.line + progressWithinCodeBlock * (previous.endLine - previous.line);
+				return calculatedLine;
+			} else if (next && offsetFromContent > contentBounds.height) {
+				// We're in the gap after the code block content (including bottom padding)
+				const gapOffset = offsetFromContent - contentBounds.height;
+				const nextBounds = getElementBounds(next);
+				const contentEnd = contentBounds.top + contentBounds.height;
+				const gapHeight = nextBounds.top - contentEnd;
+				const progressInGap = gapOffset / gapHeight;
+				const calculatedLine = previous.endLine + progressInGap * (next.line - previous.endLine);
+				return calculatedLine;
+			} else if (offsetFromContent < 0) {
+				// We're in the top padding area
+				// Fall through to original logic
+			}
+		}
+
+		// Original logic
 		if (next) {
 			const progressBetweenElements = offsetFromPrevious / (getElementBounds(next).top - previousBounds.top);
-			return previous.line + progressBetweenElements * (next.line - previous.line);
+			const calculatedLine = previous.line + progressBetweenElements * (next.line - previous.line);
+			return calculatedLine;
 		} else {
 			const progressWithinElement = offsetFromPrevious / (previousBounds.height);
-			return previous.line + progressWithinElement;
+			const calculatedLine = previous.line + progressWithinElement;
+			return calculatedLine;
 		}
 	}
 	return null;
@@ -208,3 +324,4 @@ function* getParentsWithTagName(element: HTMLElement, tag
 		}
 	}
 }
+
diff --git a/extensions/markdown-language-features/src/preview/preview.ts b/extensions/markdown-language-features/src/preview/preview.ts
index 19d1755e7eb..819702da3f2 100644
--- a/extensions/markdown-language-features/src/preview/preview.ts
+++ b/extensions/markdown-language-features/src/preview/preview.ts
@@ -57,6 +57,7 @@ class MarkdownPreview extends Disposable implements WebviewResourceProvider {
 	private _firstUpdate = true;
 	private _currentVersion?: PreviewDocumentVersion;
 	private _isScrolling = false;
+	private _scrollingTimer?: NodeJS.Timeout;
 
 	private _imageInfo: readonly ImageInfo[] = [];
 	private readonly _fileWatchersBySrc = new Map();
@@ -219,7 +220,6 @@ class MarkdownPreview extends Disposable implements WebviewResourceProvider {
 		}
 
 		if (this._isScrolling) {
-			this._isScrolling = false;
 			return;
 		}
 
@@ -299,6 +299,12 @@ class MarkdownPreview extends Disposable implements WebviewResourceProvider {
 			}
 
 			this._isScrolling = true;
+			if (this._scrollingTimer) {
+				clearTimeout(this._scrollingTimer);
+			}
+			this._scrollingTimer = setTimeout(() => {
+				this._isScrolling = false;
+			}, 200);
 			scrollEditorToLine(line, editor);
 		}
 	}

From ce82477188222ea226fbfb0c8a030fcfd31e7412 Mon Sep 17 00:00:00 2001
From: AshtonYoon 
Date: Mon, 19 Jan 2026 15:22:35 +0900
Subject: [PATCH 2/2] markdown: remove extra blank lines

---
 .../markdown-language-features/preview-src/scroll-sync.ts    | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/extensions/markdown-language-features/preview-src/scroll-sync.ts b/extensions/markdown-language-features/preview-src/scroll-sync.ts
index 5846a1b8bcb..4f5e3f7d2fe 100644
--- a/extensions/markdown-language-features/preview-src/scroll-sync.ts
+++ b/extensions/markdown-language-features/preview-src/scroll-sync.ts
@@ -56,9 +56,6 @@ const getCodeLineElements = (() => {
 					continue;
 				}
 
-
-
-
 				if (element.tagName === 'CODE' && element.parentElement && element.parentElement.tagName === 'PRE') {
 					// Fenced code blocks are a special case since the `code-line` can only be marked on
 					// the `` element and not the parent `
` element.
@@ -253,7 +250,6 @@ export function scrollToRevealSourceLine(line: number, documentVersion: number,
 		scrollTo = window.scrollY + previousTop + (rect.height * progressInElement);
 	}
 
-
 	window.scroll(window.scrollX, Math.max(1, scrollTo));
 }
 
@@ -324,4 +320,3 @@ function* getParentsWithTagName(element: HTMLElement, tag
 		}
 	}
 }
-