diff --git a/src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripDropDownMenu.cs b/src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripDropDownMenu.cs
index d4f967fecbb..f50aad7ee04 100644
--- a/src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripDropDownMenu.cs
+++ b/src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripDropDownMenu.cs
@@ -464,8 +464,10 @@ internal override void ChangeSelection(ToolStripItem? nextItem)
if (nextItem is not null)
{
Rectangle displayRect = DisplayRectangle;
- if (!displayRect.Contains(displayRect.X, nextItem.Bounds.Top)
- || !displayRect.Contains(displayRect.X, nextItem.Bounds.Bottom))
+ // Use IsItemFullyVisible so that an item whose top or bottom edge lies exactly
+ // on the display boundary is not treated as out-of-view, preventing an
+ // unnecessary scroll when the item is already fully shown.
+ if (!IsItemFullyVisible(displayRect, nextItem.Bounds))
{
int delta;
if (displayRect.Y > nextItem.Bounds.Top)
@@ -479,8 +481,15 @@ internal override void ChangeSelection(ToolStripItem? nextItem)
int index = Items.IndexOf(nextItem);
while (index >= 0)
{
+ // Adjust the item's bounds by the prospective scroll delta so we can
+ // test whether it would still overlap the display area after scrolling.
+ // Using intersection (not full-containment) so that partially-visible
+ // items are correctly identified as still being in view.
+ Rectangle adjustedItemBounds = Items[index].Bounds;
+ adjustedItemBounds.Y -= delta;
+
// we need to roll back to the index which is visible
- if ((Items[index].Visible && displayRect.Contains(displayRect.X, Items[index].Bounds.Top - delta))
+ if ((Items[index].Visible && IsItemIntersectingDisplayRectangle(displayRect, adjustedItemBounds))
|| !Items[index].Visible)
{
--index;
@@ -493,10 +502,15 @@ internal override void ChangeSelection(ToolStripItem? nextItem)
if (index >= 0)
{
- if (displayRect.Contains(displayRect.X, Items[index].Bounds.Bottom - delta))
+ // Apply the same delta-adjusted bounds used in the loop above so the
+ // truncation check is consistent with the visibility check.
+ Rectangle adjustedItemBounds = Items[index].Bounds;
+ adjustedItemBounds.Y -= delta;
+
+ if (adjustedItemBounds.Bottom > displayRect.Top && adjustedItemBounds.Bottom <= displayRect.Bottom)
{
// We found an item which is truncated at the top.
- delta += (Items[index].Bounds.Bottom - delta) - displayRect.Top;
+ delta += adjustedItemBounds.Bottom - displayRect.Top;
}
}
}
@@ -662,8 +676,10 @@ internal void RestoreScrollPosition()
{
Rectangle adjustedLastItemBounds = Items[Items.Count - 1].Bounds;
adjustedLastItemBounds.Y -= deltaToScroll;
- if (displayRectangle.Contains(displayRectangle.X, adjustedLastItemBounds.Top)
- && displayRectangle.Contains(displayRectangle.X, adjustedLastItemBounds.Bottom))
+ // Use IsItemFullyVisible (top >= display.Top && bottom <= display.Bottom) so
+ // that we stop accumulating scroll distance as soon as the last item would be
+ // completely inside the viewport — not just partially touching the edge.
+ if (IsItemFullyVisible(displayRectangle, adjustedLastItemBounds))
{
// Scrolling this amount would make the last item visible, so don't scroll any more.
break;
@@ -689,8 +705,7 @@ internal void RestoreScrollPosition()
{
Rectangle adjustedLastItemBounds = Items[0].Bounds;
adjustedLastItemBounds.Y -= deltaToScroll;
- if (displayRectangle.Contains(displayRectangle.X, adjustedLastItemBounds.Top)
- && displayRectangle.Contains(displayRectangle.X, adjustedLastItemBounds.Bottom))
+ if (IsItemFullyVisible(displayRectangle, adjustedLastItemBounds))
{
// Scrolling this amount would make the last item visible, so don't scroll any more.
break;
@@ -722,8 +737,18 @@ internal override void ScrollInternal(int delta)
internal void ScrollInternal(bool up)
{
+ // Refresh button state first so _indexOfFirstDisplayedItem is current.
UpdateScrollButtonStatus();
+ // If the corresponding scroll button is already disabled, there is
+ // nothing more to scroll. This prevents the out-of-range access that
+ // would otherwise occur when the last item is already at the top of the
+ // display area and the down-scroll path tries to read Items[index + 1].
+ if ((up && !UpScrollButton.Enabled) || (!up && !DownScrollButton.Enabled))
+ {
+ return;
+ }
+
// calling this to get ScrollWindowEx. In actuality it does nothing
// to change the display rect!
int delta;
@@ -753,9 +778,9 @@ internal void ScrollInternal(bool up)
}
else
{
- if (_indexOfFirstDisplayedItem == Items.Count - 1)
+ if (_indexOfFirstDisplayedItem >= Items.Count - 1)
{
- Debug.Fail("We're trying to scroll down, but the top item is displayed!!!");
+ return;
}
ToolStripItem itemTop = Items[_indexOfFirstDisplayedItem];
@@ -810,8 +835,13 @@ private void UpdateScrollButtonStatus()
{
Rectangle displayRectangle = DisplayRectangle;
+ // Track the first and last items that intersect the viewport.
_indexOfFirstDisplayedItem = -1;
- int minY = int.MaxValue, maxY = 0;
+ int indexOfLastDisplayedItem = -1;
+
+ // Track the first and last available (non-scroll-button, non-hidden) items.
+ int firstAvailableItemIndex = -1;
+ int lastAvailableItemIndex = -1;
for (int i = 0; i < Items.Count; ++i)
{
@@ -831,16 +861,48 @@ private void UpdateScrollButtonStatus()
continue;
}
- if (_indexOfFirstDisplayedItem == -1 && displayRectangle.Contains(displayRectangle.X, item.Bounds.Top))
+ firstAvailableItemIndex = firstAvailableItemIndex == -1 ? i : firstAvailableItemIndex;
+ lastAvailableItemIndex = i;
+
+ // An item is "displayed" when it intersects the viewport (even partially),
+ // so the first such item is a reliable scroll anchor regardless of height.
+ if (IsItemIntersectingDisplayRectangle(displayRectangle, item.Bounds))
{
- _indexOfFirstDisplayedItem = i;
+ if (_indexOfFirstDisplayedItem == -1)
+ {
+ _indexOfFirstDisplayedItem = i;
+ }
+
+ indexOfLastDisplayedItem = i;
}
+ }
- minY = Math.Min(minY, item.Bounds.Top);
- maxY = Math.Max(maxY, item.Bounds.Bottom);
+ if (_indexOfFirstDisplayedItem == -1 || firstAvailableItemIndex == -1)
+ {
+ // No available items are visible at all — disable both buttons.
+ UpScrollButton.Enabled = false;
+ DownScrollButton.Enabled = false;
+ return;
}
- UpScrollButton.Enabled = !displayRectangle.Contains(displayRectangle.X, minY);
- DownScrollButton.Enabled = !displayRectangle.Contains(displayRectangle.X, maxY);
+ // Up is enabled only when the first visible item is not already the first available item.
+ // Down is enabled only when there is at least one available item below the last displayed item.
+ // This ensures both buttons are disabled once there is no further content to scroll to.
+ UpScrollButton.Enabled = _indexOfFirstDisplayedItem > firstAvailableItemIndex;
+ DownScrollButton.Enabled = indexOfLastDisplayedItem < lastAvailableItemIndex;
}
+
+ ///
+ /// Returns when is entirely
+ /// contained within (no clipping on either edge).
+ ///
+ private static bool IsItemFullyVisible(Rectangle displayRectangle, Rectangle itemBounds)
+ => itemBounds.Top >= displayRectangle.Top && itemBounds.Bottom <= displayRectangle.Bottom;
+
+ ///
+ /// Returns when overlaps
+ /// by at least one pixel (partial visibility counts).
+ ///
+ private static bool IsItemIntersectingDisplayRectangle(Rectangle displayRectangle, Rectangle itemBounds)
+ => itemBounds.Bottom > displayRectangle.Top && itemBounds.Top < displayRectangle.Bottom;
}
diff --git a/src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripScrollButton.cs b/src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripScrollButton.cs
index 67d8fdb7423..aa20cd4132d 100644
--- a/src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripScrollButton.cs
+++ b/src/System.Windows.Forms/System/Windows/Forms/Controls/ToolStrips/ToolStripScrollButton.cs
@@ -32,6 +32,11 @@ public ToolStripScrollButton(bool up)
stickyLabel.OwnerScrollButton = this;
}
+ // Scroll buttons are navigation affordances, not actionable menu items.
+ // Suppress item-click routing so ToolStripDropDown.AutoClose doesn't dismiss
+ // the dropdown when the user scrolls to (or at) the boundary.
+ SupportsItemClick = false;
+
_up = up;
}
diff --git a/src/test/unit/System.Windows.Forms/ToolStripDropDownMenuTests.cs b/src/test/unit/System.Windows.Forms/ToolStripDropDownMenuTests.cs
index 1a99157ad9f..b18eb5e449e 100644
--- a/src/test/unit/System.Windows.Forms/ToolStripDropDownMenuTests.cs
+++ b/src/test/unit/System.Windows.Forms/ToolStripDropDownMenuTests.cs
@@ -3,6 +3,8 @@
#nullable disable
+using System.Drawing;
+
namespace System.Windows.Forms.Tests;
public class ToolStripDropDownMenuTests
@@ -12,7 +14,7 @@ public void ToolStripDropDownMenu_Constructor()
{
using ToolStripDropDownMenu menu = new();
- Assert.NotNull(menu);
+ menu.Should().NotBeNull();
}
[WinFormsFact]
@@ -23,8 +25,126 @@ public void ToolStripDropDownMenu_ConstructorOwnerItemBool()
using ToolStripDropDownMenu menu = new(owner, isAutoGenerated);
- Assert.NotNull(menu);
- Assert.Equal(owner, menu.OwnerItem);
- Assert.True(menu.IsAutoGenerated);
+ menu.Should().NotBeNull();
+ menu.OwnerItem.Should().Be(owner);
+ menu.IsAutoGenerated.Should().BeTrue();
+ }
+
+ [WinFormsFact]
+ public void ToolStripDropDownMenu_ScrollInternalDown_LastItemDisplayed_DisablesDownScrollButton()
+ {
+ using ToolStripButton owner = new();
+ using SubToolStripDropDownMenu menu = new(owner, isAutoGenerated: true)
+ {
+ // Use a full-item viewport to make the boundary condition deterministic:
+ // first item is fully above the viewport, last item is fully visible.
+ TestDisplayRectangle = new Rectangle(0, 20, 100, 20)
+ };
+
+ ToolStripItem firstItem = menu.Items.Add("First");
+ ToolStripItem lastItem = menu.Items.Add("Last");
+
+ // Enabling scroll buttons triggers internal position restoration logic.
+ // Do this first, then set explicit bounds for deterministic visibility checks.
+ menu.SetRequiresScrollButtons(true);
+
+ firstItem.SetBounds(new Rectangle(0, 0, 80, 20));
+ lastItem.SetBounds(new Rectangle(0, 20, 80, 20));
+
+ // The real scroll-button pipeline should leave the down button disabled
+ // when the last item is already visible, and a subsequent click should be a no-op.
+ Record.Exception(menu.ScrollDown).Should().BeNull();
+ menu.DownScrollButton.Enabled.Should().BeFalse();
+ }
+
+ [WinFormsFact]
+ public void ToolStripDropDownMenu_ScrollInternalDown_LastItemVisibleInMultiItemViewport_DisablesDownScrollButton()
+ {
+ using ToolStripButton owner = new();
+ using SubToolStripDropDownMenu menu = new(owner, isAutoGenerated: true)
+ {
+ // Make room for multiple full items so the last item can be visible
+ // while the first displayed item is not the last index.
+ TestDisplayRectangle = new Rectangle(0, 20, 100, 40)
+ };
+
+ ToolStripItem firstItem = menu.Items.Add("First");
+ ToolStripItem middleItem = menu.Items.Add("Middle");
+ ToolStripItem lastItem = menu.Items.Add("Last");
+
+ // Enabling scroll buttons triggers internal position restoration logic.
+ // Do this first, then set explicit bounds for deterministic visibility checks.
+ menu.SetRequiresScrollButtons(true);
+ firstItem.SetBounds(new Rectangle(0, 0, 80, 20));
+ middleItem.SetBounds(new Rectangle(0, 20, 80, 20));
+ lastItem.SetBounds(new Rectangle(0, 40, 80, 20));
+
+ // At this boundary the middle and last items are visible, so scrolling down
+ // should already be disabled even though the first displayed item is not last.
+ Record.Exception(menu.ScrollDown).Should().BeNull();
+ menu.DownScrollButton.Enabled.Should().BeFalse();
+ }
+
+ [WinFormsFact]
+ public void ToolStripDropDownMenu_ChangeSelection_ItemAtDisplayBottom_DoesNotScroll()
+ {
+ using ToolStripButton owner = new();
+ using SubToolStripDropDownMenu menu = new(owner, isAutoGenerated: true)
+ {
+ TestDisplayRectangle = new Rectangle(0, 0, 100, 40)
+ };
+
+ ToolStripItem firstItem = menu.Items.Add("First");
+ ToolStripItem secondItem = menu.Items.Add("Second");
+
+ firstItem.SetBounds(new Rectangle(0, 0, 80, 20));
+ secondItem.SetBounds(new Rectangle(0, 20, 80, 20));
+
+ menu.InvokeChangeSelection(secondItem);
+
+ menu.ScrollCalled.Should().BeFalse();
+ }
+
+ [WinFormsFact]
+ public void ToolStripDropDownMenu_ScrollButtons_DoNotSupportItemClick()
+ {
+ using ToolStripButton owner = new();
+ using SubToolStripDropDownMenu menu = new(owner, isAutoGenerated: true)
+ {
+ TestDisplayRectangle = new Rectangle(0, 0, 100, 40)
+ };
+
+ _ = menu.UpScrollButton;
+ _ = menu.DownScrollButton;
+
+ // Scroll buttons are navigation UI and must not route ItemClick, otherwise
+ // ToolStripDropDown.AutoClose can dismiss the dropdown while scrolling.
+ menu.UpScrollButton.SupportsItemClick.Should().BeFalse();
+ menu.DownScrollButton.SupportsItemClick.Should().BeFalse();
+ }
+
+ private class SubToolStripDropDownMenu : ToolStripDropDownMenu
+ {
+ internal SubToolStripDropDownMenu(ToolStripItem ownerItem, bool isAutoGenerated)
+ : base(ownerItem, isAutoGenerated)
+ {
+ }
+
+ internal Rectangle TestDisplayRectangle { get; set; }
+ internal bool ScrollCalled { get; private set; }
+
+ public override Rectangle DisplayRectangle => TestDisplayRectangle;
+
+ internal void SetRequiresScrollButtons(bool value) => base.RequiresScrollButtons = value;
+
+ internal override void ScrollInternal(int delta)
+ {
+ ScrollCalled = true;
+ base.ScrollInternal(delta);
+ }
+
+ internal void InvokeChangeSelection(ToolStripItem item) => base.ChangeSelection(item);
+
+ internal void ScrollDown() => ScrollInternal(up: false);
}
}