LT-21720: Give unpositioned dialogs centered start positions and owners

[jasonleenaylor]

Quick Summary

Partially addresses LT-21720 (remembered window locations misbehaving on multi-monitor setups).

• Revive UploadToWebonaryDlg position persistence: its restore/clamp/save code null-checked a PropertyTable auto-property that the constructor never assigned, so it has been dead since it was written. The property is now a constructor-assigned field, and the dialog is shown with the main window as owner. Per review feedback, OnShown now recenters the dialog on the owner's screen when the remembered position is on a different monitor, so a modal dialog is never stranded where the user isn't looking.
• Give DictionaryConfigurationManagerDlg a CenterParent start position and clamp its bounds back onto a screen in OnShown — it previously had no positioning code and could open off-screen, leaving the app looking hung (one of the LT-21720 reports).
• Sweep the highest-risk dialogs that had no positioning code at all: CenterParent on ten (dictionary-configuration child dialogs, Webonary log viewer, the Gecko-hosting MGA/diagnostics/import-marker dialogs, semantic-domains chooser, respeller, concordance dialog) and CenterScreen on five that can be shown before any main window exists (the four FdoUi restore dialogs and the update chooser).
• Pass an explicit owner at the six ShowDialog() call sites that had none, threading an owner parameter through DictionaryConfigurationImportController.DisplayView.

This PR does not cover the dialogs that persist and restore positions of their own (BaseGoDlg and its subclasses, ReallySimpleListChooser, InsertEntryDlg's registry restore, FwFindReplaceDlg, the shared Persistence helper, and similar) — several of the dialogs named in LT-21720 are in that family, which is follow-up work.

CI-ready checklist

• Commit messages follow .github/commit-guidelines.md (subject ≤ 72 chars, no trailing punctuation; if body present, blank line then ≤ 80-char lines).
• No whitespace warnings locally:

  git fetch origin
  git log --check --pretty=format:"---% h% s" origin/<base>..
  git diff --check --cached

• Builds/tests pass locally (or I've run the CI-style build via Bash script or MSBuild).
• If this is core-developer AI-assisted work, I followed Docs/workflows/ai-pr-workflow.md and ran pr-preflight or the equivalent branch-readiness review before requesting review.
• For any Src/** folders touched, corresponding AGENTS.md files are updated or explicitly confirmed still accurate.

Notes for reviewers (optional)

WinForms' default WindowsDefaultLocation leaves placement to the OS cascade, which on multi-monitor setups can land a dialog on the wrong screen or entirely off the visible area. Centering on an owner — or on a screen, for dialogs that can appear before any window exists — takes the OS out of the placement decision. The swept dialogs are behavior-neutral one-liners except DisplayView, whose new owner parameter has exactly one caller (updated in the same commit).

Validation: build.ps1 -SkipNative clean; test.ps1 on xWorksTests filtered to UploadToWebonary + DictionaryConfiguration, all passing after rebasing onto current main.

🤖 Generated with Claude Code

---

This change is 

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 51 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.30%. Comparing base (1ada9f6) to head (5815ecc).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
Src/xWorks/UploadToWebonaryDlg.cs	0.00%	17 Missing and 4 partials ⚠️
Src/xWorks/DictionaryConfigurationManagerDlg.cs	0.00%	7 Missing and 1 partial ⚠️
.../DetailControls/SemanticDomainReferenceLauncher.cs	0.00%	1 Missing ⚠️
.../Controls/DetailControls/SemanticDomainsChooser.cs	0.00%	1 Missing ⚠️
Src/Common/FwUtils/FwUpdateChooserDlg.cs	0.00%	1 Missing ⚠️
...ialogs/CantRestoreLinkedFilesToOriginalLocation.cs	0.00%	1 Missing ⚠️
Src/FdoUi/Dialogs/ConflictingSaveDlg.cs	0.00%	1 Missing ⚠️
Src/FdoUi/Dialogs/FilesToRestoreAreOlder.cs	0.00%	1 Missing ⚠️
...doUi/Dialogs/RestoreLinkedFilesToProjectsFolder.cs	0.00%	1 Missing ⚠️
Src/LexText/LexTextControls/InsertEntryDlg.cs	0.00%	0 Missing and 1 partial ⚠️
... and 14 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1053      +/-   ##
==========================================
+ Coverage   36.44%   40.30%   +3.86%     
==========================================
  Files        1354     1542     +188     
  Lines      295692   364888   +69196     
  Branches    40228    40229       +1     
==========================================
+ Hits       107752   147055   +39303     
- Misses     158660   188553   +29893     
  Partials    29280    29280              

Files with missing lines	Coverage Δ
.../DetailControls/SemanticDomainReferenceLauncher.cs	9.67% <0.00%> (ø)
.../Controls/DetailControls/SemanticDomainsChooser.cs	0.00% <0.00%> (ø)
Src/Common/FwUtils/FwUpdateChooserDlg.cs	0.00% <0.00%> (ø)
...ialogs/CantRestoreLinkedFilesToOriginalLocation.cs	0.00% <0.00%> (ø)
Src/FdoUi/Dialogs/ConflictingSaveDlg.cs	0.00% <0.00%> (ø)
Src/FdoUi/Dialogs/FilesToRestoreAreOlder.cs	0.00% <0.00%> (ø)
...doUi/Dialogs/RestoreLinkedFilesToProjectsFolder.cs	0.00% <0.00%> (ø)
Src/LexText/LexTextControls/InsertEntryDlg.cs	0.00% <0.00%> (ø)
...c/LexText/LexTextControls/LexImportWizardMarker.cs	0.00% <0.00%> (ø)
Src/LexText/Morphology/ConcordanceDlg.cs	0.00% <0.00%> (ø)
... and 14 more

... and 195 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   10m 59s ⏱️ -34s
5 761 tests ±0  5 680 ✅ ±0  81 💤 ±0  0 ❌ ±0 
5 770 runs  ±0  5 689 ✅ ±0  81 💤 ±0  0 ❌ ±0 

Results for commit 5815ecc. ± Comparison against base commit 3c9a28a.

♻️ This comment has been updated with latest results.

[mark-sil]

@mark-sil reviewed 24 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on jasonleenaylor).

[thejambi]

@thejambi reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on jasonleenaylor).

[thejambi]

Src/xWorks/UploadToWebonaryDlg.cs line 352 at r1 (raw file):

			}
			base.OnClosing(e);
		}

Based on my review of the changes, against some of the notes I had gathered about it -- this one especially should be verified, it may not handle multi-monitor setup well.

[thejambi]

Some Claude feedback based on my notes when looking into the issue so far. I wonder if yours would agree with this assessment?

---

Head: 36daa0d (claude/webonary-dictconfig-dialog-position). Line numbers below are at PR head.

---

1. Top-level review comment (post as the review summary)

The CenterParent / CenterScreen sweep and the owner-threading look right to me, and the
mechanism is the correct one: Form.CenterToParent() reads GWL_HWNDPARENT, centers on the
owner's rectangle and clamps to that screen's working area, so these dialogs will now land on
the monitor the main window is on. CenterScreen for the ownerless FdoUi/update dialogs is the
right call too, since it centers on the screen under the mouse rather than always the primary.
GetValue<IWin32Window>("window") matches existing usage in DictionaryConfigurationListener
and XhtmlDocView, and DisplayView(dialog, owner) has only the one caller in the repo.

Two things I'd like to sort out before this merges:

1. The revived Webonary persistence re-introduces the wrong-monitor bug (see inline comment on
UploadToWebonaryDlg.cs). That restore block was previously dead code, because PropertyTable
was never assigned. Now that it runs, it sets StartPosition = Manual from a remembered rect that
is only clamped by ScreenHelper.EnsureVisibleRect — which guarantees the dialog is on some
screen, not on the owner's screen — and that assignment overrides the owner we now pass from
FwXWindow. So this is the one dialog in the PR that can still come back on the wrong monitor,
and it's this PR that turns that path on.

2. Scope vs. title. The title reads as a general fix, but the dialogs that actually persist
and restore a position are untouched: BaseGoDlg.SetDlgInfo (and its ~10 subclasses — EntryGoDlg,
LinkEntryOrSenseDlg, LinkAllomorphDlg, RecordGoDlg, WordformGoDlg, ReversalEntryGoDlg, …),
ReallySimpleListChooser (which gets no clamp at all when constructed with a null
IPersistenceProvider, e.g. from EntrySequenceReferenceLauncher), InsertEntryDlg's own
registry restore, FwFindReplaceDlg, and the shared Persistence helper. Those are the cases in
the multi-monitor bug report I filed, and they're not covered here. Same pattern also lives in
MergeObjectDlg, AddNewSenseDlg, MasterCategoryListDlg, MasterListDlg, MsaCreatorDlg,
PhonologicalFeatureChooserDlg, MsaInflectionFeatureListDlg, ExportDialog,
XmlDocConfigureDlg and DictionaryConfigurationDlg.

I'm happy for that to stay out of this PR — but could we either narrow the title (e.g. "Give
unpositioned dialogs a centered start position and owners") or note in the description that the
position-persistence family is follow-up work? Otherwise the ticket looks closed when the reported
repro still reproduces.

Suggestion for the follow-up: a single shared helper (ScreenHelper or a new
DialogPositionHelper) that all the restore sites can call, rather than per-dialog OnShown
overrides — something like:

/// <summary>
/// If a remembered position placed the dialog on a different monitor than its owner (e.g. the
/// main window has since moved to another screen), recentre it on the owner's screen.
/// </summary>
public static void EnsureOnOwnerScreen(Form dlg)
{
    var owner = dlg.Owner ?? Form.ActiveForm;
    if (owner == null || dlg.StartPosition != FormStartPosition.Manual)
        return;
    var ownerScreen = Screen.FromControl(owner);
    if (ownerScreen.DeviceName == Screen.FromRectangle(dlg.DesktopBounds).DeviceName)
        return;   // remembered position is already on the owner's monitor — leave it alone
    var wa = ownerScreen.WorkingArea;
    dlg.Location = new Point(
        wa.Left + Math.Max(0, (wa.Width - dlg.Width) / 2),
        wa.Top + Math.Max(0, (wa.Height - dlg.Height) / 2));
}

It has to run where Owner is populated (OnShown), not in SetDlgInfo/the constructor —
ShowDialog(owner) assigns the owner after those have run.

---

2. Inline — Src/xWorks/UploadToWebonaryDlg.cs, lines 68–78 (the restore block)

This is the one change in the PR that can still put a dialog on the wrong monitor, and it's newly
live: with PropertyTable never assigned, this whole block was dead, so the dialog always got
default owner-relative placement. Now it restores a remembered rect and sets
StartPosition = Manual, which overrides the owner you just started passing in FwXWindow.

ScreenHelper.EnsureVisibleRect only guarantees the rect intersects some monitor's working
area. A rect entirely inside monitor A is already "visible", so it stays on monitor A even after
the user has moved FLEx to monitor B — and because the dialog is modal, the main window is
disabled and FLEx looks frozen on the monitor the user is actually looking at.

Could we add an owner-screen correction in OnShown (where Owner is finally populated)? e.g.

protected override void OnShown(EventArgs e)
{
    base.OnShown(e);
    // A remembered position may be on a monitor the main window has since left; recentre on the
    // owner's screen so the dialog isn't stranded off-view behind a modal state.
    var owner = Owner ?? Form.ActiveForm;
    if (owner == null || StartPosition != FormStartPosition.Manual)
        return;
    var ownerScreen = Screen.FromControl(owner);
    if (ownerScreen.DeviceName == Screen.FromRectangle(DesktopBounds).DeviceName)
        return;
    var wa = ownerScreen.WorkingArea;
    Location = new Point(wa.Left + Math.Max(0, (wa.Width - Width) / 2),
        wa.Top + Math.Max(0, (wa.Height - Height) / 2));
}

Alternatively, if we don't want the extra code here, adding StartPosition = CenterParent and
dropping the position restore for this dialog would also be consistent with the rest of the PR.

---

3. Inline — Src/xWorks/UploadToWebonaryDlg.cs, lines 342–351 (OnClosing)

Minor, but this save path is also newly live. If the user maximizes this (resizable) dialog,
Location/Size are the maximized values — origin around (-8, -8) on Windows — and we restore
them as a normal-state window, so it reopens slightly off the top-left corner. Worth using the
restore bounds:

			if (m_propertyTable != null)
			{
				var bounds = WindowState == FormWindowState.Normal ? DesktopBounds : RestoreBounds;
				m_propertyTable.SetProperty("UploadToWebonaryDlg_Location", bounds.Location, false);
				m_propertyTable.SetPropertyPersistence("UploadToWebonaryDlg_Location", true);
				m_propertyTable.SetProperty("UploadToWebonaryDlg_Size", bounds.Size, false);
				m_propertyTable.SetPropertyPersistence("UploadToWebonaryDlg_Size", true);
			}

---

4. Inline — Src/xWorks/DictionaryConfigurationManagerDlg.cs, lines 54–60 (OnShown)

Nit: after the CenterParent you added at line 28, this is close to a no-op — CenterToParent
already clamps the dialog into the owner's screen working area, so EnsureVisibleRect won't have
anything to move. And where it would fire, it only guarantees "on some screen", not "on the
owner's screen", so the comment ("not displayed off-screen on multi-monitor setups") promises a bit
more than it delivers.

Fine to leave as belt-and-braces — but if we add the shared EnsureOnOwnerScreen helper mentioned
in my summary, this override should call that instead so all the dialogs behave the same way.

---

5. Optional inline — Src/xWorks/UploadToWebonaryDlg.cs, line 38

Just noting for the record: changing protected PropertyTable PropertyTable { get; set; } to a
private field is a (harmless) API narrowing — I checked and nothing else in the repo subclasses
this dialog or reads that property, so no callers break.

[thejambi]

New changes look good!

@thejambi reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on jasonleenaylor).