Fix markdown preview scroll sync after link navigation - #271555
Fix markdown preview scroll sync after link navigation#271555Michal Jurosz (mj41) wants to merge 1 commit into
Conversation
Fixes the issue where markdown preview doesn't scroll to cursor position when clicking back in the editor after clicking a link in the preview. Problem: When using split editor with markdown file and preview side-by-side: 1. User positions cursor at a specific line in editor 2. User clicks a link in the preview (navigates to different document) 3. User clicks back in the original editor at the same cursor position 4. Preview should scroll to show that cursor position, but doesn't Root Cause: The preview wasn't syncing scroll position when user interacted with the editor after the preview had navigated via link click. Additionally, there was a race condition when clicking at the exact same cursor position: the webview loses focus before the editor becomes the activeTextEditor, so vscode.window.activeTextEditor is undefined when the webview focus change event fires. Solution: 1. Added _needsScrollSync flag to track when preview navigates to different document and needs to sync on next editor interaction 2. Enhanced onDidChangeTextEditorSelection to call scrollTo() - handles clicking at different cursor positions 3. Added onDidChangeViewState handler with setTimeout(10ms) - handles the race condition when clicking at exact same cursor position. The delay allows the event loop to process editor activation before checking if sync is needed. Testing: - Clicking at different cursor positions: Works via selection change event - Clicking at same cursor position: Works via deferred webview focus check - Normal editing/scrolling: No unwanted syncing (flag-based control) - Multiple link navigations: Each navigation correctly syncs back
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a scroll synchronization issue in the Markdown preview where the preview fails to sync to the editor's cursor position after navigating via a link in the preview. The fix introduces a state flag to track when scroll sync is needed and handles both normal selection changes and an edge case where clicking at the same cursor position doesn't trigger selection change events.
Key Changes:
- Added
_needsScrollSyncflag to track when preview navigation requires scroll synchronization - Enhanced selection change handler to always sync preview scroll position to cursor
- Added webview focus handler with deferred check to handle race condition when clicking at the same cursor position
| } else if (line !== undefined) { | ||
| // Same document - sync preview to cursor position | ||
| // This handles clicking back in editor at the same cursor position | ||
| // after preview navigated via a link | ||
| this._preview.scrollTo(line); |
There was a problem hiding this comment.
The logic in lines 715-724 has redundant scroll synchronization. Lines 715-718 handle the case when _needsScrollSync is true, but lines 719-723 perform the same scrollTo() operation when _needsScrollSync is false. This means every editor activation with a visible line will trigger a scroll, which contradicts the stated purpose of the _needsScrollSync flag to 'prevent unwanted scrolling during normal editing'. Consider removing lines 719-724 or clarifying the intended behavior.
| } else if (line !== undefined) { | |
| // Same document - sync preview to cursor position | |
| // This handles clicking back in editor at the same cursor position | |
| // after preview navigated via a link | |
| this._preview.scrollTo(line); |
| // When preview loses focus (user clicks editor), vscode.window.activeTextEditor is not yet | ||
| // populated - it's still undefined because the editor activation happens after this event. | ||
| // This is a race condition in VS Code's event ordering. The 10ms setTimeout defers our check | ||
| // until the next event loop tick, allowing time for the editor to become the activeTextEditor. | ||
| // Without this delay, clicking at the same cursor position wouldn't trigger onDidChangeTextEditorSelection, | ||
| // and we'd miss the opportunity to sync the preview scroll position. | ||
| if (!e.webviewPanel.active && this._needsScrollSync) { | ||
| setTimeout(() => { | ||
| const editor = vscode.window.activeTextEditor; | ||
| if (editor && this._preview.isPreviewOf(editor.document.uri) && this._needsScrollSync) { | ||
| const cursorLine = editor.selection.active.line; | ||
| this._needsScrollSync = false; | ||
| this._preview.scrollTo(cursorLine); | ||
| } | ||
| }, 10); // 10ms delay allows editor activation to complete | ||
| } | ||
| })); | ||
|
|
There was a problem hiding this comment.
Using a hard-coded 10ms delay to handle race conditions is fragile and may not work reliably across different system loads or VS Code versions. Consider using vscode.window.onDidChangeActiveTextEditor event instead, or if the timing is truly unavoidable, use a named constant for the delay value to make it easier to adjust if needed.
| // When preview loses focus (user clicks editor), vscode.window.activeTextEditor is not yet | |
| // populated - it's still undefined because the editor activation happens after this event. | |
| // This is a race condition in VS Code's event ordering. The 10ms setTimeout defers our check | |
| // until the next event loop tick, allowing time for the editor to become the activeTextEditor. | |
| // Without this delay, clicking at the same cursor position wouldn't trigger onDidChangeTextEditorSelection, | |
| // and we'd miss the opportunity to sync the preview scroll position. | |
| if (!e.webviewPanel.active && this._needsScrollSync) { | |
| setTimeout(() => { | |
| const editor = vscode.window.activeTextEditor; | |
| if (editor && this._preview.isPreviewOf(editor.document.uri) && this._needsScrollSync) { | |
| const cursorLine = editor.selection.active.line; | |
| this._needsScrollSync = false; | |
| this._preview.scrollTo(cursorLine); | |
| } | |
| }, 10); // 10ms delay allows editor activation to complete | |
| } | |
| })); | |
| // When preview loses focus (user clicks editor), we set a flag to sync scroll when the editor becomes active. | |
| if (!e.webviewPanel.active && this._needsScrollSync) { | |
| // The actual scroll sync will be handled in onDidChangeActiveTextEditor. | |
| } | |
| })); | |
| this._register(vscode.window.onDidChangeActiveTextEditor(editor => { | |
| if (editor && this._preview.isPreviewOf(editor.document.uri) && this._needsScrollSync) { | |
| const cursorLine = editor.selection.active.line; | |
| this._needsScrollSync = false; | |
| this._preview.scrollTo(cursorLine); | |
| } | |
| })); |
|
@microsoft-github-policy-service agree |
|
Is there a linked issue? |
|
Superseded by #333464 |
Note: All this was vibe coded by Claude Sonnet 4.5. and GPT-5-Codex. Seems reasonable to me (typescipt/vscode codebase newbie) so I got courage to submit this pull request.
Markdown Preview Scroll Sync Fix
Issue Description
Problem: Markdown preview doesn't scroll to cursor position when clicking back in the editor after clicking a link in the preview.
Scenario:
Root Cause
Two related issues were identified:
1. Missing Scroll Sync
The preview wasn't calling
scrollTo()when the user interacted with the editor after the preview had navigated to a different document via link click.2. Race Condition (Edge Case)
When clicking at the exact same cursor position after preview navigation:
onDidChangeTextEditorSelectiondoesn't fire (selection didn't change)onDidChangeViewState:vscode.window.activeTextEditoris stillundefinedSolution
1. Added State Tracking Flag
truewhen preview navigates to different document2. Enhanced Selection Change Handler
This handles clicking at different cursor positions.
3. Added Webview Focus Handler (Race Condition Fix)
Why setTimeout(10ms)?
activeTextEditorcheck until the next event loop tickvscode.window.activeTextEditoris populatedThis handles clicking at the exact same cursor position.
4. Updated Preview Navigation
Testing Performed
Test Cases
✅ Different cursor positions (106 → 108)
onDidChangeTextEditorSelection✅ Same cursor position (106 → 106)
onDidChangeViewState+ setTimeout✅ Multiple navigations
✅ Normal editing
Related GitHub Issues
Markdown preview does not scroll to top on new URI #164071 - "Markdown preview scroll position resets when switching between editors"
markdown files preview does not save scroll position #63690 - Older related issue about preview position sync
Swapping between a file and a Markdown file causes the Markdown to scroll up each time #125964 - Another related sync issue
Technical Notes
Event Ordering in VS Code
When user clicks in editor after webview has focus:
webviewPanel.activebecomesfalse→onDidChangeViewStatefiresvscode.window.activeTextEditorisundefinedvscode.window.activeTextEditorgets populatedonDidChangeTextEditorSelectionfiresWhy Not Use Other Events?
onDidChangeActiveTextEditor- Doesn't fire when clicking in already-active editoronDidChangeTextEditorVisibleRanges- Too aggressive (fires on every scroll)onDidChangeViewStatewith setTimeout - Perfect for catching same-position clicksPerformance Considerations
setTimeout(10ms)is minimal overhead_needsScrollSyncflag is trueDebugging Log Pattern (Used During Development)
Emoji markers were used during debugging to track event flow:
Example log sequence for edge case (same position):