fix: guard stale showEndForLine index in sticky scroll rendering (fixes #325642)

[vs-code-engineering]

Summary

TypeError: Cannot read properties of undefined (reading 'isVisible') thrown from ViewModelLinesFromProjectedModel.modelPositionIsVisible while the sticky scroll widget renders. The sticky scroll widget injects an undefined value into its number[] line-number array when the "show end line" index (showEndForLine) is stale relative to the current sticky-scroll state, and that undefined flows into new Position(undefined, 1) and defeats the numeric bounds guard in modelPositionIsVisible.

_findRenderingData (in stickyScrollWidget.ts) does:

const candidateLineNumbers = [...state.startLineNumbers];
if (state.showEndForLine !== null) {
	candidateLineNumbers[state.showEndForLine] = state.endLineNumbers[state.showEndForLine];
}

state.showEndForLine is an index captured earlier from a rendered DOM node (on Shift+mouse-move, _onMouseMoveOrKeyDown → _showEndForLine = currentEndForLineIndex). It is held on the controller across model changes. When the model content shrinks before the next render, findScrollWidgetState() produces shorter startLineNumbers/endLineNumbers arrays, but the stale showEndForLine index can be >= startLineNumbers.length. The assignment then writes state.endLineNumbers[showEndForLine] (which is undefined, and can also leave holes) into candidateLineNumbers, so the declared number[] now contains undefined. The existing position.lineNumber <= viewModel.getLineCount() guard only skips the height accumulation — the invalid entry still remains in the returned lineNumbers array and is later rendered.

Fixes #325642
Recommended reviewer: @aiday-mar

Culprit Commit

The showEndForLine feature was introduced in PR #236244 ("Immediately re-render sticky scroll on pressing Shift key to show end line") by @aiday-mar (merged 2024-12-16). That change added the unguarded candidateLineNumbers[state.showEndForLine] = state.endLineNumbers[state.showEndForLine] assignment. The crash surfaces through the newer getLineHeightForPosition path, but the type-contract violation originates in that assignment.

Code Flow

flowchart TD
  A[Shift+mouse-move: _onMouseMoveOrKeyDown] -->|"_showEndForLine = currentEndForLineIndex (valid now)"| B[controller state]
  C[Model content shrinks] --> D[_renderStickyScroll -> _updateState]
  D --> E["findScrollWidgetState(): shorter startLineNumbers/endLineNumbers"]
  E --> F["StickyScrollWidgetState(..., showEndForLine = stale index)"]
  F --> G["setState -> _findRenderingData"]
  G -->|"candidateLineNumbers[showEndForLine] = endLineNumbers[showEndForLine] = undefined"| H["number[] now contains undefined"]
  H --> I["_renderRootNode -> _renderChildNode -> new RenderedStickyLine"]
  I -->|"getLineHeightForPosition(new Position(undefined, 1))"| J["coordinatesConverter.modelPositionIsVisible"]
  J -->|"modelLineProjections[undefined - 1] = modelLineProjections[NaN] = undefined"| K["undefined.isVisible() -> TypeError"]

Loading

Affected Files

• src/vs/editor/contrib/stickyScroll/browser/stickyScrollWidget.ts — _findRenderingData (bypass site, line 160).

Repro Steps

1. Open a large file with sticky scroll enabled and word wrap/folding active (projected-model path).
2. Scroll so several sticky lines are shown, then hold Shift and hover a lower sticky line so showEndForLine points at a high index.
3. Trigger an edit that removes lines above/within the sticky region (or an external content change) so the sticky candidate set shrinks before the next render.
4. The stale showEndForLine index writes undefined into the rendered line list, and the next render throws Cannot read properties of undefined (reading 'isVisible').

How the Fix Works

Chosen approach (stickyScrollWidget.ts, _findRenderingData): guard the end-line substitution so it only runs when showEndForLine is a valid index into the current arrays:

if (state.showEndForLine !== null && state.showEndForLine < candidateLineNumbers.length) {
	candidateLineNumbers[state.showEndForLine] = state.endLineNumbers[state.showEndForLine];
}

startLineNumbers and endLineNumbers are always built together with equal length in findScrollWidgetState(), so showEndForLine < candidateLineNumbers.length (== startLineNumbers.length) guarantees endLineNumbers[showEndForLine] is a defined number. This fixes the bug at the data producer — the exact line that injected undefined into the number[] — rather than guarding the crash site. After this change, _findRenderingData cannot write undefined into candidateLineNumbers, because the only assignment that could do so now runs only for in-range indices, so no invalid position reaches getLineHeightForPosition/modelPositionIsVisible.

Alternatives considered:

• Add a null/bounds guard inside modelPositionIsVisible (crash site) — rejected: hides the producer bug and leaves the corrupt number[] flowing to other consumers.
• Reset _showEndForLine on every model content change in the controller — rejected: broader, easy to miss an event path, and the widget still must reconcile the index against the freshly computed arrays it receives; the assignment site is the precise producer boundary.

Recommended Owner

@aiday-mar — author of the sticky-scroll showEndForLine feature (PR #236244) and the getLineHeightForPosition change; owns this code path.

Generated by errors-fix · 1.5K AIC · ⌖ 34.3 AIC · ⊞ 70.8K · ◷

[Copilot]

Copilot can't review bot-authored pull requests automatically. A user with Copilot access can request a review manually.

[Copilot]

Copilot can't review bot-authored pull requests automatically. A user with Copilot access can request a review manually.