Skip to content

Fix terminal mouse wheel zoom conflicting with scroll#310769

Open
yogeshwaran-c wants to merge 1 commit intomicrosoft:mainfrom
yogeshwaran-c:fix/terminal-wheel-zoom-242351
Open

Fix terminal mouse wheel zoom conflicting with scroll#310769
yogeshwaran-c wants to merge 1 commit intomicrosoft:mainfrom
yogeshwaran-c:fix/terminal-wheel-zoom-242351

Conversation

@yogeshwaran-c
Copy link
Copy Markdown
Contributor

Fixes #242351

Root cause

terminal.zoom.contribution.ts cribs from editor/browser/controller/mouseHandler.ts but omits the single most important line of that flow: it never feeds the MouseWheelClassifier with the incoming wheel event before querying isPhysicalMouseWheel(). Because MouseWheelClassifier.INSTANCE is a shared singleton whose memory is only ever populated by AbstractScrollableElement (and by XtermTerminal'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() returns false, forcing physical-wheel Ctrl+scroll down the trackpad / inertial branch. That branch has two additional bugs:

  1. gestureAccumulatedDelta += browserEvent.deltaY is executed twice per event (once outside the if, once inside), so the accumulator grows at double the intended rate.
  2. It accumulates raw WheelEvent.deltaY (pixel-mode, ~100 per physical tick) rather than the normalized StandardWheelEvent.deltaY (~1 per tick) the editor uses. The /5 math 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:

  • Wrap the raw event in StandardWheelEvent and call classifier.acceptStandardWheelEvent(...) before consulting isPhysicalMouseWheel().
  • Use standardEvent.deltaY (normalized, and with the editor's sign convention where positive = scroll up / zoom in).
  • Remove the duplicated gestureAccumulatedDelta += ... inside the zoom branch.
  • Simplify the delta computation to Math.round(gestureAccumulatedDelta / 5), matching the editor's gestureStartZoomLevel + gestureAccumulatedDelta / 5.

How this addresses each of the 4 reported cases

  • Case 1 (physical wheel up at buffer bottom zooms, not scrolls): With the classifier fed, a physical tick (integer wheelDelta) is correctly classified as physical on the first event, so the +1 / -1 fixed-step branch runs, preventDefault fires, and the wheel no longer reaches xterm's viewport scroll.
  • Case 2 (physical wheel down at buffer top zooms, not scrolls): Same mechanism, opposite direction. The sign convention now matches the editor: standardEvent.deltaY < 0 (scroll down) yields delta = -1, zooming out.
  • Case 3 (continuous Ctrl+wheel cycle, already worked): Still works. Behaviour is unchanged for trackpad gestures where the modifier is held throughout; the 50 ms gesture grouping is preserved.
  • Case 4 (after zoom-out + focus change, Ctrl+wheel up does nothing): The previous code combined three problems here — double accumulation, unnormalized deltas, and stale classifier state — which in combination could leave gestureAccumulatedDelta at 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

  • Set "terminal.integrated.mouseWheelZoom": true.
  • Open a terminal, print ~20 lines, leave cursor at the bottom.
  • Ctrl+wheel-up on a physical mouse: font size increases by 1 per tick, terminal does not scroll.
  • Scroll to the top of scrollback, Ctrl+wheel-down: font size decreases by 1 per tick, terminal does not scroll.
  • Zoom out until all lines fit, release Ctrl, focus another window, refocus the terminal, Ctrl+wheel-up: font size increases.
  • Trackpad Ctrl+pinch/two-finger-scroll still produces smooth, proportional zoom without zooming during inertia after Ctrl is released.
  • Regular (no-Ctrl) wheel still scrolls the terminal buffer normally.

Unit tests for clampTerminalFontSize in terminal.zoom.test.ts continue to pass (clamp helper is unchanged).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Terminal zoom with mouse wheel conflicts with scrolling

2 participants