Fix terminal font size bounds checking in mouse wheel zoom #253010

Copilot · 2025-06-30T14:47:49Z

When using Ctrl+Mouse Wheel to adjust terminal font size, excessive scrolling could set the font size to negative or very small values, causing the terminal to become unusable. The displayed font size would remain at the minimum (6) but the underlying setting could be negative, preventing users from increasing the font size again via mouse wheel.

Root Cause

The mouse wheel zoom handlers in TerminalMouseWheelZoomContribution were updating font size values directly without applying bounds checking:

// Before - no bounds checking
this._configurationService.updateValue(TerminalSettingId.FontSize, this._getConfigFontSize() + delta);

Solution

Added bounds checking to clamp font size values between 6 and 100 (matching the terminal configuration schema):

// After - with bounds checking
const newFontSize = Math.max(6, Math.min(100, this._getConfigFontSize() + delta));
this._configurationService.updateValue(TerminalSettingId.FontSize, newFontSize);

Changes Made

Applied bounds checking to physical mouse wheel zoom handler
Applied bounds checking to gesture-based mouse wheel zoom handler
Added consistent bounds checking to manual zoom actions (FontZoomIn/FontZoomOut)
Added unit tests to verify bounds enforcement

Testing

The fix prevents the font size from going below 6 or above 100, ensuring:

Excessive scroll down stops at minimum font size (6)
Users can still scroll up to increase font size from minimum
Excessive scroll up stops at maximum font size (100)
Mouse wheel zoom functionality remains fully operational

Fixes #245768.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

electronjs.org
- Triggering command: node-gyp (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to my firewall allow list

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: meganrogge <29464607+meganrogge@users.noreply.github.com>

meganrogge · 2025-06-30T16:16:39Z

@copilot, you repeat the same code in several places - make a helper function to reduce redundancy

Co-authored-by: meganrogge <29464607+meganrogge@users.noreply.github.com>

Copilot · 2025-06-30T16:26:14Z

@copilot, you repeat the same code in several places - make a helper function to reduce redundancy

I've refactored the repetitive bounds checking logic into a dedicated clampTerminalFontSize() helper function. The Math.max(6, Math.min(100, fontSize)) pattern was repeated in 4 places and is now centralized in a single function for better maintainability. Changes applied in commit 4a766d7.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

electronjs.org
- Triggering command: node-gyp (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to my firewall allow list

Tyriar · 2025-06-30T17:24:26Z

src/vs/workbench/contrib/terminalContrib/zoom/test/browser/terminal.zoom.test.ts

+	test('font size bounds are respected when calculating new font size', () => {
+		// Test minimum bound
+		const minBoundResult = clampTerminalFontSize(3 + (-2)); // 3 - 2 = 1, clamped to 6
+		strictEqual(minBoundResult, 6, 'Font size should be clamped to minimum value of 6');
+
+		// Test maximum bound
+		const maxBoundResult = clampTerminalFontSize(99 + 5); // 99 + 5 = 104, clamped to 100
+		strictEqual(maxBoundResult, 100, 'Font size should be clamped to maximum value of 100');
+
+		// Test normal operation within bounds
+		const normalResult = clampTerminalFontSize(12 + 3); // 12 + 3 = 15, within bounds
+		strictEqual(normalResult, 15, 'Font size should remain unchanged when within bounds');
+
+		// Test edge cases
+		const exactMinResult = clampTerminalFontSize(6 + (-1)); // 6 - 1 = 5, clamped to 6
+		strictEqual(exactMinResult, 6, 'Font size should be clamped when going below minimum');
+
+		const exactMaxResult = clampTerminalFontSize(100 + 1); // 100 + 1 = 101, clamped to 100
+		strictEqual(exactMaxResult, 100, 'Font size should be clamped when going above maximum');
+	});


Instead of comments these should be split into separate test(...) cases

Split the multi-assertion test into separate test cases as requested. Each test now focuses on a single specific behavior. Commit 0a1f383.

Tyriar · 2025-06-30T17:24:44Z

src/vs/workbench/contrib/terminalContrib/zoom/test/browser/terminal.zoom.test.ts

+		// Test updating to a value below minimum (simulating excessive scroll down)
+		const currentSize = configurationService.getValue(TerminalSettingId.FontSize);
+		const newSize = clampTerminalFontSize(currentSize + (-20)); // Simulate large negative delta
+		await configurationService.updateValue(TerminalSettingId.FontSize, newSize);
+		strictEqual(configurationService.getValue(TerminalSettingId.FontSize), 6, 'Font size should be clamped to minimum');
+
+		// Test updating to a value above maximum
+		const currentSize2 = configurationService.getValue(TerminalSettingId.FontSize);
+		const newSize2 = clampTerminalFontSize(currentSize2 + 200); // Simulate large positive delta
+		await configurationService.updateValue(TerminalSettingId.FontSize, newSize2);
+		strictEqual(configurationService.getValue(TerminalSettingId.FontSize), 100, 'Font size should be clamped to maximum');


Same here, these should be separate test cases.

Split the configuration update test into separate test cases as requested. Each test now validates a single scenario independently. Commit 0a1f383.

Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com>

Copilot · 2025-06-30T17:30:57Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

electronjs.org
- Triggering command: node-gyp (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to my firewall allow list

Initial plan

fc0b939

Copilot AI assigned Copilot and meganrogge Jun 30, 2025

Copilot started work on behalf of meganrogge June 30, 2025 14:47 View session

Fix terminal font size bounds checking in mouse wheel zoom

5a97c19

Co-authored-by: meganrogge <29464607+meganrogge@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Ctrl+Mouse Wheel Font Size Adjustment Can Lead to Unusable Terminal and Prevents Further Adjustment (Related to #239161, potentially #226925, #209670)~~ Fix terminal font size bounds checking in mouse wheel zoom Jun 30, 2025

Copilot AI requested a review from meganrogge June 30, 2025 15:01

Copilot finished work on behalf of meganrogge June 30, 2025 15:01

Copilot started work on behalf of meganrogge June 30, 2025 16:16 View session

meganrogge added this to the June 2025 milestone Jun 30, 2025

Refactor font size bounds checking into helper function

4a766d7

Co-authored-by: meganrogge <29464607+meganrogge@users.noreply.github.com>

Copilot finished work on behalf of meganrogge June 30, 2025 16:26

improve things

3d01673

meganrogge approved these changes Jun 30, 2025

View reviewed changes

meganrogge requested a review from Tyriar June 30, 2025 16:53

Tyriar requested changes Jun 30, 2025

View reviewed changes

Copilot started work on behalf of Tyriar June 30, 2025 17:24 View session

Split terminal zoom tests into separate test cases

0a1f383

Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com>

Copilot AI requested a review from Tyriar June 30, 2025 17:31

Copilot finished work on behalf of Tyriar June 30, 2025 17:31

Tyriar previously approved these changes Jun 30, 2025

View reviewed changes

rm failing tests

a864811

meganrogge dismissed Tyriar’s stale review via a864811 June 30, 2025 17:42

meganrogge approved these changes Jun 30, 2025

View reviewed changes

meganrogge marked this pull request as ready for review June 30, 2025 17:42

meganrogge enabled auto-merge (squash) June 30, 2025 17:42

simplify

7fdc1ef

meganrogge approved these changes Jun 30, 2025

View reviewed changes

Tyriar approved these changes Jul 1, 2025

View reviewed changes

meganrogge merged commit 53133e6 into main Jul 1, 2025
8 checks passed

meganrogge deleted the copilot/fix-245768 branch July 1, 2025 16:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix terminal font size bounds checking in mouse wheel zoom #253010

Fix terminal font size bounds checking in mouse wheel zoom #253010

Uh oh!

Copilot AI commented Jun 30, 2025 •

edited

Loading

Uh oh!

meganrogge commented Jun 30, 2025

Uh oh!

Copilot AI commented Jun 30, 2025 •

edited

Loading

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Tyriar Jun 30, 2025

Uh oh!

Copilot AI Jun 30, 2025

Uh oh!

Tyriar Jun 30, 2025

Uh oh!

Copilot AI Jun 30, 2025

Uh oh!

Copilot AI commented Jun 30, 2025

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Uh oh!

Fix terminal font size bounds checking in mouse wheel zoom #253010

Fix terminal font size bounds checking in mouse wheel zoom #253010

Uh oh!

Conversation

Copilot AI commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Root Cause

Solution

Changes Made

Testing

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

meganrogge commented Jun 30, 2025

Uh oh!

Copilot AI commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Tyriar Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

Tyriar Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jun 30, 2025

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Jun 30, 2025 •

edited

Loading

Copilot AI commented Jun 30, 2025 •

edited

Loading