Fix issue 14795: ComboBox with DropDownStyle.Simple renders incorrectly and displays an unexpected vertical scrollbar - #14827
Conversation
…ly and displays an unexpected vertical scrollbar
There was a problem hiding this comment.
Pull request overview
This PR addresses WinForms issue #14795 by correcting the Net11 modern-visual-styles rendering/layout of ComboBoxStyle.Simple, eliminating the visual interference that produced an unexpected vertical scrollbar strip and improving edit/list geometry consistency.
Changes:
- Adjusts modern Simple-mode layout to stabilize edit/list sizing, remove list borders, and clip the list bottom to preserve the rounded outer frame.
- Adds an accent-colored divider between the edit area and the list area during modern painting.
- Adds regression tests covering Simple-mode edit/list bounds, divider rendering, list border removal, and CreateParams flags.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/test/unit/System.Windows.Forms/System/Windows/Forms/ComboBoxTests.cs | Adds regression tests for modern Net11 + Simple layout, divider rendering, and style flags. |
| src/System.Windows.Forms/System/Windows/Forms/Controls/ComboBox/ComboBox.ModernComboAdapter.cs | Draws an accent divider between the Simple edit and list areas during modern rendering. |
| src/System.Windows.Forms/System/Windows/Forms/Controls/ComboBox/ComboBox.Modern.cs | Reworks modern Simple-mode geometry; removes list borders; applies a clip region to avoid rounding artifacts. |
| src/System.Windows.Forms/System/Windows/Forms/Controls/ComboBox/ComboBox.cs | Tweaks CreateParams (integral height for modern Simple) and applies list border adjustments on handle creation; modernizes NCCALCSIZE/NCHITTEST behavior for Simple. |
Comments suppressed due to low confidence (2)
src/test/unit/System.Windows.Forms/System/Windows/Forms/ComboBoxTests.cs:1315
- These assertions only check one-sided bounds (<= / >=), which can let significant misalignment slip through. Using InRange with a small tolerance makes the test better at catching regressions while still allowing minor off-by-one differences.
Assert.True(listBounds.Left <= control.ModernChromeInsets.Left);
Assert.True(listBounds.Right >= control.ClientRectangle.Right - control.ModernChromeInsets.Right - 1);
src/System.Windows.Forms/System/Windows/Forms/Controls/ComboBox/ComboBox.Modern.cs:610
- The value "2" is a magic number used to crop the visible list area. Giving it a named constant (even a local const) makes it clearer what the crop represents and helps keep related calculations consistent if it needs adjustment later.
int bottomShrink = ScaleHelper.ScaleToDpi(2, DeviceDpiInternal);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (isModernSimpleList) | ||
| { | ||
| ConfigureModernSimpleListClipRegion( |
There was a problem hiding this comment.
The clip region is re-applied on every layout pass. ConfigureModernSimpleListClipRegion runs unconditionally in ApplyChildBounds for the modern Simple list, even when the target bounds are unchanged. While the SetWindowPos call is guarded against no-op moves, this SetWindowRgn(..., fRedraw: true) is not, so every relayout forces a region reset and repaint. Consider skipping the region update when the computed clip rect matches the previously applied one to avoid unnecessary work and potential flicker.
| return; | ||
| } | ||
|
|
||
| int bottomShrink = ScaleHelper.ScaleToDpi(2, DeviceDpiInternal); |
There was a problem hiding this comment.
Possible double bottom-crop. The Simple layout already reserves simpleBottomShrink = ScaleToDpi(bottomCropLogicalPixels) (plus dividerThickness) when computing simpleListBottom (line ~187-188), and here the clip region shrinks the visible height by an additional ScaleToDpi(2). If both are intentional the total reserved space at the bottom is larger than it may appear from either site alone. Could you confirm this is deliberate, and ideally derive both from a single shared constant so they can't drift apart?
|
|
||
| if (hasComboBoxInfo) | ||
| { | ||
| ConfigureModernSimpleListSurface(comboBoxInfo.hwndList); |
There was a problem hiding this comment.
ConfigureModernSimpleListSurface strips WS_BORDER / WS_EX_CLIENTEDGE from the list only once, at handle creation. For a persistently visible Simple list these styles can be re-applied by later theme changes, DPI changes, or a SetWindowTheme/settings-change round-trip, after which the border could reappear. Consider re-running this normalization on the relevant change notifications (e.g. WM_THEMECHANGED / DPI-changed) rather than only in OnHandleCreated.
| case PInvokeCore.WM_NCCALCSIZE: | ||
| if (UsesModernComboAdapter | ||
| && DropDownStyle != ComboBoxStyle.Simple | ||
| && m.WParamInternal != 0u) |
There was a problem hiding this comment.
This now routes WM_NCCALCSIZE (and WM_NCHITTEST below) through the modern non-client handling for DropDownStyle.Simple as well, whereas Simple was previously excluded. Since the Simple list is an always-visible child with its own scrollbar, please double-check that expanding/adjusting the client area here does not interfere with the list's vertical scrollbar hit-testing and dragging, mouse wheel, and keyboard navigation. Worth an explicit manual test on the Simple list with more items than fit.
| if (!_integralHeight) | ||
| if (!_integralHeight | ||
| || (UsesModernComboAdapter | ||
| && DropDownStyle == ComboBoxStyle.Simple)) |
There was a problem hiding this comment.
Forcing CBS_NOINTEGRALHEIGHT for the modern Simple combo silently overrides an explicit IntegralHeight = true set by the user. IntegralHeight is a public property, so a consumer setting it to true will now see it ignored with no feedback. Consider only forcing this style when IntegralHeight is at its default (or documenting that modern Simple mode does not honor IntegralHeight), so the public contract isn't silently broken.
|
|
||
| Rectangle listBounds = control.GetListBounds(); | ||
| int sampleX = Math.Clamp(control.Width / 2, 0, control.Width - 1); | ||
| int startY = Math.Max(0, listBounds.Top - 4); |
There was a problem hiding this comment.
This divider assertion samples the band [Top-4, Top-1] while the divider is drawn at Top - dividerThickness. At higher DPI dividerThickness scales up and the accent line can fall above Top-4, or the fixed 4px window can miss it, making this test DPI-fragile. Consider deriving the sampled band from the same dividerThickness/DPI value used by the production code instead of the hard-coded -4/-1 offsets.
Fixes #14795
Root Cause
When
VisualStylesModeis set toNet11,ComboBoxStyle.Simpleretains certain "classic/native" geometric and non-client area behaviors, resulting in inconsistencies when overlaid with modern rendering:Proposed changes
Customer Impact
Regression?
Risk
Screenshots
Before
After
Normal
DarkMode
Test methodology
Test environment(s)
Microsoft Reviewers: Open in CodeFlow