fix: clamp inverted visible range in ListView to prevent RangeError (fixes #333230) - #333236
Merged
Connor Peet (connor4312) merged 6 commits intoAug 31, 2026
Conversation
…ixes microsoft#333230) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
VS Code PR Bot (vscodebot-pr)
August 28, 2026 19:33
View session
Contributor
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: Benjamin Christopher Simmonds (@benibenj)Matched files:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents ListView crashes when collapsed viewports produce inverted visible ranges.
Changes:
- Reuses the calculated visible-range start.
- Clamps the range end to preserve
start <= end.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ession test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Driver cycle recordederrors-fix-driver:cycle head:61d2314e92f40910223a7165019ff720fcb55d6f |
Copilot started reviewing on behalf of
VS Code PR Bot (vscodebot-pr)
August 28, 2026 22:08
View session
Restore the accidentally-removed teardown/closing of the shift-click
selection test and rewrite the zero-height regression test so it actually
triggers the inverted range: with zero-height dynamic items and a
collapsed viewport, getVisibleRange previously produced { start: 3, end: 0 }
which crashed probeDynamicHeights.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Driver cycle recordederrors-fix-driver:cycle head:055d71cef5eba530c7fb14a37e5740c3a7edb886 |
Copilot started reviewing on behalf of
VS Code PR Bot (vscodebot-pr)
August 28, 2026 23:13
View session
…ment to one line Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Driver cycle recordederrors-fix-driver:cycle head:cb7a02b2b836eb7516cb2809aeef241e2c48caa2 |
Copilot started reviewing on behalf of
VS Code PR Bot (vscodebot-pr)
August 28, 2026 23:48
View session
Connor Peet (connor4312)
approved these changes
Aug 31, 2026
Connor Peet (connor4312)
enabled auto-merge
August 31, 2026 15:42
Contributor
|
The Screenshots & Tests (Component Fixtures) check failed only at the Determine base SHA step ( |
Connor Peet (connor4312)
approved these changes
Aug 31, 2026
roblourens
approved these changes
Aug 31, 2026
Dmitriy Vasyura (dmitrivMS)
approved these changes
Aug 31, 2026
Copilot started reviewing on behalf of
VS Code PR Bot (vscodebot-pr)
August 31, 2026 19:49
View session
Copilot started reviewing on behalf of
VS Code PR Bot (vscodebot-pr)
August 31, 2026 21:16
View session
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RangeError: Invalid array lengthis thrown inListView.probeDynamicHeights(src/vs/base/browser/ui/list/listView.ts) when it evaluatesnew Array<number>(range.end - range.start)with an inverted range (end < start). The negative length throws immediately.The inverted range originates in
getVisibleRange(renderTop, renderHeight). When the viewport is collapsed or hidden during a layout pass (renderHeight <= 0), the expressionrenderTop + renderHeight - 1becomes smaller thanrenderTop. BecauserangeMap.indexAfter(position) = min(indexAt(position) + 1, count)is monotonic inposition, the resultingendcan resolve to an index beforestart, yielding{ start, end }withend < start. This malformedIRangeviolates thestart <= endinvariant that consumers assume.This is a recent regression (new bucket in 1.135.0, 411 users): the previous
_rerenderiterated withfor (let i = range.start; i < range.end; i++), which silently tolerated an inverted range by not iterating. The batched-measurement rewrite replaced that loop with an eagerly-sized array, turning the previously-harmless inverted range into a hard crash.Fixes #333230
Recommended reviewer:
@connor4312Culprit Commit
c2b336daae7101676f179b30f92641cdfcc6b38c— "list: batch dynamic height measurements (#330967)" by Connor Peet (connor4312), 2026-08-18. This commit falls within the reported regression range (0d0c8a6...68161d9, 1.134.0-insider → 1.135.0-insider) and introducedprobeDynamicHeights, which allocatesnew Array<number>(range.end - range.start)without guaranteeingend >= start.Code Flow
flowchart TD A[layout / setScrollDimensions with renderHeight <= 0] --> B[onScroll] B --> C[_rerender] C --> D["getVisibleRange(renderTop, renderHeight)"] D --> E["end = indexAfter(renderTop + renderHeight - 1)<br/>can be < start when renderHeight ≤ 0"] E --> F["inverted IRange: end < start"] F --> G["probeDynamicHeights(range)"] G --> H["new Array(range.end - range.start)<br/>negative length"] H --> I["RangeError: Invalid array length"]Affected Files
src/vs/base/browser/ui/list/listView.ts—getVisibleRange(producer of the inverted range) andprobeDynamicHeights(crash site).Repro Steps
Not reliably reproducible via manual steps; occurs under a layout race. Conceptually:
ListViewinside a widget whose container can be laid out with zero/negative height (e.g., a collapsed chat/inline-chat widget).renderHeight <= 0.getVisibleRangereturns an inverted range;probeDynamicHeightsallocates a negative-length array and throwsRangeError: Invalid array length.How the Fix Works
Chosen approach —
src/vs/base/browser/ui/list/listView.ts,getVisibleRange: computestartfirst, then clampendwithMath.max(start, indexAfter(renderTop + renderHeight - 1)). This fixes the bug at the data producer rather than at the crash site:getVisibleRangeconstructs theIRange, so enforcing thestart <= endinvariant there guarantees every consumer (includingprobeDynamicHeights) receives a well-formed range. An empty range (start === end) is valid and yields a zero-length array, restoring the harmless behavior the previousfor-loop had.After this change,
getVisibleRangecannot produce a range withend < startbecauseendis explicitly clamped to be at leaststart, sorange.end - range.startis always>= 0andnew Array(...)can no longer receive a negative length.Alternatives considered:
probeDynamicHeights(e.g., early-return or clamp the array length there) — rejected because it patches the symptom at the bottom of the stack while leaving the malformedIRangeflowing to every other consumer; the fix must live where the invalid data is produced.Recommended Owner
connor4312(Connor Peet) — authored the culprit commitc2b336daae71and is an active repository collaborator with recent commits.