Fix terminal mouse wheel zoom conflicting with scroll#310769
Open
yogeshwaran-c wants to merge 1 commit intomicrosoft:mainfrom
Open
Fix terminal mouse wheel zoom conflicting with scroll#310769yogeshwaran-c wants to merge 1 commit intomicrosoft:mainfrom
yogeshwaran-c wants to merge 1 commit intomicrosoft:mainfrom
Conversation
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.
Fixes #242351
Root cause
terminal.zoom.contribution.tscribs fromeditor/browser/controller/mouseHandler.tsbut omits the single most important line of that flow: it never feeds theMouseWheelClassifierwith the incoming wheel event before queryingisPhysicalMouseWheel(). BecauseMouseWheelClassifier.INSTANCEis a shared singleton whose memory is only ever populated byAbstractScrollableElement(and byXtermTerminal's bubble-phase listener, which fires after the capture-phase zoom listener), the zoom listener consistently reads stale or empty classifier state on a fresh terminal session.With empty memory,
isPhysicalMouseWheel()returnsfalse, forcing physical-wheel Ctrl+scroll down the trackpad / inertial branch. That branch has two additional bugs:gestureAccumulatedDelta += browserEvent.deltaYis executed twice per event (once outside theif, once inside), so the accumulator grows at double the intended rate.WheelEvent.deltaY(pixel-mode, ~100 per physical tick) rather than the normalizedStandardWheelEvent.deltaY(~1 per tick) the editor uses. The/5math was tuned for the normalized value, so on physical wheels the first tick would slam the font size into the clamp.The fix
Mirror the editor's proven flow:
StandardWheelEventand callclassifier.acceptStandardWheelEvent(...)before consultingisPhysicalMouseWheel().standardEvent.deltaY(normalized, and with the editor's sign convention where positive = scroll up / zoom in).gestureAccumulatedDelta += ...inside the zoom branch.Math.round(gestureAccumulatedDelta / 5), matching the editor'sgestureStartZoomLevel + gestureAccumulatedDelta / 5.How this addresses each of the 4 reported cases
wheelDelta) is correctly classified as physical on the first event, so the+1 / -1fixed-step branch runs,preventDefaultfires, and the wheel no longer reaches xterm's viewport scroll.standardEvent.deltaY < 0(scroll down) yieldsdelta = -1, zooming out.gestureAccumulatedDeltaat a wrong sign/magnitude across the focus change and, for some users, route the event through the no-op branch. Using the normalized delta and removing the double-accumulation produces a consistent, monotonic relationship between scroll direction and font-size change, so re-zooming in after a full zoom-out works the same as a regular zoom.Test plan
"terminal.integrated.mouseWheelZoom": true.Unit tests for
clampTerminalFontSizeinterminal.zoom.test.tscontinue to pass (clamp helper is unchanged).