Bring the main window to the front when reopening a collection (BL-16784)

[StephenMcConnel]

Switching collections doesn't restart Bloom — it closes the Shell and opens a new one in the same process. The only code that forces the main window to the front, Shell.ReallyComeToFront(), was reached only through the one-shot StartupScreenManager.DoLastOfAllAfterClosingSplashScreen, which is consumed when the splash screen closes at first startup. So on a reopen nothing brought the new window forward: it had only Show()'s implicit activation, which Windows refuses once another application (Chrome, say) took the foreground as Bloom's previous window closed. The window was displayed behind Chrome.

Why it looked new in 6.5 and only off the main monitor: the activation is a race, and Application.SetHighDpiMode(HighDpiMode.PerMonitorV2) (BL-16269) means Shell_Load setting saved bounds onto a different-DPI monitor now triggers a rescale before the window settles — enough extra work to lose a race it used to win.

The change

• Program.OpenProjectWindow — after Show(), bring the new Shell forward when nothing else will, using the new StartupScreenManager.WillBringMainWindowToFrontWhenSplashCloses predicate. This is the one place every way of opening a collection funnels through, so it covers switching collections and the close/reopen after a UI-language or Collection Settings change. First startup is excluded, because the splash one-shot does it there and doing it twice would put the main window over the dialogs startup puts up.
• Extensions — extracted BringToFrontNow from the BL-16690 BringToFrontWhenShown, so both share the topmost-then-drop idiom.
• Shell.ReallyComeToFront — uses it, instead of the instant TopMost toggle that BL-16690 already documented as losing this race.
• ProcessExtra.ForceWindowToForeground — being topmost is not enough on its own: when another application holds the foreground Windows refuses our Activate(), so the raised window is still not the active one and dropping topmost lands us behind it again. Measured: with only the topmost toggle, Bloom came up second, directly behind Chrome. This briefly attaches our input queue to the foreground window's thread, the standard way to be allowed to take the foreground.

Also fixed as a consequence: Shell._finishedLoading is set only in ReallyComeToFront, and Shell_ResizeEnd won't save window bounds until it is true — so after a collection switch Bloom silently stopped saving its window size and position.

Testing

Two tests in StartupScreenManagerTests cover the predicate the fix hangs on: that CloseSplashScreen consumes the one-shot (so a reopen answers "nobody will do this"), and that HideSplashScreenForDialog deliberately does not (so the startup route that shows the collection chooser still answers "no need").

Verified by hand on a two-monitor setup with Bloom and Chrome both maximized on the secondary monitor, switching collections:

• before the fix: Bloom's window ended up below Chrome, not even in the top twelve of the z-order;
• after: Chrome foreground before the switch, Bloom rank 0 and foreground after.

Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16784

Devin review

---

This change is 

[greptile-apps]

Greptile Summary

The PR makes newly reopened collection windows reliably become active and foreground on Windows while preserving the existing splash-screen startup behavior.

• Adds a reusable delayed TopMost/foreground helper and uses it for Shell activation and existing dialog activation.
• Tracks whether splash closure will perform the initial foreground action, allowing later collection opens to do it directly.
• Adds native foreground-window handling and tests for splash one-shot lifecycle behavior.

Important Files Changed

Filename	Overview
src/BloomExe/Program.cs	Adds the foreground action after opening a collection when the startup splash one-shot will not perform it.
src/BloomExe/Extensions.cs	Extracts the shared delayed TopMost activation behavior and adds explicit Windows foreground activation.
src/BloomExe/ToPalaso/ProcessExtra.cs	Adds guarded foreground-window activation using temporary input-thread attachment with cleanup in a finally block.
src/BloomExe/MiscUI/StartupScreenManager.cs	Exposes whether the splash-close foreground one-shot remains pending.
src/BloomExe/Shell.cs	Reuses the new foreground helper while retaining the loading-complete state transition.
src/BloomTests/MiscUI/StartupScreenManagerTests.cs	Verifies that closing the splash consumes the foreground one-shot while temporarily hiding it for a dialog does not.

_{Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/Ver..." | Re-trigger Greptile}

[StephenMcConnel]

[Claude Opus 5 (1M context)] Consulted Devin on 2026-08-31 20:31 UTC up to commit 1afdf537e077e08d34fbb46563d188250fb5dd65.

Devin's review completed for this commit with nothing to report: no bugs, no Investigate flags, and no Informational items. Greptile posted a summary with no findings, and both PR checks passed.

[JohnThomson]

Couple of minor suggestions, just for clarity

@JohnThomson reviewed 6 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on StephenMcConnel).

---

src/BloomExe/Extensions.cs line 86 at r1 (raw file):

        /// <summary>
        /// Put the form in front of other applications right now. We stay topmost for a moment
        /// instead of dropping it immediately: when the window we are racing was made foreground

There's a couple of mentions of "the window we are racing" (not new here, but the other occurrence is) and it reads strangely, as if the comment had already mentioned such a window. I think the idea is that Bloom closed its window, that has a side effect of causing some other window to come to the front, and there is a race between it coming to the front and our own attempt to come back to the front. Something like "when another window is in the process of coming to the front as a result of Bloom recently closing its own window..."

---

src/BloomExe/Shell.cs line 405 at r1 (raw file):

            // may raise itself just after we do, and an instant toggle loses to that. BringToFrontNow
            // explains why the late drop wins either way. (An instant toggle is what we used to do
            // here, and it is why Bloom could come up behind Chrome. BL-16784)

I don't think this comment adds anything. It just repeats half of what the comment in Extensions.BringToFrontNow() says, with even less context for "the window we are racing". (And is there even a good reason to have this method now? It means we have two methods on the same class with different names that do essentially the same thing...if it's only called in one or two places I would be inclined to just inline it.)

[StephenMcConnel]

@StephenMcConnel reviewed 2 files and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on JohnThomson).

---

src/BloomExe/Extensions.cs line 86 at r1 (raw file):

Previously, JohnThomson (John Thomson) wrote…

There's a couple of mentions of "the window we are racing" (not new here, but the other occurrence is) and it reads strangely, as if the comment had already mentioned such a window. I think the idea is that Bloom closed its window, that has a side effect of causing some other window to come to the front, and there is a race between it coming to the front and our own attempt to come back to the front. Something like "when another window is in the process of coming to the front as a result of Bloom recently closing its own window..."

Done. I changed the comment.

---

src/BloomExe/Shell.cs line 405 at r1 (raw file):

Previously, JohnThomson (John Thomson) wrote…

I don't think this comment adds anything. It just repeats half of what the comment in Extensions.BringToFrontNow() says, with even less context for "the window we are racing". (And is there even a good reason to have this method now? It means we have two methods on the same class with different names that do essentially the same thing...if it's only called in one or two places I would be inclined to just inline it.)

Done. I changed the comment. This method also sets an internal flag for Shell, and is called once in Shell.cs and in two different places in Program.cs, so it can't simply be inlined.

[StephenMcConnel]

[Claude Opus 5 (1M context)] Consulted Devin on 2026-09-03 17:30 UTC up to commit b148a5dab8a77667b33db7c5209946dbb4ca28e5.

Devin raised one thing this time, a non-severe bug: repeated raises of the same window can cut
each other's 1.5-second protection short, because each raise starts its own timer. It is mirrored
as a review thread on Extensions.cs and left open — it is going to Steve as a decision
rather than being changed during preflight, because the tidy fix needs per-form state that has to
be cleaned up when the form closes. No Investigate flags and no Informational items.

The full C# suite is green at this commit (3313 passed, 13 skipped) and pr-automation passed.

[JohnThomson]

I believe this will work...I'm just being picky about a name here. If it doesn't seem worth changing (or not even desirable) you can just merge.

@JohnThomson reviewed 2 files and all commit messages, made 2 comments, and resolved 3 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on StephenMcConnel).

---

src/BloomExe/Shell.cs line 405 at r1 (raw file):

Previously, StephenMcConnel (Steve McConnel) wrote…

Done. I changed the comment. This method also sets an internal flag for Shell, and is called once in Shell.cs and in two different places in Program.cs, so it can't simply be inlined.

I still don't like the different names which seem to mean the same, but are different enough to leave the reader wondering what the difference is...and as you say, there's an important difference, so we don't want a new user picking the wrong one. This method, for example, should not be used if there was some reason to come to the front before we finished loading. Looking again, it seems to me that actually the purpose of this method is to be where Shell handles the last things it needs do to when it is loaded...critically, it sets the flag so everything that cares knows loading is done...and it happens that another thing we want to do when loading is finished is to come to the front. Would it make sense to rename the method FinishLoading() or OnFinishedLoading()?