Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/System.Windows.Forms/System/Windows/Forms/Form.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4290,7 +4290,7 @@ protected virtual void OnLoad(EventArgs e)

// Also, at this time we can now locate the form on the correct area of the screen.
// We must do this after applying any autoscaling.
if (GetState(States.Modal))
if (GetState(States.Modal) || (FormStartPosition)_formState[s_formStateStartPos] == FormStartPosition.CenterParent)

@LeafShi1 LeafShi1 Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a cross-DPI scenario (e.g., parent is on a hi-DPI monitor), the sequence could be:

  1. CreateHandle() — window created on primary monitor
  2. WM_DPICHANGED fires (!Visible && CenterParent) → AdjustFormPosition()
  3. OnLoad fires (CenterParent) → AdjustFormPosition() again

This results in AdjustFormPosition() being called twice. It’s harmless (idempotent), but worth noting. Consider adding a comment here explaining why the double call is acceptable, or adding a guard to skip if already adjusted.

@LeafShi1 LeafShi1 Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously only modal forms (ShowDialog) reached this path, and ShowDialog always has an owner. Now non-modal Show() can also reach here. What happens if the user calls:

childForm.StartPosition = FormStartPosition.CenterParent;
childForm.Show();  // no owner

Does AdjustFormPosition() handle Owner == null gracefully? Could you verify this edge case doesn’t throw or produce unexpected positioning?

{
AdjustFormPosition();
}
Expand Down Expand Up @@ -4564,6 +4564,10 @@ private void WmDpiChanged(ref Message m)
DeviceDpiInternal = e.DeviceDpiNew;

OnDpiChanged(e);
if (!Visible && (FormStartPosition)_formState[s_formStateStartPos] == FormStartPosition.CenterParent)
{
AdjustFormPosition();

@LeafShi1 LeafShi1 Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The !Visible guard is correct — it prevents overriding the user’s position after the form is shown. However, the intent isn’t immediately obvious to future readers. Consider adding a comment, e.g.:

// Only re-center before the form is shown to the user. Once visible,
// the user may have repositioned it and we must not override that.

}
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2763,6 +2763,77 @@ public void Form_DoesNot_DisposeUserIcon()
smallIcon.Handle.Should().NotBe(0);
}

[WinFormsFact]
public void Form_ShowDialog_WithCenterParent_PositionsCenteredOnParent()
{
using Form parentForm = new()
{
Size = new Size(400, 400),
StartPosition = FormStartPosition.Manual,
Location = new Point(200, 200)
};

parentForm.Show();
using Form childForm = new()
{
Size = new Size(200, 200),
StartPosition = FormStartPosition.CenterParent
};

Point capturedLocation = Point.Empty;
childForm.Load += (sender, e) =>
{
capturedLocation = childForm.Location;
childForm.Close();
};

childForm.ShowDialog(parentForm);
// Expected center: parentForm center minus half of childForm size.

@LeafShi1 LeafShi1 Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

childForm.Width and childForm.Height are read after ShowDialog returns (i.e., after Close() was called inside the Load handler). If Close() triggers a WM_DPICHANGED (e.g., the form moves to a different monitor during teardown), the size could change.

Consider capturing the size inside the Load handler alongside the location:

Size capturedSize = Size.Empty;
childForm.Load += (sender, e) =>
{
    capturedLocation = childForm.Location;
    capturedSize = childForm.Size;
    childForm.Close();
};

Then use capturedSize for the expected position calculation.

int expectedX = parentForm.Left + (parentForm.Width - childForm.Width) / 2;
int expectedY = parentForm.Top + (parentForm.Height - childForm.Height) / 2;
int deltaX = SystemInformation.Border3DSize.Width + 1;
int deltaY = SystemInformation.Border3DSize.Height + 1;

capturedLocation.X.Should().BeInRange(expectedX - deltaX, expectedX + deltaX);
capturedLocation.Y.Should().BeInRange(expectedY - deltaY, expectedY + deltaY);
}

[WinFormsFact]
public void Form_Show_WithCenterParent_PositionsCenteredOnParent()
{
using Form parentForm = new()
{
Size = new Size(400, 400),
StartPosition = FormStartPosition.Manual,
Location = new Point(200, 200)
};

parentForm.Show();
using Form childForm = new()
{
Size = new Size(200, 200),
StartPosition = FormStartPosition.CenterParent
};

Point capturedLocation = Point.Empty;
childForm.Load += (sender, e) =>
{
capturedLocation = childForm.Location;
};

childForm.Show(parentForm);
childForm.Close();

// Expected center: parentForm center minus half of childForm size.
int expectedX = parentForm.Left + (parentForm.Width - childForm.Width) / 2;
int expectedY = parentForm.Top + (parentForm.Height - childForm.Height) / 2;
int deltaX = SystemInformation.Border3DSize.Width + 1;
int deltaY = SystemInformation.Border3DSize.Height + 1;

capturedLocation.X.Should().BeInRange(expectedX - deltaX, expectedX + deltaX);
capturedLocation.Y.Should().BeInRange(expectedY - deltaY, expectedY + deltaY);
}

public partial class ParentedForm : Form
{
private ParentingForm _parentForm;
Expand Down
Loading