From 4c4e2bd69c30b2eba11f8713d7f7873e0b82129e Mon Sep 17 00:00:00 2001 From: Hatton Date: Wed, 2 Sep 2026 15:39:21 -0600 Subject: [PATCH 1/8] Run the e2e suite off-screen (BL-16804) Every run of the src/BloomE2E suite opened a Bloom window on the developer's desktop, which makes the suite unusable while working. A new --headless startup flag puts the shell and the splash screen far to the left of every monitor and keeps them out of the task bar, and Bloom does not save those bounds as the window placement. The window goes off-screen rather than minimized or hidden because WebView2 stops painting a minimized window, which makes every screenshot blank; off-screen it paints exactly as it would in front of a person, so rendering and keyboard input behave the same. Windows reports a window that far out as occluded, and Chromium then stops rendering it, so a headless run also turns off CalculateNativeWinOcclusion and the backgrounding of occluded windows. Shell sets ShowInTaskbar in its constructor rather than in Shell_Load, where the rest of the headless placement happens. Shell_Load runs during Show(), when the form already has a window handle, and Windows Forms answers an assignment to ShowInTaskbar there by recreating that handle and every child handle under it, including the WebView2 host. The Edit tab was left with a browser that no longer answered editView/jumpToPage, so every e2e test that moves between pages hung until goToPage gave up. The e2e fixture passes the flag by default. BLOOM_E2E_HEADED=1 shows the window, and Playwright's --debug implies it, because stepping through a test whose window you cannot see is pointless. Split out of BL-16799 so that it can be reviewed on its own. Co-Authored-By: Claude Opus 5 (1M context) --- .github/skills/add-e2e-test/SKILL.md | 11 ++- src/BloomE2E/README.md | 13 +++- src/BloomE2E/fixtures/launchBloom.ts | 25 ++++-- src/BloomExe/Program.cs | 15 ++++ src/BloomExe/Shell.cs | 77 +++++++++++++++++-- src/BloomExe/SplashScreen.cs | 28 +++++-- src/BloomExe/WebView2Browser.cs | 8 ++ .../web/controllers/ProblemReportApi.cs | 8 +- src/BloomTests/ProgramTests.cs | 27 +++++++ 9 files changed, 190 insertions(+), 22 deletions(-) diff --git a/.github/skills/add-e2e-test/SKILL.md b/.github/skills/add-e2e-test/SKILL.md index 25bbc75c8999..9b42496750b2 100644 --- a/.github/skills/add-e2e-test/SKILL.md +++ b/.github/skills/add-e2e-test/SKILL.md @@ -338,9 +338,14 @@ pnpm exec playwright test tests/workspace-tabs.spec.ts # one file pnpm exec playwright test -g "switching workspace tabs" # one test by title ``` -A run opens a real Bloom window; that is expected. It needs a built `Bloom.exe` under -`output/{Debug,Release}/{x64,AnyCPU,}/` (build it yourself; see "Build Bloom whenever it -helps") and the inputs at `output/testing-inputs`. Point +A run launches a real Bloom, but no window appears: the fixture passes `--headless`, which puts +Bloom's window far outside every monitor, so a run does not take your desktop over. Set +`BLOOM_E2E_HEADED=1` to watch it (`--debug` sets it for you). The window goes off-screen rather +than minimized or hidden because WebView2 stops painting a minimized window, which makes every +screenshot blank. + +A run needs a built `Bloom.exe` under `output/{Debug,Release}/{x64,AnyCPU,}/` (build it yourself; +see "Build Bloom whenever it helps") and the inputs at `output/testing-inputs`. Point `BLOOM_TESTING_INPUTS_DIR` at a bloom-testing-inputs checkout to use your own in-progress collections instead of the pinned ones. diff --git a/src/BloomE2E/README.md b/src/BloomE2E/README.md index a21850ea1240..54a1955ed66c 100644 --- a/src/BloomE2E/README.md +++ b/src/BloomE2E/README.md @@ -135,7 +135,18 @@ pnpm exec playwright test -g "switching workspace tabs" # one test by titl pnpm exec playwright test --debug # step through it ``` -A run opens a real Bloom window. That is expected; do not click in it. +A run opens a real Bloom, but you will not see it: Bloom is launched with `--headless`, which puts +its window far outside every monitor, so a run can go on while you work. The window is moved +off-screen rather than minimized or hidden because WebView2 stops painting a minimized window, +which would make every screenshot blank. + +To watch the run instead, set `BLOOM_E2E_HEADED=1`; `--debug` turns it on for you. + +```bash +BLOOM_E2E_HEADED=1 pnpm exec playwright test tests/workspace-tabs.spec.ts +``` + +A headed run opens a real Bloom window. That is expected; do not click in it. The suite needs a built `Bloom.exe` under `output/{Debug,Release}/{x64,AnyCPU,}/` and the test inputs at `output/testing-inputs`, fetched by `node build/get-testing-inputs.mjs` at the commit diff --git a/src/BloomE2E/fixtures/launchBloom.ts b/src/BloomE2E/fixtures/launchBloom.ts index 7b8e6c566e8c..c7e1fc1ea8bd 100644 --- a/src/BloomE2E/fixtures/launchBloom.ts +++ b/src/BloomE2E/fixtures/launchBloom.ts @@ -158,6 +158,22 @@ function samePath(a: string, b: string): boolean { const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); +/** + * Whether the Bloom we launch should appear on a monitor. By default it does not: --headless puts + * its window far outside every screen, so a run does not take the developer's desktop over. + * + * The window is moved off-screen rather than minimized or hidden because WebView2 stops painting a + * minimized window: screenshots come back blank and the layout is the wrong size. Off-screen, the + * window paints exactly as it would in front of a person, so keyboard input and rendering behave + * the same (see Shell.GetHeadlessBounds). + * + * Set BLOOM_E2E_HEADED=1 to watch the run. Playwright's --debug (which sets PWDEBUG) implies it: + * there is no point stepping through a test whose window you cannot see. + */ +function shouldShowBloomOnScreen(): boolean { + return process.env.BLOOM_E2E_HEADED === "1" || !!process.env.PWDEBUG; +} + /** What common/instanceInfo tells us about a running Bloom. Only the fields we use. */ interface IInstanceInfo { editableCollectionFolder?: string; @@ -363,11 +379,10 @@ async function startBloomOn( // --e2e: skip the DEBUG "attach debugger now" prompt and suppress modal error dialogs. // --automation: let this instance run alongside a Bloom the developer already has open. - const bloomProcess: ChildProcess = execFile(exe, [ - findCollectionFile(collectionDir), - "--e2e", - "--automation", - ]); + // --headless: keep the window off every screen (see shouldShowBloomOnScreen). + const args = [findCollectionFile(collectionDir), "--e2e", "--automation"]; + if (!shouldShowBloomOnScreen()) args.push("--headless"); + const bloomProcess: ChildProcess = execFile(exe, args); let exitStatus: { code: number | null; signal: string | null } | undefined; bloomProcess.stdout?.on("data", (d) => recordOutput(String(d))); bloomProcess.stderr?.on("data", (d) => recordOutput(String(d))); diff --git a/src/BloomExe/Program.cs b/src/BloomExe/Program.cs index d90eb537146b..9a22212450aa 100644 --- a/src/BloomExe/Program.cs +++ b/src/BloomExe/Program.cs @@ -108,6 +108,11 @@ static class Program internal static string StartupLabel { get; private set; } internal static bool StartupAutomation { get; private set; } + // True when --headless was passed. An e2e run wants Bloom's window to paint (screenshots + // and keyboard input both need that) but not to appear on any monitor, so Shell places the + // window far outside every screen instead of minimizing or hiding it. See Shell_Load. + internal static bool StartupHeadless { get; private set; } + // Control port of the dev launcher (scripts/watchBloomExe.mjs) that started // this Bloom, passed as --launcher-port. When present, DevLauncher watches for // pending C# changes and offers a dev-only toast that asks the launcher to @@ -120,6 +125,7 @@ static class Program new[] { StartupAutomation ? "automation=true" : null, + StartupHeadless ? "headless=true" : null, StartupVitePort.HasValue ? $"vitePort={StartupVitePort.Value}" : null, StartupLauncherPort.HasValue ? $"launcherPort={StartupLauncherPort.Value}" @@ -785,6 +791,7 @@ internal static string[] ParseStartupPortArguments(string[] args, out string err StartupVitePort = null; StartupLabel = null; StartupAutomation = false; + StartupHeadless = false; StartupLauncherPort = null; RunningE2eTests = false; @@ -826,6 +833,14 @@ out errorMessage value => StartupAutomation = value, out errorMessage ) + || TryHandleStartupFlagArgument( + args, + ref i, + "--headless", + () => StartupHeadless, + value => StartupHeadless = value, + out errorMessage + ) || TryHandleStartupFlagArgument( args, ref i, diff --git a/src/BloomExe/Shell.cs b/src/BloomExe/Shell.cs index 54cf046b558e..44cf99e01c15 100644 --- a/src/BloomExe/Shell.cs +++ b/src/BloomExe/Shell.cs @@ -48,8 +48,10 @@ public static Form GetShellOrOtherOpenForm() private bool _finishedLoading; // During an automation run (--automation, e.g. the Playwright suites) the window must - // not steal the user's keyboard focus when it is shown. - protected override bool ShowWithoutActivation => Program.StartupAutomation; + // not steal the user's keyboard focus when it is shown. The same goes for a headless run + // (--headless), whose window sits off-screen where the user cannot see it at all. + protected override bool ShowWithoutActivation => + Program.StartupAutomation || Program.StartupHeadless; /// /// The screen that an automation run (--automation) should open windows on: the one @@ -72,6 +74,39 @@ public static Screen GetAutomationScreen() return Screen.PrimaryScreen; } + /// + /// Where a headless run (--headless) puts its window: the size of the automation screen's + /// working area, but positioned to the left of every monitor, so that not one pixel of it + /// is on any screen. + /// + /// The window is moved rather than minimized or hidden because a minimized WebView2 stops + /// painting: screenshots come back blank and the layout is the wrong size. An off-screen + /// window of the normal size keeps painting, so a test sees exactly what a user would. + /// + public static Rectangle GetHeadlessBounds() + { + var size = GetAutomationScreen().WorkingArea.Size; + // Two bounds, and the window has to respect both. + // + // The first is the leftmost monitor: the window's right edge has to be left of it, or + // part of the window shows. The 1000-pixel cushion is there because Windows and this + // process do not always agree about how many pixels wide a monitor is, which is what + // happens when the monitors have different scale factors. + // + // The second is -32000, as far left as a window may go: Windows still places a window + // there, and anything beyond about -32768 runs into the 16-bit coordinates that some + // of the older window messages still carry. + // + // On any real layout the first bound gives a few thousand pixels to the left, well + // inside the second. A leftward run of monitors more than about 30000 pixels wide + // would need a position that satisfies neither, and then the limit wins: a window at + // a coordinate Windows will not honour is worse than one that overlaps a monitor. + const int farLeftWindowsAllows = -32000; + var leftmostX = Screen.AllScreens.Min(screen => screen.Bounds.Left); + var x = Math.Max(farLeftWindowsAllows, leftmostX - size.Width - 1000); + return new Rectangle(x, 0, size.Width, size.Height); + } + public Shell( Func projectViewFactory, CollectionSettings collectionSettings, @@ -91,6 +126,19 @@ AudioRecording audioRecording _controlKeyEvent = controlKeyEvent; _audioRecording = audioRecording; InitializeComponent(); + if (Program.StartupHeadless) + { + // Keep the off-screen window out of the task bar, so a headless run leaves no + // trace on the developer's desktop. + // + // This has to happen before the window handle exists, which is why it is here + // and not in Shell_Load with the rest of the headless placement. Assigning + // ShowInTaskbar on a form that is already showing makes Windows Forms recreate + // the form's handle, and every child handle with it, including the WebView2 + // host. The Edit tab survived that with a browser that no longer answered a + // jump to another page, so every e2e test that moves between pages hung. + ShowInTaskbar = false; + } Activated += (sender, args) => { // In at least one case (BL-15060) we seem to have gotten activated @@ -424,8 +472,10 @@ public static void ComeToFront() public void ReallyComeToFront() { // During an automation run, grabbing focus would yank the user's keyboard away - // from whatever they are doing on another monitor while tests run. - if (!Program.StartupAutomation) + // from whatever they are doing on another monitor while tests run. A headless + // window must not come to the front either: it is off-screen on purpose, and + // TopMost/BringToFront on it would take the foreground away for nothing. + if (!Program.StartupAutomation && !Program.StartupHeadless) { //try really hard to become top most. See http://stackoverflow.com/questions/5282588/how-can-i-bring-my-application-window-to-the-front TopMost = true; @@ -446,7 +496,18 @@ private void Shell_Load(object sender, EventArgs e) { SuspendLayout(); - if (Program.StartupAutomation) + if (Program.StartupHeadless) + { + // A headless run keeps the window off every screen and out of the task bar, + // so a test can run while the developer works. The window stays Normal (not + // minimized) and full size, because WebView2 only paints a window that is + // neither minimized nor hidden. See GetHeadlessBounds. + StartPosition = FormStartPosition.Manual; + WindowState = FormWindowState.Normal; + Bounds = GetHeadlessBounds(); + // ShowInTaskbar is set in the constructor, not here. See the comment there. + } + else if (Program.StartupAutomation) { // An automation run must not open on whichever monitor the user is // currently working on, and must not disturb the saved window placement. @@ -468,7 +529,7 @@ private void Shell_Load(object sender, EventArgs e) // This feature is not yet a normal part of Bloom, since we think just maximizing is more rice-farmer-friendly. // However, we added the ability to remember this stuff at the request of the person making videos, who needs // Bloom to open in the same place / size each time. - if (Program.StartupAutomation) + if (Program.StartupAutomation || Program.StartupHeadless) { // Placement is already pinned above; leave the user's saved placement alone. } @@ -530,6 +591,10 @@ private void Shell_ResizeEnd(object sender, EventArgs e) return; if (WindowState != FormWindowState.Normal) return; + // A headless window is deliberately off every screen and is Normal rather than + // maximized, so saving its bounds would leave the developer's next Bloom invisible. + if (Program.StartupHeadless || Program.StartupAutomation) + return; Settings.Default.RestoreBounds = new Rectangle(Left, Top, Width, Height); Settings.Default.Save(); diff --git a/src/BloomExe/SplashScreen.cs b/src/BloomExe/SplashScreen.cs index aee6218cb1a9..f4f8a037960b 100644 --- a/src/BloomExe/SplashScreen.cs +++ b/src/BloomExe/SplashScreen.cs @@ -29,13 +29,28 @@ public void FadeAndClose() } // During an automation run (--automation) the splash must not steal the user's - // keyboard focus when it is shown. - protected override bool ShowWithoutActivation => Program.StartupAutomation; + // keyboard focus when it is shown. Neither must a headless run (--headless), whose + // windows all sit off-screen. + protected override bool ShowWithoutActivation => + Program.StartupAutomation || Program.StartupHeadless; private SplashScreen() { InitializeComponent(); - if (Program.StartupAutomation) + if (Program.StartupHeadless) + { + // A headless run shows nothing on any monitor, so the splash goes off-screen with + // the main window and out of the task bar with it. See Shell.GetHeadlessBounds. + // + // The splash normally has a task bar entry, and an off-screen window with one is + // worse than either: the developer gets a task bar button for a window they + // cannot bring into view. + StartPosition = FormStartPosition.Manual; + var headlessArea = Shell.GetHeadlessBounds(); + Location = new System.Drawing.Point(headlessArea.Left, headlessArea.Top); + ShowInTaskbar = false; + } + else if (Program.StartupAutomation) { // An automation run must not open on whichever monitor the user is currently // working on. Center the splash on the automation screen instead. @@ -111,8 +126,9 @@ private void _fadeOutTimer_Tick(object sender, EventArgs e) private void SplashScreen_Load(object sender, EventArgs e) { // During an automation run, grabbing focus would yank the user's keyboard away - // from whatever they are doing on another monitor while tests run. - if (!Program.StartupAutomation) + // from whatever they are doing on another monitor while tests run. A headless splash + // is off-screen, so bringing it to the front would take the foreground for nothing. + if (!Program.StartupAutomation && !Program.StartupHeadless) { //try really hard to become top most. See http://stackoverflow.com/questions/5282588/how-can-i-bring-my-application-window-to-the-front TopMost = true; @@ -122,7 +138,7 @@ private void SplashScreen_Load(object sender, EventArgs e) _channelLabel.Visible = channel.ToLowerInvariant() != "release"; _channelLabel.Text = channel; // No need to localize this: seen only by testers or special users (BL-4451) _copyrightlabel.Text = $"© 2011-{DateTime.Now.Year} SIL Global"; - if (!Program.StartupAutomation) + if (!Program.StartupAutomation && !Program.StartupHeadless) BringToFront(); } diff --git a/src/BloomExe/WebView2Browser.cs b/src/BloomExe/WebView2Browser.cs index e47eb7f19b2c..638b9e74647c 100644 --- a/src/BloomExe/WebView2Browser.cs +++ b/src/BloomExe/WebView2Browser.cs @@ -396,6 +396,14 @@ private async Task InitWebView() { additionalBrowserArgs += " --accept-lang=" + _uiLanguageOfThisRun; } + if (Program.StartupHeadless) + { + // A headless run keeps Bloom's window far off-screen (see Shell.GetHeadlessBounds), + // so Windows reports the window as occluded and Chromium stops rendering it. A + // screenshot of an unrendered page comes back blank, so turn that behavior off. + featuresToDisable.Add("CalculateNativeWinOcclusion"); + additionalBrowserArgs += " --disable-backgrounding-occluded-windows"; + } if (RemoteDebuggingPort.HasValue && !Program.RunningUnitTests) { // Expose a CDP endpoint so Playwright and other automation can attach to the real Bloom WebView2 surface. diff --git a/src/BloomExe/web/controllers/ProblemReportApi.cs b/src/BloomExe/web/controllers/ProblemReportApi.cs index 567ec4e5b05c..3eded937d1ba 100644 --- a/src/BloomExe/web/controllers/ProblemReportApi.cs +++ b/src/BloomExe/web/controllers/ProblemReportApi.cs @@ -1173,10 +1173,16 @@ private static void TryGetScreenshot(Control controlForScreenshotting) { ResetScreenshotFile(); } - else if (IsBloomProcessInForeground()) + else if (IsBloomProcessInForeground() && !Program.StartupHeadless) { // Bloom is the foreground app: a plain screen copy is cheaper // and avoids re-triggering any paint-related bugs. + // + // Not under --headless, though. That run's window sits far + // outside every monitor, so copying from those screen coordinates + // would save whatever the desktop has there, which is nothing. + // Render the window itself instead, the way the not-in-front case + // already does. var scaledBounds = controlForScreenshotting.Bounds; #if !__MonoCS__ scaledBounds = diff --git a/src/BloomTests/ProgramTests.cs b/src/BloomTests/ProgramTests.cs index f4c68c636f68..4dd021366f6c 100644 --- a/src/BloomTests/ProgramTests.cs +++ b/src/BloomTests/ProgramTests.cs @@ -56,6 +56,33 @@ out var automationErrorMessage Assert.That(Program.StartupRequestedPortSummary, Is.EqualTo("automation=true")); } + [Test] + public void ParseStartupPortArguments_StoresHeadlessFlag() + { + var remainingArgs = Program.ParseStartupPortArguments( + new[] { "--automation", "--headless", @"C:\Temp\Example.bloomcollection" }, + out var errorMessage + ); + + Assert.That(errorMessage, Is.Null); + Assert.That(Program.StartupHeadless, Is.True); + Assert.That(Program.StartupAutomation, Is.True); + Assert.That( + Program.StartupRequestedPortSummary, + Is.EqualTo("automation=true, headless=true") + ); + Assert.That(remainingArgs, Is.EqualTo(new[] { @"C:\Temp\Example.bloomcollection" })); + } + + [Test] + public void ParseStartupPortArguments_LeavesHeadlessFalseWithoutTheFlag() + { + Program.ParseStartupPortArguments(new[] { "--automation" }, out var errorMessage); + + Assert.That(errorMessage, Is.Null); + Assert.That(Program.StartupHeadless, Is.False); + } + [Test] public void ParseStartupPortArguments_VitePortAloneDoesNotEnableAutomation() { From f072eed95cfc21ec30af75583261432f9db7dbfd Mon Sep 17 00:00:00 2001 From: Hatton Date: Thu, 3 Sep 2026 07:14:14 -0600 Subject: [PATCH 2/8] One variable decides where an automation run's windows go (BL-16804) BLOOM_AUTOMATION_MONITOR becomes the only control, and every window an automation run opens obeys it, the splash screen included. It takes a 1-based monitor number, the word "headless" for off every monitor, or anything else, which leaves Bloom to place its windows as it always does. This replaces three controls that each did part of the job: the --headless startup flag, the BLOOM_E2E_HEADED variable in the Playwright fixture, and a silent fallback to the primary monitor when the variable named a monitor the machine did not have. A value Bloom cannot use now gives a visible window, which is the outcome that tells the developer the variable did not take effect. The new class AutomationWindowPlacement holds the whole decision. Its Parse method takes the raw value and the monitor count as arguments, so the 21 new unit tests need neither the environment variable nor a machine with a particular set of monitors. The off-screen position leaves four window widths of clearance, not a fixed 1000 pixels. Bloom computes the position in its own coordinate space, and on a monitor scaled to 160% a window it believed was 1587 pixels wide came out 2560 pixels wide, which left 27 pixels of the old cushion. Windows scales a monitor by at most 400%, so four widths clears the leftmost monitor whatever the scale factors are. Co-Authored-By: Claude Opus 5 (1M context) --- .github/skills/add-e2e-test/SKILL.md | 13 +- src/BloomE2E/README.md | 34 +++- src/BloomE2E/fixtures/launchBloom.ts | 34 ++-- src/BloomExe/AutomationWindowPlacement.cs | 168 ++++++++++++++++ src/BloomExe/Program.cs | 15 -- src/BloomExe/Shell.cs | 122 ++++-------- src/BloomExe/SplashScreen.cs | 39 ++-- src/BloomExe/WebView2Browser.cs | 9 +- .../web/controllers/ProblemReportApi.cs | 14 +- .../AutomationWindowPlacementTests.cs | 183 ++++++++++++++++++ src/BloomTests/ProgramTests.cs | 27 --- 11 files changed, 479 insertions(+), 179 deletions(-) create mode 100644 src/BloomExe/AutomationWindowPlacement.cs create mode 100644 src/BloomTests/AutomationWindowPlacementTests.cs diff --git a/.github/skills/add-e2e-test/SKILL.md b/.github/skills/add-e2e-test/SKILL.md index 9b42496750b2..569c5a394377 100644 --- a/.github/skills/add-e2e-test/SKILL.md +++ b/.github/skills/add-e2e-test/SKILL.md @@ -338,11 +338,14 @@ pnpm exec playwright test tests/workspace-tabs.spec.ts # one file pnpm exec playwright test -g "switching workspace tabs" # one test by title ``` -A run launches a real Bloom, but no window appears: the fixture passes `--headless`, which puts -Bloom's window far outside every monitor, so a run does not take your desktop over. Set -`BLOOM_E2E_HEADED=1` to watch it (`--debug` sets it for you). The window goes off-screen rather -than minimized or hidden because WebView2 stops painting a minimized window, which makes every -screenshot blank. +A run launches a real Bloom and its window appears on the developer's desktop, unless +`BLOOM_AUTOMATION_MONITOR` says otherwise. That one variable decides where every window a run +opens goes, the splash screen included: `headless` puts them all off every monitor, a 1-based +monitor number puts them on that monitor, and any other value, unset included, leaves Bloom to +place them as it always does. `headless` moves the window off-screen rather than minimizing it, +because WebView2 stops painting a minimized window and every screenshot then comes back blank. +`--debug` clears a `headless` setting, so a debug session has a window to step through. See +`src/BloomE2E/README.md` for the table. A run needs a built `Bloom.exe` under `output/{Debug,Release}/{x64,AnyCPU,}/` (build it yourself; see "Build Bloom whenever it helps") and the inputs at `output/testing-inputs`. Point diff --git a/src/BloomE2E/README.md b/src/BloomE2E/README.md index 54a1955ed66c..3e3fe47d6e97 100644 --- a/src/BloomE2E/README.md +++ b/src/BloomE2E/README.md @@ -135,18 +135,38 @@ pnpm exec playwright test -g "switching workspace tabs" # one test by titl pnpm exec playwright test --debug # step through it ``` -A run opens a real Bloom, but you will not see it: Bloom is launched with `--headless`, which puts -its window far outside every monitor, so a run can go on while you work. The window is moved -off-screen rather than minimized or hidden because WebView2 stops painting a minimized window, -which would make every screenshot blank. +A run opens a real Bloom window on your desktop. That is expected; do not click in it. -To watch the run instead, set `BLOOM_E2E_HEADED=1`; `--debug` turns it on for you. +### Where the Bloom window goes: `BLOOM_AUTOMATION_MONITOR` + +One environment variable decides where every window an e2e run opens goes, the main window and +the splash screen alike. Set it in your shell, or per run: + +| Value | What happens | +| --- | --- | +| `headless` | Every window opens far outside every monitor. You see nothing, so a run can go on while you work. | +| a 1-based monitor number | Every window opens on that monitor, so a run stays off the one you are working on. | +| unset, or anything else | Bloom places its windows as it always does, and you see the run. | ```bash -BLOOM_E2E_HEADED=1 pnpm exec playwright test tests/workspace-tabs.spec.ts +BLOOM_AUTOMATION_MONITOR=headless pnpm test # see nothing +BLOOM_AUTOMATION_MONITOR=2 pnpm test # on the second monitor +pnpm test # wherever Bloom normally opens ``` -A headed run opens a real Bloom window. That is expected; do not click in it. +A value Bloom cannot use, a typo or a monitor you do not have, counts as "anything else": you get +a visible window, which is exactly what tells you the variable did not take effect. A setting that +hid the window on a typo would leave you nothing to notice. + +`headless` moves the window off-screen rather than minimizing or hiding it, because WebView2 stops +painting a minimized window, which would make every screenshot blank. Off-screen the window paints +exactly as it would in front of you, so rendering and keyboard input behave the same. + +`--debug` clears a `headless` setting for you: stepping through a test whose window you cannot see +is pointless. A setting that names a monitor is left alone, because that window is visible anyway. + +The variable applies only to a run under `--automation`, which is every e2e run and nothing else. +A Bloom you start yourself is unaffected, however the variable is set. The suite needs a built `Bloom.exe` under `output/{Debug,Release}/{x64,AnyCPU,}/` and the test inputs at `output/testing-inputs`, fetched by `node build/get-testing-inputs.mjs` at the commit diff --git a/src/BloomE2E/fixtures/launchBloom.ts b/src/BloomE2E/fixtures/launchBloom.ts index c7e1fc1ea8bd..4535ed63a24c 100644 --- a/src/BloomE2E/fixtures/launchBloom.ts +++ b/src/BloomE2E/fixtures/launchBloom.ts @@ -159,19 +159,22 @@ function samePath(a: string, b: string): boolean { const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); /** - * Whether the Bloom we launch should appear on a monitor. By default it does not: --headless puts - * its window far outside every screen, so a run does not take the developer's desktop over. + * The environment the Bloom we launch runs in. One variable decides where its windows go, + * BLOOM_AUTOMATION_MONITOR, and Bloom reads it itself (see AutomationWindowPlacement.cs): + * "headless" puts every window off every monitor, a monitor number puts them on that monitor, and + * anything else, the variable being unset included, leaves Bloom to place its windows as it always + * does. So the child inherits this process's environment untouched, with one exception. * - * The window is moved off-screen rather than minimized or hidden because WebView2 stops painting a - * minimized window: screenshots come back blank and the layout is the wrong size. Off-screen, the - * window paints exactly as it would in front of a person, so keyboard input and rendering behave - * the same (see Shell.GetHeadlessBounds). - * - * Set BLOOM_E2E_HEADED=1 to watch the run. Playwright's --debug (which sets PWDEBUG) implies it: - * there is no point stepping through a test whose window you cannot see. + * The exception is Playwright's --debug (which sets PWDEBUG): stepping through a test whose window + * nobody can see is pointless, so a debug session clears a "headless" setting and gets a window. + * A setting that names a monitor is left alone, because that window IS visible. */ -function shouldShowBloomOnScreen(): boolean { - return process.env.BLOOM_E2E_HEADED === "1" || !!process.env.PWDEBUG; +function environmentForBloom(): NodeJS.ProcessEnv { + const asked = process.env.BLOOM_AUTOMATION_MONITOR?.trim().toLowerCase(); + if (process.env.PWDEBUG && asked === "headless") { + return { ...process.env, BLOOM_AUTOMATION_MONITOR: "" }; + } + return process.env; } /** What common/instanceInfo tells us about a running Bloom. Only the fields we use. */ @@ -378,11 +381,12 @@ async function startBloomOn( }; // --e2e: skip the DEBUG "attach debugger now" prompt and suppress modal error dialogs. - // --automation: let this instance run alongside a Bloom the developer already has open. - // --headless: keep the window off every screen (see shouldShowBloomOnScreen). + // --automation: let this instance run alongside a Bloom the developer already has open, and + // let BLOOM_AUTOMATION_MONITOR say where its windows go (see environmentForBloom). const args = [findCollectionFile(collectionDir), "--e2e", "--automation"]; - if (!shouldShowBloomOnScreen()) args.push("--headless"); - const bloomProcess: ChildProcess = execFile(exe, args); + const bloomProcess: ChildProcess = execFile(exe, args, { + env: environmentForBloom(), + }); let exitStatus: { code: number | null; signal: string | null } | undefined; bloomProcess.stdout?.on("data", (d) => recordOutput(String(d))); bloomProcess.stderr?.on("data", (d) => recordOutput(String(d))); diff --git a/src/BloomExe/AutomationWindowPlacement.cs b/src/BloomExe/AutomationWindowPlacement.cs new file mode 100644 index 000000000000..16da9a04b1b7 --- /dev/null +++ b/src/BloomExe/AutomationWindowPlacement.cs @@ -0,0 +1,168 @@ +using System; +using System.Drawing; +using System.Linq; +using System.Windows.Forms; + +namespace Bloom +{ + /// + /// What an automation run (--automation, e.g. the Playwright suites) does with the windows it + /// opens. One environment variable decides it, BLOOM_AUTOMATION_MONITOR: + /// + /// "2" (any 1-based index into Screen.AllScreens) every window opens on that monitor + /// "headless" every window opens off every monitor + /// absent, empty, or anything else Bloom places its windows as it always does + /// + /// The variable applies ONLY under --automation. A developer who leaves it set in their shell + /// therefore still gets an ordinary, visible Bloom when they start one themselves; only a run + /// that already declared itself automation obeys it. + /// + /// Nothing here changes whether an automation window takes the keyboard focus. It never does, + /// wherever it is (see Shell.ShowWithoutActivation and Shell.ReallyComeToFront). + /// + /// The variable is read on each call rather than cached. It costs nothing worth saving, and + /// nothing in Bloom writes it: the run's parent, normally the Playwright fixture, sets it + /// before Bloom starts, so every caller gets the same answer. That matters because the callers + /// have to agree. Shell places the window, and WebView2Browser turns off the occlusion check + /// for an off-screen one; if those two disagreed, every screenshot would come back blank. + /// + public static class AutomationWindowPlacement + { + /// The value of BLOOM_AUTOMATION_MONITOR that asks for off-screen windows. + public const string HeadlessSetting = "headless"; + + public const string VariableName = "BLOOM_AUTOMATION_MONITOR"; + + /// Where an automation run puts its windows. + public enum Choice + { + /// + /// Bloom places its windows the way it normally does, so a run appears on the + /// developer's desktop. This is what an absent or unrecognized value means: nobody + /// asked for anything, so nothing is imposed. + /// + AsBloomNormallyWould, + + /// The variable named a monitor that exists. Every window goes on it. + OnTheChosenMonitor, + + /// The variable said "headless". Every window goes off every monitor. + OffEveryMonitor, + } + + /// + /// Read BLOOM_AUTOMATION_MONITOR and say what this run should do. Answers + /// AsBloomNormallyWould for a run that is not automation, whatever the variable says. + /// + public static Choice GetChoice() + { + if (!Program.StartupAutomation) + return Choice.AsBloomNormallyWould; + return Parse( + Environment.GetEnvironmentVariable(VariableName), + Screen.AllScreens.Length, + out _ + ); + } + + /// + /// Turn one raw value of BLOOM_AUTOMATION_MONITOR into a choice. Split out from GetChoice + /// so it can be tested without an environment variable or a real set of monitors. + /// + /// A value naming a monitor this machine does not have counts as unrecognized, and so does + /// a typo: both mean Bloom places its windows normally. That is deliberate. The developer + /// then SEES a Bloom window, which is the outcome that tells them the variable did not take + /// effect; a typo that silently hid the window would leave them with no way to notice. + /// + /// The raw value, or null when the variable is not set. + /// How many monitors this machine has. + /// + /// The monitor asked for, when the answer is OnTheChosenMonitor; 0 otherwise. + /// + public static Choice Parse(string setting, int screenCount, out int oneBasedMonitor) + { + oneBasedMonitor = 0; + var trimmed = setting?.Trim(); + if (string.IsNullOrEmpty(trimmed)) + return Choice.AsBloomNormallyWould; + if (trimmed.Equals(HeadlessSetting, StringComparison.OrdinalIgnoreCase)) + return Choice.OffEveryMonitor; + if (int.TryParse(trimmed, out var index) && index >= 1 && index <= screenCount) + { + oneBasedMonitor = index; + return Choice.OnTheChosenMonitor; + } + return Choice.AsBloomNormallyWould; + } + + /// + /// True when this run keeps its windows off every monitor. Read this rather than the + /// variable: it is also false for a run that is not automation. + /// + public static bool IsOffEveryMonitor => GetChoice() == Choice.OffEveryMonitor; + + /// + /// The monitor an automation run opens its windows on. Only meaningful when the choice is + /// OnTheChosenMonitor; falls back to the primary screen otherwise, so that callers wanting + /// a size rather than a position have one. + /// + public static Screen GetChosenMonitor() + { + if ( + Program.StartupAutomation + && Parse( + Environment.GetEnvironmentVariable(VariableName), + Screen.AllScreens.Length, + out var oneBasedMonitor + ) == Choice.OnTheChosenMonitor + ) + { + return Screen.AllScreens[oneBasedMonitor - 1]; + } + return Screen.PrimaryScreen; + } + + /// + /// Where an off-every-monitor run puts a window: the size of the primary screen's working + /// area, positioned to the left of every monitor, so that not one pixel of it is on any + /// screen. + /// + /// The window is moved rather than minimized or hidden because a minimized WebView2 stops + /// painting: screenshots come back blank and the layout is the wrong size. An off-screen + /// window of the normal size keeps painting, so a test sees exactly what a user would. + /// + public static Rectangle GetBoundsOffEveryMonitor() + { + var size = Screen.PrimaryScreen.WorkingArea.Size; + // Two bounds, and the window has to respect both. + // + // The first is the leftmost monitor: the window's right edge has to be left of it, or + // part of the window shows. Both the position and the width above are in the + // coordinates this process sees, and the desktop does not always agree with them: on + // a machine whose primary monitor is scaled to 160%, Bloom asked for a window 1587 + // pixels wide and Windows made one 2560 pixels wide, which ate all but 27 pixels of a + // 1000-pixel cushion. So leave room for the whole error rather than a fixed number of + // pixels: Windows scales a monitor by at most 400%, so a window this process believes + // is W wide covers at most 4W pixels of the desktop. Four widths to the left of the + // leftmost monitor therefore clears it whatever the scale factors are, and the + // 1000 pixels on top of that keep the two edges from meeting exactly. + // + // The second is -32000, as far left as a window may go: Windows still places a window + // there, and anything beyond about -32768 runs into the 16-bit coordinates that some + // of the older window messages still carry. + // + // On any real layout the first bound gives several thousand pixels to the left, well + // inside the second. A leftward run of monitors more than about 30000 pixels wide + // would need a position that satisfies neither, and then the limit wins: a window at + // a coordinate Windows will not honour is worse than one that overlaps a monitor. + const int farLeftWindowsAllows = -32000; + const int largestScaleFactorWindowsAllows = 4; + var leftmostX = Screen.AllScreens.Min(screen => screen.Bounds.Left); + var x = Math.Max( + farLeftWindowsAllows, + leftmostX - (size.Width * largestScaleFactorWindowsAllows) - 1000 + ); + return new Rectangle(x, 0, size.Width, size.Height); + } + } +} diff --git a/src/BloomExe/Program.cs b/src/BloomExe/Program.cs index 9a22212450aa..d90eb537146b 100644 --- a/src/BloomExe/Program.cs +++ b/src/BloomExe/Program.cs @@ -108,11 +108,6 @@ static class Program internal static string StartupLabel { get; private set; } internal static bool StartupAutomation { get; private set; } - // True when --headless was passed. An e2e run wants Bloom's window to paint (screenshots - // and keyboard input both need that) but not to appear on any monitor, so Shell places the - // window far outside every screen instead of minimizing or hiding it. See Shell_Load. - internal static bool StartupHeadless { get; private set; } - // Control port of the dev launcher (scripts/watchBloomExe.mjs) that started // this Bloom, passed as --launcher-port. When present, DevLauncher watches for // pending C# changes and offers a dev-only toast that asks the launcher to @@ -125,7 +120,6 @@ static class Program new[] { StartupAutomation ? "automation=true" : null, - StartupHeadless ? "headless=true" : null, StartupVitePort.HasValue ? $"vitePort={StartupVitePort.Value}" : null, StartupLauncherPort.HasValue ? $"launcherPort={StartupLauncherPort.Value}" @@ -791,7 +785,6 @@ internal static string[] ParseStartupPortArguments(string[] args, out string err StartupVitePort = null; StartupLabel = null; StartupAutomation = false; - StartupHeadless = false; StartupLauncherPort = null; RunningE2eTests = false; @@ -833,14 +826,6 @@ out errorMessage value => StartupAutomation = value, out errorMessage ) - || TryHandleStartupFlagArgument( - args, - ref i, - "--headless", - () => StartupHeadless, - value => StartupHeadless = value, - out errorMessage - ) || TryHandleStartupFlagArgument( args, ref i, diff --git a/src/BloomExe/Shell.cs b/src/BloomExe/Shell.cs index 44cf99e01c15..9a9bfb4fe9e4 100644 --- a/src/BloomExe/Shell.cs +++ b/src/BloomExe/Shell.cs @@ -48,64 +48,9 @@ public static Form GetShellOrOtherOpenForm() private bool _finishedLoading; // During an automation run (--automation, e.g. the Playwright suites) the window must - // not steal the user's keyboard focus when it is shown. The same goes for a headless run - // (--headless), whose window sits off-screen where the user cannot see it at all. - protected override bool ShowWithoutActivation => - Program.StartupAutomation || Program.StartupHeadless; - - /// - /// The screen that an automation run (--automation) should open windows on: the one - /// chosen by the BLOOM_AUTOMATION_MONITOR environment variable (a 1-based index into - /// Screen.AllScreens), or the primary screen when the variable is absent or out of - /// range. Without this, Windows opens each test-launched window on whichever monitor - /// the user is currently working on. - /// - public static Screen GetAutomationScreen() - { - var setting = Environment.GetEnvironmentVariable("BLOOM_AUTOMATION_MONITOR"); - if ( - int.TryParse(setting, out var oneBasedIndex) - && oneBasedIndex >= 1 - && oneBasedIndex <= Screen.AllScreens.Length - ) - { - return Screen.AllScreens[oneBasedIndex - 1]; - } - return Screen.PrimaryScreen; - } - - /// - /// Where a headless run (--headless) puts its window: the size of the automation screen's - /// working area, but positioned to the left of every monitor, so that not one pixel of it - /// is on any screen. - /// - /// The window is moved rather than minimized or hidden because a minimized WebView2 stops - /// painting: screenshots come back blank and the layout is the wrong size. An off-screen - /// window of the normal size keeps painting, so a test sees exactly what a user would. - /// - public static Rectangle GetHeadlessBounds() - { - var size = GetAutomationScreen().WorkingArea.Size; - // Two bounds, and the window has to respect both. - // - // The first is the leftmost monitor: the window's right edge has to be left of it, or - // part of the window shows. The 1000-pixel cushion is there because Windows and this - // process do not always agree about how many pixels wide a monitor is, which is what - // happens when the monitors have different scale factors. - // - // The second is -32000, as far left as a window may go: Windows still places a window - // there, and anything beyond about -32768 runs into the 16-bit coordinates that some - // of the older window messages still carry. - // - // On any real layout the first bound gives a few thousand pixels to the left, well - // inside the second. A leftward run of monitors more than about 30000 pixels wide - // would need a position that satisfies neither, and then the limit wins: a window at - // a coordinate Windows will not honour is worse than one that overlaps a monitor. - const int farLeftWindowsAllows = -32000; - var leftmostX = Screen.AllScreens.Min(screen => screen.Bounds.Left); - var x = Math.Max(farLeftWindowsAllows, leftmostX - size.Width - 1000); - return new Rectangle(x, 0, size.Width, size.Height); - } + // not steal the user's keyboard focus when it is shown. That holds wherever the window + // is: on the developer's desktop, on a monitor of its own, or off every monitor. + protected override bool ShowWithoutActivation => Program.StartupAutomation; public Shell( Func projectViewFactory, @@ -126,10 +71,10 @@ AudioRecording audioRecording _controlKeyEvent = controlKeyEvent; _audioRecording = audioRecording; InitializeComponent(); - if (Program.StartupHeadless) + if (AutomationWindowPlacement.IsOffEveryMonitor) { - // Keep the off-screen window out of the task bar, so a headless run leaves no - // trace on the developer's desktop. + // Keep the off-screen window out of the task bar, so such a run leaves no trace + // on the developer's desktop. // // This has to happen before the window handle exists, which is why it is here // and not in Shell_Load with the rest of the headless placement. Assigning @@ -472,10 +417,10 @@ public static void ComeToFront() public void ReallyComeToFront() { // During an automation run, grabbing focus would yank the user's keyboard away - // from whatever they are doing on another monitor while tests run. A headless - // window must not come to the front either: it is off-screen on purpose, and - // TopMost/BringToFront on it would take the foreground away for nothing. - if (!Program.StartupAutomation && !Program.StartupHeadless) + // from whatever they are doing while tests run, and a window placed off every + // monitor cannot come to the front at all: TopMost and BringToFront on it would + // take the foreground away for nothing. + if (!Program.StartupAutomation) { //try really hard to become top most. See http://stackoverflow.com/questions/5282588/how-can-i-bring-my-application-window-to-the-front TopMost = true; @@ -496,28 +441,42 @@ private void Shell_Load(object sender, EventArgs e) { SuspendLayout(); - if (Program.StartupHeadless) + // Where an automation run puts its window is BLOOM_AUTOMATION_MONITOR's to + // decide (see AutomationWindowPlacement). A run that it says nothing about falls + // through to the ordinary cases below, window placement and all, so the + // developer sees the Bloom they would see without the variable. + var placement = AutomationWindowPlacement.GetChoice(); + if (placement == AutomationWindowPlacement.Choice.OffEveryMonitor) { - // A headless run keeps the window off every screen and out of the task bar, - // so a test can run while the developer works. The window stays Normal (not - // minimized) and full size, because WebView2 only paints a window that is - // neither minimized nor hidden. See GetHeadlessBounds. + // The window goes off every monitor and out of the task bar, so a test can + // run while the developer works. It stays Normal (not minimized) and full + // size, because WebView2 only paints a window that is neither minimized nor + // hidden. See AutomationWindowPlacement.GetBoundsOffEveryMonitor. StartPosition = FormStartPosition.Manual; WindowState = FormWindowState.Normal; - Bounds = GetHeadlessBounds(); + Bounds = AutomationWindowPlacement.GetBoundsOffEveryMonitor(); // ShowInTaskbar is set in the constructor, not here. See the comment there. } - else if (Program.StartupAutomation) + else if (placement == AutomationWindowPlacement.Choice.OnTheChosenMonitor) { - // An automation run must not open on whichever monitor the user is - // currently working on, and must not disturb the saved window placement. - // Pin the window to the automation screen (see GetAutomationScreen). + // Open on the monitor the variable named, not on whichever one the developer + // is working on, and leave the saved window placement alone. StartPosition = FormStartPosition.Manual; WindowState = FormWindowState.Normal; - Bounds = GetAutomationScreen().WorkingArea; + Bounds = AutomationWindowPlacement.GetChosenMonitor().WorkingArea; // Maximizing keeps the window on the screen that contains its bounds. WindowState = FormWindowState.Maximized; } + else if (Program.StartupAutomation) + { + // An automation run the variable said nothing about: open exactly where a + // Bloom with no saved placement opens, and write nothing. The guard below + // keeps it from touching the developer's saved placement either way, which + // it must not do whatever the variable says: every Bloom of one build shares + // one user.config. + StartPosition = FormStartPosition.WindowsDefaultLocation; + WindowState = FormWindowState.Maximized; + } else if (Settings.Default.WindowSizeAndLocation == null) { StartPosition = FormStartPosition.WindowsDefaultLocation; @@ -529,9 +488,9 @@ private void Shell_Load(object sender, EventArgs e) // This feature is not yet a normal part of Bloom, since we think just maximizing is more rice-farmer-friendly. // However, we added the ability to remember this stuff at the request of the person making videos, who needs // Bloom to open in the same place / size each time. - if (Program.StartupAutomation || Program.StartupHeadless) + if (Program.StartupAutomation) { - // Placement is already pinned above; leave the user's saved placement alone. + // Placement is settled above; leave the developer's saved placement alone. } else if (Settings.Default.MaximizeWindow == false) { @@ -591,9 +550,10 @@ private void Shell_ResizeEnd(object sender, EventArgs e) return; if (WindowState != FormWindowState.Normal) return; - // A headless window is deliberately off every screen and is Normal rather than - // maximized, so saving its bounds would leave the developer's next Bloom invisible. - if (Program.StartupHeadless || Program.StartupAutomation) + // An automation run must never write the saved bounds. Where BLOOM_AUTOMATION_MONITOR + // put its window is somewhere the developer is not looking, off every monitor or on a + // monitor of the run's choosing, and saving that would move their next Bloom there. + if (Program.StartupAutomation) return; Settings.Default.RestoreBounds = new Rectangle(Left, Top, Width, Height); diff --git a/src/BloomExe/SplashScreen.cs b/src/BloomExe/SplashScreen.cs index f4f8a037960b..c3962ec96cd2 100644 --- a/src/BloomExe/SplashScreen.cs +++ b/src/BloomExe/SplashScreen.cs @@ -29,33 +29,34 @@ public void FadeAndClose() } // During an automation run (--automation) the splash must not steal the user's - // keyboard focus when it is shown. Neither must a headless run (--headless), whose - // windows all sit off-screen. - protected override bool ShowWithoutActivation => - Program.StartupAutomation || Program.StartupHeadless; + // keyboard focus when it is shown. That holds wherever the splash is. + protected override bool ShowWithoutActivation => Program.StartupAutomation; private SplashScreen() { InitializeComponent(); - if (Program.StartupHeadless) + // The splash obeys BLOOM_AUTOMATION_MONITOR exactly as the main window does (see + // AutomationWindowPlacement). It is the second window every run opens, so a splash + // that ignored the variable would put a window on the developer's desktop however + // carefully the main one was placed. + var placement = AutomationWindowPlacement.GetChoice(); + if (placement == AutomationWindowPlacement.Choice.OffEveryMonitor) { - // A headless run shows nothing on any monitor, so the splash goes off-screen with - // the main window and out of the task bar with it. See Shell.GetHeadlessBounds. - // - // The splash normally has a task bar entry, and an off-screen window with one is + // Off every monitor with the main window, and out of the task bar with it. The + // splash normally has a task bar entry, and an off-screen window with one is // worse than either: the developer gets a task bar button for a window they // cannot bring into view. StartPosition = FormStartPosition.Manual; - var headlessArea = Shell.GetHeadlessBounds(); - Location = new System.Drawing.Point(headlessArea.Left, headlessArea.Top); + var offScreenArea = AutomationWindowPlacement.GetBoundsOffEveryMonitor(); + Location = new System.Drawing.Point(offScreenArea.Left, offScreenArea.Top); ShowInTaskbar = false; } - else if (Program.StartupAutomation) + else if (placement == AutomationWindowPlacement.Choice.OnTheChosenMonitor) { - // An automation run must not open on whichever monitor the user is currently - // working on. Center the splash on the automation screen instead. + // Center the splash on the monitor the variable named, rather than on whichever + // one the developer is working on. + var area = AutomationWindowPlacement.GetChosenMonitor().WorkingArea; StartPosition = FormStartPosition.Manual; - var area = Shell.GetAutomationScreen().WorkingArea; Location = new System.Drawing.Point( area.Left + (area.Width - Width) / 2, area.Top + (area.Height - Height) / 2 @@ -126,9 +127,9 @@ private void _fadeOutTimer_Tick(object sender, EventArgs e) private void SplashScreen_Load(object sender, EventArgs e) { // During an automation run, grabbing focus would yank the user's keyboard away - // from whatever they are doing on another monitor while tests run. A headless splash - // is off-screen, so bringing it to the front would take the foreground for nothing. - if (!Program.StartupAutomation && !Program.StartupHeadless) + // from whatever they are doing while tests run, and a splash off every monitor + // cannot come to the front at all. + if (!Program.StartupAutomation) { //try really hard to become top most. See http://stackoverflow.com/questions/5282588/how-can-i-bring-my-application-window-to-the-front TopMost = true; @@ -138,7 +139,7 @@ private void SplashScreen_Load(object sender, EventArgs e) _channelLabel.Visible = channel.ToLowerInvariant() != "release"; _channelLabel.Text = channel; // No need to localize this: seen only by testers or special users (BL-4451) _copyrightlabel.Text = $"© 2011-{DateTime.Now.Year} SIL Global"; - if (!Program.StartupAutomation && !Program.StartupHeadless) + if (!Program.StartupAutomation) BringToFront(); } diff --git a/src/BloomExe/WebView2Browser.cs b/src/BloomExe/WebView2Browser.cs index 638b9e74647c..d70b68c34e8b 100644 --- a/src/BloomExe/WebView2Browser.cs +++ b/src/BloomExe/WebView2Browser.cs @@ -396,11 +396,12 @@ private async Task InitWebView() { additionalBrowserArgs += " --accept-lang=" + _uiLanguageOfThisRun; } - if (Program.StartupHeadless) + if (AutomationWindowPlacement.IsOffEveryMonitor) { - // A headless run keeps Bloom's window far off-screen (see Shell.GetHeadlessBounds), - // so Windows reports the window as occluded and Chromium stops rendering it. A - // screenshot of an unrendered page comes back blank, so turn that behavior off. + // This run keeps Bloom's window far off-screen (see + // AutomationWindowPlacement.GetBoundsOffEveryMonitor), so Windows reports the + // window as occluded and Chromium stops rendering it. A screenshot of an + // unrendered page comes back blank, so turn that behavior off. featuresToDisable.Add("CalculateNativeWinOcclusion"); additionalBrowserArgs += " --disable-backgrounding-occluded-windows"; } diff --git a/src/BloomExe/web/controllers/ProblemReportApi.cs b/src/BloomExe/web/controllers/ProblemReportApi.cs index 3eded937d1ba..8b144b12fc5d 100644 --- a/src/BloomExe/web/controllers/ProblemReportApi.cs +++ b/src/BloomExe/web/controllers/ProblemReportApi.cs @@ -1173,16 +1173,18 @@ private static void TryGetScreenshot(Control controlForScreenshotting) { ResetScreenshotFile(); } - else if (IsBloomProcessInForeground() && !Program.StartupHeadless) + else if ( + IsBloomProcessInForeground() + && !AutomationWindowPlacement.IsOffEveryMonitor + ) { // Bloom is the foreground app: a plain screen copy is cheaper // and avoids re-triggering any paint-related bugs. // - // Not under --headless, though. That run's window sits far - // outside every monitor, so copying from those screen coordinates - // would save whatever the desktop has there, which is nothing. - // Render the window itself instead, the way the not-in-front case - // already does. + // Not when the window is off every monitor, though. Copying from + // those screen coordinates would save whatever the desktop has + // there, which is nothing. Render the window itself instead, the + // way the not-in-front case already does. var scaledBounds = controlForScreenshotting.Bounds; #if !__MonoCS__ scaledBounds = diff --git a/src/BloomTests/AutomationWindowPlacementTests.cs b/src/BloomTests/AutomationWindowPlacementTests.cs new file mode 100644 index 000000000000..765e23bcf23e --- /dev/null +++ b/src/BloomTests/AutomationWindowPlacementTests.cs @@ -0,0 +1,183 @@ +using System; +using System.Linq; +using Bloom; +using NUnit.Framework; +using Choice = Bloom.AutomationWindowPlacement.Choice; + +namespace BloomTests +{ + /// + /// Covers how BLOOM_AUTOMATION_MONITOR is read. These call + /// AutomationWindowPlacement.Parse directly, so they need neither the environment variable + /// nor a machine with a particular set of monitors: the number of monitors is an argument. + /// + [TestFixture] + public class AutomationWindowPlacementTests + { + /// + /// ParseStartupPortArguments writes into Program statics that live for the rest of the + /// test run; re-parse empty args after each test to restore the defaults (the method + /// resets them all on entry). Same rationale as the ProgramTests TearDown. + /// + [TearDown] + public void TearDown() + { + Program.ParseStartupPortArguments(Array.Empty(), out _); + Environment.SetEnvironmentVariable(AutomationWindowPlacement.VariableName, null); + } + + [TestCase(null, TestName = "Parse_VariableNotSet_PlacesWindowsNormally")] + [TestCase("", TestName = "Parse_VariableEmpty_PlacesWindowsNormally")] + [TestCase(" ", TestName = "Parse_VariableAllSpaces_PlacesWindowsNormally")] + public void Parse_NothingAsked_PlacesWindowsNormally(string setting) + { + Assert.That( + AutomationWindowPlacement.Parse(setting, 3, out var monitor), + Is.EqualTo(Choice.AsBloomNormallyWould) + ); + Assert.That(monitor, Is.EqualTo(0)); + } + + [TestCase("headless")] + [TestCase("HEADLESS")] + [TestCase("Headless")] + [TestCase(" headless ")] + public void Parse_Headless_GoesOffEveryMonitor(string setting) + { + Assert.That( + AutomationWindowPlacement.Parse(setting, 3, out var monitor), + Is.EqualTo(Choice.OffEveryMonitor) + ); + Assert.That(monitor, Is.EqualTo(0), "No monitor was chosen, so none is reported."); + } + + [TestCase("1", 1)] + [TestCase("2", 2)] + [TestCase("3", 3)] + [TestCase(" 2 ", 2)] + public void Parse_MonitorThatExists_GoesOnThatMonitor(string setting, int expected) + { + Assert.That( + AutomationWindowPlacement.Parse(setting, 3, out var monitor), + Is.EqualTo(Choice.OnTheChosenMonitor) + ); + Assert.That(monitor, Is.EqualTo(expected)); + } + + /// + /// A monitor this machine does not have, a zero or negative index, and a typo all mean the + /// same thing: nobody asked for anything Bloom can honour, so it places its windows + /// normally and the developer sees the window. See the remarks on Parse. + /// + [TestCase("4", TestName = "Parse_MonitorBeyondTheLast_PlacesWindowsNormally")] + [TestCase("0", TestName = "Parse_ZeroMonitor_PlacesWindowsNormally")] + [TestCase("-1", TestName = "Parse_NegativeMonitor_PlacesWindowsNormally")] + [TestCase("headles", TestName = "Parse_HeadlessMisspelt_PlacesWindowsNormally")] + [TestCase("true", TestName = "Parse_Nonsense_PlacesWindowsNormally")] + [TestCase("2.5", TestName = "Parse_NotAWholeNumber_PlacesWindowsNormally")] + public void Parse_UnusableValue_PlacesWindowsNormally(string setting) + { + Assert.That( + AutomationWindowPlacement.Parse(setting, 3, out var monitor), + Is.EqualTo(Choice.AsBloomNormallyWould) + ); + Assert.That(monitor, Is.EqualTo(0)); + } + + [Test] + public void Parse_OnlyOneMonitor_TakesTheFirstAndRefusesTheSecond() + { + Assert.That( + AutomationWindowPlacement.Parse("1", 1, out _), + Is.EqualTo(Choice.OnTheChosenMonitor) + ); + Assert.That( + AutomationWindowPlacement.Parse("2", 1, out _), + Is.EqualTo(Choice.AsBloomNormallyWould), + "Monitor 2 does not exist on a one-monitor machine." + ); + } + + /// + /// The variable is only for automation runs. A developer who leaves it set in their shell + /// must still get an ordinary, visible Bloom when they start one themselves. + /// + [Test] + public void GetChoice_WithoutTheAutomationFlag_IgnoresTheVariable() + { + Environment.SetEnvironmentVariable( + AutomationWindowPlacement.VariableName, + AutomationWindowPlacement.HeadlessSetting + ); + + Program.ParseStartupPortArguments(Array.Empty(), out var errorMessage); + Assert.That(errorMessage, Is.Null); + Assert.That( + Program.StartupAutomation, + Is.False, + "Sanity check: this test needs a run that is NOT automation." + ); + + Assert.That( + AutomationWindowPlacement.GetChoice(), + Is.EqualTo(Choice.AsBloomNormallyWould) + ); + Assert.That(AutomationWindowPlacement.IsOffEveryMonitor, Is.False); + } + + [Test] + public void GetChoice_WithTheAutomationFlag_ObeysTheVariable() + { + Environment.SetEnvironmentVariable( + AutomationWindowPlacement.VariableName, + AutomationWindowPlacement.HeadlessSetting + ); + + Program.ParseStartupPortArguments(new[] { "--automation" }, out var errorMessage); + Assert.That(errorMessage, Is.Null); + Assert.That(Program.StartupAutomation, Is.True, "Sanity check."); + + Assert.That(AutomationWindowPlacement.GetChoice(), Is.EqualTo(Choice.OffEveryMonitor)); + Assert.That(AutomationWindowPlacement.IsOffEveryMonitor, Is.True); + } + + /// + /// The window has to end up wholly off every monitor, and at a coordinate Windows will + /// honour. This runs against however many monitors the machine has, which is the point: + /// the answer has to hold on the CI runner's one screen and on a developer's several. + /// + [Test] + public void GetBoundsOffEveryMonitor_IsWhollyOffEveryMonitorAndWithinWindowsLimit() + { + var bounds = AutomationWindowPlacement.GetBoundsOffEveryMonitor(); + + Assert.That(bounds.Width, Is.GreaterThan(0), "Sanity check: a real size."); + Assert.That(bounds.Height, Is.GreaterThan(0), "Sanity check: a real size."); + Assert.That( + bounds.Left, + Is.GreaterThanOrEqualTo(-32000), + "Windows does not honour a position further left than this." + ); + foreach (var screen in System.Windows.Forms.Screen.AllScreens) + { + Assert.That( + bounds.IntersectsWith(screen.Bounds), + Is.False, + $"The window overlaps the monitor at {screen.Bounds}." + ); + } + + // The desktop can make the window as much as four times as wide as this process asked + // for, because Windows scales a monitor by at most 400%. So a clearance of one width + // is not enough: measure it, and require four. See GetBoundsOffEveryMonitor. + var leftmostX = System.Windows.Forms.Screen.AllScreens.Min(screen => + screen.Bounds.Left + ); + Assert.That( + leftmostX - bounds.Left, + Is.GreaterThanOrEqualTo(bounds.Width * 4), + "A window four times this wide would still have to clear the leftmost monitor." + ); + } + } +} diff --git a/src/BloomTests/ProgramTests.cs b/src/BloomTests/ProgramTests.cs index 4dd021366f6c..f4c68c636f68 100644 --- a/src/BloomTests/ProgramTests.cs +++ b/src/BloomTests/ProgramTests.cs @@ -56,33 +56,6 @@ out var automationErrorMessage Assert.That(Program.StartupRequestedPortSummary, Is.EqualTo("automation=true")); } - [Test] - public void ParseStartupPortArguments_StoresHeadlessFlag() - { - var remainingArgs = Program.ParseStartupPortArguments( - new[] { "--automation", "--headless", @"C:\Temp\Example.bloomcollection" }, - out var errorMessage - ); - - Assert.That(errorMessage, Is.Null); - Assert.That(Program.StartupHeadless, Is.True); - Assert.That(Program.StartupAutomation, Is.True); - Assert.That( - Program.StartupRequestedPortSummary, - Is.EqualTo("automation=true, headless=true") - ); - Assert.That(remainingArgs, Is.EqualTo(new[] { @"C:\Temp\Example.bloomcollection" })); - } - - [Test] - public void ParseStartupPortArguments_LeavesHeadlessFalseWithoutTheFlag() - { - Program.ParseStartupPortArguments(new[] { "--automation" }, out var errorMessage); - - Assert.That(errorMessage, Is.Null); - Assert.That(Program.StartupHeadless, Is.False); - } - [Test] public void ParseStartupPortArguments_VitePortAloneDoesNotEnableAutomation() { From 55a6623db3f242058308e1a5394d120679b9ae62 Mon Sep 17 00:00:00 2001 From: Hatton Date: Thu, 3 Sep 2026 07:20:46 -0600 Subject: [PATCH 3/8] Ask the nightly e2e run for off-screen windows (BL-16804) Before this branch the Playwright fixture passed --headless on every launch, so the nightly ran off-screen. Removing that flag left nothing in its place, so the nightly would have started opening a window without anybody choosing that. The Run BloomE2E tests step now sets BLOOM_AUTOMATION_MONITOR=headless. Devin raised the consequence. It suggested making off-screen the default in the fixture instead, which is declined: a developer's own run showing a window is the point of the change. The workflow is the one caller that cannot ask for what it wants at the moment it runs. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/nightly.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 1580914ccfd6..d5e120d1eeb5 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -273,6 +273,12 @@ jobs: # The html reporter would otherwise try to serve the report at the end of a # failing run, which on a runner means a step that never returns. PLAYWRIGHT_HTML_OPEN: never + # Put every window this run opens off every monitor. Bloom obeys this variable + # only under --automation, which is what the fixture launches; see + # src/BloomE2E/README.md. The runner has a desktop, so a visible window would + # also work, but asking for it here says what the run needs instead of leaving + # it to whatever the runner happens to do. + BLOOM_AUTOMATION_MONITOR: headless run: pnpm test --reporter=list,junit,html # What explains an e2e failure: Playwright's HTML report, plus the trace and failure From a6bde378876045ca0a021497aa7965605dae6d42 Mon Sep 17 00:00:00 2001 From: Hatton Date: Thu, 3 Sep 2026 07:52:31 -0600 Subject: [PATCH 4/8] Number the monitors left to right, and let 0 mean headless BLOOM_AUTOMATION_MONITOR took a 1-based index into Screen.AllScreens, which is the order of the display drivers. That order means nothing a developer can see: on a three-monitor machine it was 1 (right), 2 (left), 3 (primary, centre), while Windows Settings printed 1 (primary, centre), 2 (right), 3 (left). So a developer who read a number off Windows Settings got a different monitor, and nothing said so. Windows Settings' own numbers cannot be reproduced. QueryDisplayConfig with QDC_ONLY_ACTIVE_PATHS returns the paths in exactly the AllScreens order, the \.\DISPLAY device names do not match either, and Microsoft documents no rule. So count left to right instead: monitor 1 is the leftmost. That follows the arrangement picture in Windows Settings, which a developer can read, even though it is not the numbers printed on it. Also accept "0" as a synonym for "headless". It reads as "no monitor" beside the numbers that name one, and it is quicker to type. Bloom now writes the whole mapping to its log on every automation start, so a developer can see which monitor a number means rather than guess. Co-Authored-By: Claude Opus 5 (1M context) --- .github/skills/add-e2e-test/SKILL.md | 12 +-- src/BloomE2E/README.md | 22 +++++- src/BloomE2E/fixtures/launchBloom.ts | 7 +- src/BloomExe/AutomationWindowPlacement.cs | 76 ++++++++++++++++++- src/BloomExe/Shell.cs | 7 ++ .../AutomationWindowPlacementTests.cs | 62 ++++++++++++++- 6 files changed, 170 insertions(+), 16 deletions(-) diff --git a/.github/skills/add-e2e-test/SKILL.md b/.github/skills/add-e2e-test/SKILL.md index 569c5a394377..d1fafb90738e 100644 --- a/.github/skills/add-e2e-test/SKILL.md +++ b/.github/skills/add-e2e-test/SKILL.md @@ -340,12 +340,14 @@ pnpm exec playwright test -g "switching workspace tabs" # one test by title A run launches a real Bloom and its window appears on the developer's desktop, unless `BLOOM_AUTOMATION_MONITOR` says otherwise. That one variable decides where every window a run -opens goes, the splash screen included: `headless` puts them all off every monitor, a 1-based -monitor number puts them on that monitor, and any other value, unset included, leaves Bloom to -place them as it always does. `headless` moves the window off-screen rather than minimizing it, +opens goes, the splash screen included: `headless`, or `0`, puts them all off every monitor; a +1-based monitor number puts them on that monitor; and any other value, unset included, leaves Bloom +to place them as it always does. `headless` moves the window off-screen rather than minimizing it, because WebView2 stops painting a minimized window and every screenshot then comes back blank. -`--debug` clears a `headless` setting, so a debug session has a window to step through. See -`src/BloomE2E/README.md` for the table. +`--debug` clears a `headless` setting, so a debug session has a window to step through. The monitor +number counts left to right, so 1 is the leftmost monitor; it is **not** the number Windows Settings +shows beside each display, and no API reproduces those. Bloom writes the whole mapping to its log at +startup. See `src/BloomE2E/README.md` for the table and that log line. A run needs a built `Bloom.exe` under `output/{Debug,Release}/{x64,AnyCPU,}/` (build it yourself; see "Build Bloom whenever it helps") and the inputs at `output/testing-inputs`. Point diff --git a/src/BloomE2E/README.md b/src/BloomE2E/README.md index 3e3fe47d6e97..0ae3d8ec4c19 100644 --- a/src/BloomE2E/README.md +++ b/src/BloomE2E/README.md @@ -144,13 +144,14 @@ the splash screen alike. Set it in your shell, or per run: | Value | What happens | | --- | --- | -| `headless` | Every window opens far outside every monitor. You see nothing, so a run can go on while you work. | -| a 1-based monitor number | Every window opens on that monitor, so a run stays off the one you are working on. | +| `headless`, or `0` | Every window opens far outside every monitor. You see nothing, so a run can go on while you work. | +| a 1-based monitor number, counted left to right | Every window opens on that monitor, so a run stays off the one you are working on. | | unset, or anything else | Bloom places its windows as it always does, and you see the run. | ```bash BLOOM_AUTOMATION_MONITOR=headless pnpm test # see nothing -BLOOM_AUTOMATION_MONITOR=2 pnpm test # on the second monitor +BLOOM_AUTOMATION_MONITOR=0 pnpm test # the same thing: no monitor at all +BLOOM_AUTOMATION_MONITOR=2 pnpm test # on the second monitor from the left pnpm test # wherever Bloom normally opens ``` @@ -158,6 +159,21 @@ A value Bloom cannot use, a typo or a monitor you do not have, counts as "anythi a visible window, which is exactly what tells you the variable did not take effect. A setting that hid the window on a typo would leave you nothing to notice. +**The number counts left to right, which is not the number Windows Settings shows.** Monitor 1 is +your leftmost monitor, 2 the next one to the right, and so on, matching the arrangement picture in +Windows Settings but not the numbers printed on it. Windows does not document how the Settings app +makes those numbers, and no API reproduces them: on one three-monitor machine Windows Settings said +1 (primary, centre), 2 (right) and 3 (left), while left to right is 1 (left), 2 (primary, centre) +and 3 (right). So read the arrangement, not the numbers on it. Bloom also writes the whole mapping +to `%TEMP%\SIL\Bloom\Log.txt` on every automation start: + +``` +BLOOM_AUTOMATION_MONITOR='2': every window goes on the monitor at {X=0,Y=0,Width=2560,Height=1440}. +The monitors this process sees, numbered left to right as this variable numbers them (which is NOT +how Windows Settings numbers them): 1=(-1920,601) 1920x1200, 2=(0,0) 2560x1440 primary, +3=(3840,432) 1920x1200. +``` + `headless` moves the window off-screen rather than minimizing or hiding it, because WebView2 stops painting a minimized window, which would make every screenshot blank. Off-screen the window paints exactly as it would in front of you, so rendering and keyboard input behave the same. diff --git a/src/BloomE2E/fixtures/launchBloom.ts b/src/BloomE2E/fixtures/launchBloom.ts index 4535ed63a24c..7fb7c640626f 100644 --- a/src/BloomE2E/fixtures/launchBloom.ts +++ b/src/BloomE2E/fixtures/launchBloom.ts @@ -166,12 +166,13 @@ const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); * does. So the child inherits this process's environment untouched, with one exception. * * The exception is Playwright's --debug (which sets PWDEBUG): stepping through a test whose window - * nobody can see is pointless, so a debug session clears a "headless" setting and gets a window. - * A setting that names a monitor is left alone, because that window IS visible. + * nobody can see is pointless, so a debug session clears a "headless" setting, or the "0" that says + * the same thing, and gets a window. A setting that names a monitor is left alone, because that + * window IS visible. */ function environmentForBloom(): NodeJS.ProcessEnv { const asked = process.env.BLOOM_AUTOMATION_MONITOR?.trim().toLowerCase(); - if (process.env.PWDEBUG && asked === "headless") { + if (process.env.PWDEBUG && (asked === "headless" || asked === "0")) { return { ...process.env, BLOOM_AUTOMATION_MONITOR: "" }; } return process.env; diff --git a/src/BloomExe/AutomationWindowPlacement.cs b/src/BloomExe/AutomationWindowPlacement.cs index 16da9a04b1b7..ce48e83d22e1 100644 --- a/src/BloomExe/AutomationWindowPlacement.cs +++ b/src/BloomExe/AutomationWindowPlacement.cs @@ -9,9 +9,9 @@ namespace Bloom /// What an automation run (--automation, e.g. the Playwright suites) does with the windows it /// opens. One environment variable decides it, BLOOM_AUTOMATION_MONITOR: /// - /// "2" (any 1-based index into Screen.AllScreens) every window opens on that monitor - /// "headless" every window opens off every monitor - /// absent, empty, or anything else Bloom places its windows as it always does + /// "2" (a 1-based monitor number, counted left to right) every window opens on that monitor + /// "headless", or "0" for no monitor at all every window opens off every monitor + /// absent, empty, or anything else Bloom places windows as it always does /// /// The variable applies ONLY under --automation. A developer who leaves it set in their shell /// therefore still gets an ordinary, visible Bloom when they start one themselves; only a run @@ -31,6 +31,9 @@ public static class AutomationWindowPlacement /// The value of BLOOM_AUTOMATION_MONITOR that asks for off-screen windows. public const string HeadlessSetting = "headless"; + /// The number that says the same thing as HeadlessSetting: no monitor at all. + public const string NoMonitorSetting = "0"; + public const string VariableName = "BLOOM_AUTOMATION_MONITOR"; /// Where an automation run puts its windows. @@ -87,6 +90,10 @@ public static Choice Parse(string setting, int screenCount, out int oneBasedMoni return Choice.AsBloomNormallyWould; if (trimmed.Equals(HeadlessSetting, StringComparison.OrdinalIgnoreCase)) return Choice.OffEveryMonitor; + // "0" says the same thing as "headless": no monitor at all. It reads as "none" beside + // the numbers that name a monitor, and it is quicker to type. + if (trimmed == NoMonitorSetting) + return Choice.OffEveryMonitor; if (int.TryParse(trimmed, out var index) && index >= 1 && index <= screenCount) { oneBasedMonitor = index; @@ -101,6 +108,32 @@ public static Choice Parse(string setting, int screenCount, out int oneBasedMoni /// public static bool IsOffEveryMonitor => GetChoice() == Choice.OffEveryMonitor; + /// + /// This machine's monitors in the order the variable numbers them: left to right, so + /// monitor 1 is the leftmost. Two monitors at the same horizontal position, one above the + /// other, come out top first. + /// + /// The order is by position rather than the order Screen.AllScreens happens to return, + /// which is the order of the display drivers and means nothing a developer can see. A + /// developer reads a number off the picture of their monitors, so the number has to follow + /// the picture. + /// + /// This is still NOT the number Windows Settings prints on each monitor. Windows does not + /// document how the Settings app makes those labels, and neither the AllScreens order nor + /// the \\.\DISPLAY<n> device name nor the display-config path order reproduces them: on + /// one three-monitor machine Windows Settings said 1 (primary, centre), 2 (right) and + /// 3 (left), while left to right is 1 (left), 2 (primary, centre) and 3 (right). So Bloom + /// counts left to right, which a developer can work out from the arrangement they see, + /// and DescribeChoice writes the whole mapping to the log. + /// + public static Screen[] MonitorsLeftToRight() + { + return Screen + .AllScreens.OrderBy(screen => screen.Bounds.Left) + .ThenBy(screen => screen.Bounds.Top) + .ToArray(); + } + /// /// The monitor an automation run opens its windows on. Only meaningful when the choice is /// OnTheChosenMonitor; falls back to the primary screen otherwise, so that callers wanting @@ -117,11 +150,46 @@ out var oneBasedMonitor ) == Choice.OnTheChosenMonitor ) { - return Screen.AllScreens[oneBasedMonitor - 1]; + return MonitorsLeftToRight()[oneBasedMonitor - 1]; } return Screen.PrimaryScreen; } + /// + /// One line for the log saying what the variable said and what this run did with it, + /// naming every monitor by position and size. + /// + /// This exists because the number in the variable counts left to right, which is NOT the + /// number Windows Settings prints beside each display; see MonitorsLeftToRight. A developer + /// who reads a number off Windows Settings therefore gets a different monitor, and nothing + /// on screen says so. The log line is what lets them see which monitor Bloom actually + /// chose, and work out the number they want. + /// + public static string DescribeChoice() + { + var raw = Environment.GetEnvironmentVariable(VariableName); + var monitors = string.Join( + ", ", + MonitorsLeftToRight() + .Select( + (screen, zeroBased) => + $"{zeroBased + 1}=({screen.Bounds.X},{screen.Bounds.Y}) " + + $"{screen.Bounds.Width}x{screen.Bounds.Height}" + + (screen.Primary ? " primary" : "") + ) + ); + var what = GetChoice() switch + { + Choice.OffEveryMonitor => "every window goes off every monitor", + Choice.OnTheChosenMonitor => + $"every window goes on the monitor at {GetChosenMonitor().Bounds}", + _ => "Bloom places its windows as it always does", + }; + return $"{VariableName}={(raw == null ? "(not set)" : $"'{raw}'")}: {what}. " + + $"The monitors this process sees, numbered left to right as this variable " + + $"numbers them (which is NOT how Windows Settings numbers them): {monitors}."; + } + /// /// Where an off-every-monitor run puts a window: the size of the primary screen's working /// area, positioned to the left of every monitor, so that not one pixel of it is on any diff --git a/src/BloomExe/Shell.cs b/src/BloomExe/Shell.cs index 9a9bfb4fe9e4..fed313bf7b55 100644 --- a/src/BloomExe/Shell.cs +++ b/src/BloomExe/Shell.cs @@ -446,6 +446,13 @@ private void Shell_Load(object sender, EventArgs e) // through to the ordinary cases below, window placement and all, so the // developer sees the Bloom they would see without the variable. var placement = AutomationWindowPlacement.GetChoice(); + if (Program.StartupAutomation) + { + // Say in the log which monitor this run chose and what the alternatives were. + // The number in the variable is not the number Windows Settings shows; see + // AutomationWindowPlacement.DescribeChoice. + Logger.WriteEvent(AutomationWindowPlacement.DescribeChoice()); + } if (placement == AutomationWindowPlacement.Choice.OffEveryMonitor) { // The window goes off every monitor and out of the task bar, so a test can diff --git a/src/BloomTests/AutomationWindowPlacementTests.cs b/src/BloomTests/AutomationWindowPlacementTests.cs index 765e23bcf23e..4445e9e12bd0 100644 --- a/src/BloomTests/AutomationWindowPlacementTests.cs +++ b/src/BloomTests/AutomationWindowPlacementTests.cs @@ -42,6 +42,8 @@ public void Parse_NothingAsked_PlacesWindowsNormally(string setting) [TestCase("HEADLESS")] [TestCase("Headless")] [TestCase(" headless ")] + [TestCase("0", TestName = "Parse_ZeroMonitor_GoesOffEveryMonitor")] + [TestCase(" 0 ", TestName = "Parse_ZeroMonitorWithSpaces_GoesOffEveryMonitor")] public void Parse_Headless_GoesOffEveryMonitor(string setting) { Assert.That( @@ -70,7 +72,6 @@ public void Parse_MonitorThatExists_GoesOnThatMonitor(string setting, int expect /// normally and the developer sees the window. See the remarks on Parse. /// [TestCase("4", TestName = "Parse_MonitorBeyondTheLast_PlacesWindowsNormally")] - [TestCase("0", TestName = "Parse_ZeroMonitor_PlacesWindowsNormally")] [TestCase("-1", TestName = "Parse_NegativeMonitor_PlacesWindowsNormally")] [TestCase("headles", TestName = "Parse_HeadlessMisspelt_PlacesWindowsNormally")] [TestCase("true", TestName = "Parse_Nonsense_PlacesWindowsNormally")] @@ -141,6 +142,65 @@ public void GetChoice_WithTheAutomationFlag_ObeysTheVariable() Assert.That(AutomationWindowPlacement.IsOffEveryMonitor, Is.True); } + /// + /// The log line is the only place a developer can see which monitor a number means, because + /// the number is not the one Windows Settings shows. So it has to name every monitor, and it + /// has to say that the numbering differs. + /// + [Test] + public void DescribeChoice_NamesEveryMonitorAndWarnsAboutTheNumbering() + { + Environment.SetEnvironmentVariable( + AutomationWindowPlacement.VariableName, + AutomationWindowPlacement.HeadlessSetting + ); + Program.ParseStartupPortArguments(new[] { "--automation" }, out _); + + var line = AutomationWindowPlacement.DescribeChoice(); + + Assert.That(line, Does.Contain(AutomationWindowPlacement.VariableName)); + Assert.That(line, Does.Contain("off every monitor")); + Assert.That( + line, + Does.Contain("NOT how Windows Settings"), + "A developer reading a number off Windows Settings has to be warned." + ); + var screens = AutomationWindowPlacement.MonitorsLeftToRight(); + for (var oneBased = 1; oneBased <= screens.Length; oneBased++) + { + Assert.That( + line, + Does.Contain($"{oneBased}=({screens[oneBased - 1].Bounds.X},"), + $"Monitor {oneBased} is missing from the log line." + ); + } + } + + /// + /// Monitor 1 is the leftmost monitor, whatever order Screen.AllScreens returns. This runs + /// against however many monitors the machine has, so it says nothing on the CI runner's + /// single screen and everything on a developer's several. + /// + [Test] + public void MonitorsLeftToRight_AreOrderedByPosition() + { + var monitors = AutomationWindowPlacement.MonitorsLeftToRight(); + + Assert.That( + monitors.Length, + Is.EqualTo(System.Windows.Forms.Screen.AllScreens.Length), + "Every monitor has to be there; the order is all that changes." + ); + for (var next = 1; next < monitors.Length; next++) + { + Assert.That( + monitors[next].Bounds.Left, + Is.GreaterThanOrEqualTo(monitors[next - 1].Bounds.Left), + $"Monitor {next + 1} is left of monitor {next}." + ); + } + } + /// /// The window has to end up wholly off every monitor, and at a coordinate Windows will /// honour. This runs against however many monitors the machine has, which is the point: From b40c65ae98bfc48b999b1e860ce327512f3c9edc Mon Sep 17 00:00:00 2001 From: Hatton Date: Thu, 3 Sep 2026 08:19:32 -0600 Subject: [PATCH 5/8] Put the off-screen window below the primary, not left of it An off-screen window asked for the primary monitor's working-area size, but Windows gives a window the scale factor of the monitor nearest to it. Out to the left, the nearest monitor is the leftmost one, whose scale factor is very likely not the primary's. On a machine with a 150% primary and a 100% monitor beside it, a window meant to match the primary's 3840x2100 came out 3840x2100 REAL pixels, taller than any monitor on the machine, and the page inside it laid out at a viewport height of 1990 CSS pixels that no user could ever have. format-gear-positioning.spec.ts failed on exactly that: it asserts the format gear sits in the lower half of the view, and the gear landed at 846 of 1990. The file passes on monitor 2 (150%) and on monitor 3 (100%) and failed only off-screen, on the same build, in back-to-back runs, which is what isolated it. So the window now goes straight down from the primary monitor, in line with its left edge. The primary stays the nearest monitor, so an off-screen window paints at the size a visible one would. The clearance rule is unchanged in substance: four window heights below the lowest monitor, plus 1000 pixels, clamped to 32000, for the same reason the old rule left four widths. This is the second bug from one cause. The first ate all but 27 pixels of a 1000-pixel cushion. Both passed every unit test, because a unit test compares numbers inside one process's own coordinate space, so AUTOMATION-DEBT.md now carries an entry asking for the ability to run the suite at a chosen resolution and scale factor. The full suite off-screen is back to 23 passed, 2 failed, 10 did not run, the two failures being the pre-existing pair on BL-16807. Co-Authored-By: Claude Opus 5 (1M context) --- src/BloomE2E/AUTOMATION-DEBT.md | 38 +++++++++++ src/BloomE2E/README.md | 8 +++ src/BloomExe/AutomationWindowPlacement.cs | 64 +++++++++++-------- .../AutomationWindowPlacementTests.cs | 24 ++++--- 4 files changed, 98 insertions(+), 36 deletions(-) diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index f489f060e2d8..e2d761b9f8ae 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -287,3 +287,41 @@ the file said `en` again a moment later. The same test has to restore the zoom i that setting is shared too. Fix direction: under `--e2e`, point the settings provider at a per-instance folder (a sibling of the temp collection would do), so a test's Bloom starts from defaults and its changes die with it. + +## No way to run the suite at a chosen monitor resolution and scale factor + +Every run takes the resolution and the scale factor of whatever monitor it lands on, so a +suite proves the layout only at the DPI of the machine that ran it. That is exactly where a +class of Bloom bugs lives: a control that fits at 100% and overlaps at 150%, a dialog that +opens off the edge on a short screen, a size computed in one coordinate space and used in +another. A developer at 150% and a CI runner at 100% each pass while the other's bug goes +unseen, and neither can reproduce what a user reports. + +Found 2026-09-03, twice in one change (BL-16804), which is what makes this worth scheduling: + +- The off-screen window asked for the primary monitor's working-area size, and Windows + interpreted that size at the scale factor of the nearest monitor. On a machine with a 150% + primary and a 100% monitor beside it, a window meant to be 3840x2100 came out 3840x2100 real + pixels, taller than any monitor on the machine. `format-gear-positioning.spec.ts` failed + because the page viewport was 1990 CSS pixels high, a size no user has. The same mismatch, + in its first guise, had eaten all but 27 pixels of a 1000-pixel off-screen cushion. +- Both bugs passed every unit test, because a unit test compares numbers inside one process's + own coordinate space. Only a real window at a real scale factor shows them. + +Fix direction, cheapest first, none of it tried yet: + +- **An RDP session to the machine.** An `.rdp` file takes `desktopwidth`, `desktopheight` and + `desktopscalefactor` (100, 125, 150, 175, 200), so one connection per combination gives a + real desktop at a chosen scale with no driver to install. This looks like the least work and + the most likely to run in CI, but nobody has tried driving the suite inside one. +- **A virtual display driver.** Windows has an indirect-display driver model (IddCx), and + several drivers built on it create a monitor with no hardware behind it, at a resolution the + driver is told to offer. Setting that monitor's *scale factor* is the harder half: Windows + exposes per-monitor scale only through display-config calls Microsoft does not document. + Worth an afternoon of investigation before committing to it. +- **A virtual machine or Windows Sandbox** at a chosen resolution and scale. Heaviest, but it + is the only one that also isolates the shared `user.config` described above. + +Whatever the mechanism, the suite needs the same thing from it: a way to say "run these tests +at 1920x1080 at 150%" and have the run either honour it or refuse, rather than silently using +the desktop it found. diff --git a/src/BloomE2E/README.md b/src/BloomE2E/README.md index 0ae3d8ec4c19..c7f58794d51d 100644 --- a/src/BloomE2E/README.md +++ b/src/BloomE2E/README.md @@ -178,6 +178,14 @@ how Windows Settings numbers them): 1=(-1920,601) 1920x1200, 2=(0,0) 2560x1440 p painting a minimized window, which would make every screenshot blank. Off-screen the window paints exactly as it would in front of you, so rendering and keyboard input behave the same. +It goes **below** your primary monitor, not off to one side, and that matters on a machine whose +monitors run at different scale factors. Windows gives a window the scale factor of the monitor +nearest to it. A window out to the left would take the leftmost monitor's scale factor while +carrying a size measured in the primary's, and on a 150% primary beside a 100% monitor that made a +window 3840x2100 real pixels, taller than any monitor on the machine, with a page viewport no user +could have. Directly below the primary, the primary stays the nearest monitor and the size is +right. See `AutomationWindowPlacement.GetBoundsOffEveryMonitor`. + `--debug` clears a `headless` setting for you: stepping through a test whose window you cannot see is pointless. A setting that names a monitor is left alone, because that window is visible anyway. diff --git a/src/BloomExe/AutomationWindowPlacement.cs b/src/BloomExe/AutomationWindowPlacement.cs index ce48e83d22e1..be2a9f6bc152 100644 --- a/src/BloomExe/AutomationWindowPlacement.cs +++ b/src/BloomExe/AutomationWindowPlacement.cs @@ -192,8 +192,8 @@ public static string DescribeChoice() /// /// Where an off-every-monitor run puts a window: the size of the primary screen's working - /// area, positioned to the left of every monitor, so that not one pixel of it is on any - /// screen. + /// area, positioned below every monitor and in line with the primary, so that not one + /// pixel of it is on any screen and it still gets the primary's scale factor. /// /// The window is moved rather than minimized or hidden because a minimized WebView2 stops /// painting: screenshots come back blank and the layout is the wrong size. An off-screen @@ -201,36 +201,46 @@ public static string DescribeChoice() /// public static Rectangle GetBoundsOffEveryMonitor() { - var size = Screen.PrimaryScreen.WorkingArea.Size; - // Two bounds, and the window has to respect both. + var workingArea = Screen.PrimaryScreen.WorkingArea; + var size = workingArea.Size; + + // The window goes straight DOWN from the primary monitor, not off to the left, and the + // reason is the DPI of the monitor Windows thinks the window is on. // - // The first is the leftmost monitor: the window's right edge has to be left of it, or - // part of the window shows. Both the position and the width above are in the - // coordinates this process sees, and the desktop does not always agree with them: on - // a machine whose primary monitor is scaled to 160%, Bloom asked for a window 1587 - // pixels wide and Windows made one 2560 pixels wide, which ate all but 27 pixels of a - // 1000-pixel cushion. So leave room for the whole error rather than a fixed number of - // pixels: Windows scales a monitor by at most 400%, so a window this process believes - // is W wide covers at most 4W pixels of the desktop. Four widths to the left of the - // leftmost monitor therefore clears it whatever the scale factors are, and the - // 1000 pixels on top of that keep the two edges from meeting exactly. + // Windows gives a window the scale factor of the monitor nearest to it, and this + // process asked for a size in the primary monitor's own scaled pixels. Put the window + // out to the left and the nearest monitor is the leftmost one, whose scale factor is + // very likely not the primary's: on a machine whose primary runs at 150% and whose + // left monitor runs at 100%, a window meant to match the primary's 3840x2100 came out + // 3840x2100 REAL pixels, taller than any monitor on the machine, and the page inside + // it laid out at a viewport height no user could ever have. Keeping the window + // directly under the primary, aligned with its left edge, keeps the primary the + // nearest monitor, so an off-screen window paints at the size a visible one would. // - // The second is -32000, as far left as a window may go: Windows still places a window - // there, and anything beyond about -32768 runs into the 16-bit coordinates that some - // of the older window messages still carry. + // How far down: far enough that the window clears every monitor even after Windows + // scales it. This process's idea of the height can be out by the ratio of two scale + // factors, and Windows scales a monitor by at most 400%, so a window this process + // believes is H high covers at most 4H pixels of the desktop. Four heights below the + // lowest monitor therefore clears them all, and the 1000 pixels on top of that keep + // the two edges from meeting exactly. (An earlier version of this went left and left + // one window width of room; on a 160% primary that left 27 pixels of a 1000-pixel + // cushion, which is how the ratio came to be measured rather than guessed.) // - // On any real layout the first bound gives several thousand pixels to the left, well - // inside the second. A leftward run of monitors more than about 30000 pixels wide - // would need a position that satisfies neither, and then the limit wins: a window at - // a coordinate Windows will not honour is worse than one that overlaps a monitor. - const int farLeftWindowsAllows = -32000; + // 32000 is as far down as a window may go: Windows still places a window there, and + // anything past about 32768 runs into the 16-bit coordinates that some of the older + // window messages still carry. On any real layout four heights is several thousand + // pixels, well inside that. A downward run of monitors more than about 30000 pixels + // tall would need a position that satisfies neither, and then the limit wins: a + // window at a coordinate Windows will not honour is worse than one that overlaps a + // monitor. + const int farDownWindowsAllows = 32000; const int largestScaleFactorWindowsAllows = 4; - var leftmostX = Screen.AllScreens.Min(screen => screen.Bounds.Left); - var x = Math.Max( - farLeftWindowsAllows, - leftmostX - (size.Width * largestScaleFactorWindowsAllows) - 1000 + var lowestY = Screen.AllScreens.Max(screen => screen.Bounds.Bottom); + var y = Math.Min( + farDownWindowsAllows, + lowestY + (size.Height * largestScaleFactorWindowsAllows) + 1000 ); - return new Rectangle(x, 0, size.Width, size.Height); + return new Rectangle(workingArea.X, y, size.Width, size.Height); } } } diff --git a/src/BloomTests/AutomationWindowPlacementTests.cs b/src/BloomTests/AutomationWindowPlacementTests.cs index 4445e9e12bd0..ea9bc4bac2f4 100644 --- a/src/BloomTests/AutomationWindowPlacementTests.cs +++ b/src/BloomTests/AutomationWindowPlacementTests.cs @@ -213,10 +213,16 @@ public void GetBoundsOffEveryMonitor_IsWhollyOffEveryMonitorAndWithinWindowsLimi Assert.That(bounds.Width, Is.GreaterThan(0), "Sanity check: a real size."); Assert.That(bounds.Height, Is.GreaterThan(0), "Sanity check: a real size."); + Assert.That( + bounds.Top, + Is.LessThanOrEqualTo(32000), + "Windows does not honour a position further down than this." + ); Assert.That( bounds.Left, - Is.GreaterThanOrEqualTo(-32000), - "Windows does not honour a position further left than this." + Is.EqualTo(System.Windows.Forms.Screen.PrimaryScreen.WorkingArea.X), + "The window has to line up with the primary monitor, or Windows gives it another " + + "monitor's scale factor. See GetBoundsOffEveryMonitor." ); foreach (var screen in System.Windows.Forms.Screen.AllScreens) { @@ -227,16 +233,16 @@ public void GetBoundsOffEveryMonitor_IsWhollyOffEveryMonitorAndWithinWindowsLimi ); } - // The desktop can make the window as much as four times as wide as this process asked - // for, because Windows scales a monitor by at most 400%. So a clearance of one width + // The desktop can make the window as much as four times as high as this process asked + // for, because Windows scales a monitor by at most 400%. So a clearance of one height // is not enough: measure it, and require four. See GetBoundsOffEveryMonitor. - var leftmostX = System.Windows.Forms.Screen.AllScreens.Min(screen => - screen.Bounds.Left + var lowestY = System.Windows.Forms.Screen.AllScreens.Max(screen => + screen.Bounds.Bottom ); Assert.That( - leftmostX - bounds.Left, - Is.GreaterThanOrEqualTo(bounds.Width * 4), - "A window four times this wide would still have to clear the leftmost monitor." + bounds.Top - lowestY, + Is.GreaterThanOrEqualTo(bounds.Height * 4), + "A window four times this high would still have to clear the lowest monitor." ); } } From 0d4dbbd4c3589f4032f212541927bc130aaaf536 Mon Sep 17 00:00:00 2001 From: Hatton Date: Thu, 3 Sep 2026 08:30:36 -0600 Subject: [PATCH 6/8] Record the small default e2e window size as automation debt The suite sizes its window to whatever monitor it lands on, so it proves the layout only at the size of a developer's screen. That hides a class of bugs that users on inexpensive machines meet and we never do. The change itself is small, so the entry states the design: 1024x586 (the working area of a 1024x768 screen) for every run, BLOOM_AUTOMATION_WINDOW_SIZE to ask for something else, a 400x300 floor, and the same size whatever BLOOM_AUTOMATION_MONITOR says. The reason it is debt rather than a change is what it costs. A full run at 1024x586 gave 16 passed, 6 failed and 13 not run, against 23 passed and 2 failed at monitor size; the entry names the four tests the small window breaks and the triage each one needs. The developer chose to record the plan rather than carry a red suite, so the code that was written for it is not in the history and the entry says how to rebuild it. Co-Authored-By: Claude Opus 5 (1M context) --- src/BloomE2E/AUTOMATION-DEBT.md | 45 +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index e2d761b9f8ae..ce61d338ceb0 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -325,3 +325,48 @@ Fix direction, cheapest first, none of it tried yet: Whatever the mechanism, the suite needs the same thing from it: a way to say "run these tests at 1920x1080 at 150%" and have the run either honour it or refuse, rather than silently using the desktop it found. + +## Every run takes the developer's window size, so small-screen bugs go unseen + +A run makes its window as big as the monitor it lands on, so the suite proves the layout only at +the size of a developer's screen. Many Bloom users are on inexpensive machines with small screens, +and that is where a class of bugs lives that nobody on the team meets: a control that overlaps +another, a dialog that opens past an edge, a toolbar that quietly drops an item. This is the +window-size half of the DPI entry above, and it is much cheaper to fix, because it needs no +virtual monitor. + +The plan: give every automation run a window of **1024x586**, the working area of a 1024x768 +screen once a task bar of the usual height is taken off, wherever the window goes. +`BLOOM_AUTOMATION_WINDOW_SIZE=1600x900` asks for a different size, for chasing a bug that only +shows on a big screen. The floor is 400x300, which is `Shell.MinimumSize`; anything Bloom cannot +use, a typo included, gives the default rather than a broken run. The size must be the same for +all three values of `BLOOM_AUTOMATION_MONITOR`, so that variable decides only *where* a window +goes: a suite whose size changed with its placement would let one test pass in one mode and fail +in another, a trap that caught this code twice on BL-16804. + +The work is not the window size, which is about thirty lines in `AutomationWindowPlacement.cs` and +`Shell.cs`. The work is the suite going red, which is the point of the change. One full run of the +35 tests at 1024x586 on 2026-09-03 gave **16 passed, 6 failed, 13 did not run**, against 23 passed +and 2 failed at the size of a developer's monitor. Two of the six fail at any size (Test Case ID +349 is BL-16807, Test Case ID 169 is BL-16806), so the small window is what broke these four: + +- `copy-page.spec.ts:85` (Test Case ID 348), failed in 8 seconds. +- `derivative-keeps-template-pages.spec.ts:106` (Test Case ID 72), failed after 48 seconds. +- `format-gear-positioning.spec.ts:130` (Test Case ID 356), failed in 335 ms: the Format dialog no + longer opened close to its gear, while the test above it in the same file passed. So the small + window moved the dialog. +- `upload-required-items.spec.ts:88` (Test Case ID 606), failed after 1.2 minutes. + +Because the suite is serial per file, those 6 failures also stop 13 more tests from running, so +the small window costs 7 passes and hides 13 results until the fixes land. + +Each failure then needs triage into one of two piles, and the second pile is the reason to do any +of this: either the test assumed a large window and has to be rewritten, or **Bloom itself +misbehaves at 1024x586**, which is a real user-facing bug and wants its own card. Timeouts rather +than quick failures are the common failure mode, so the suite is also much slower while the fixes +are outstanding. Whoever picks this up has to decide what the nightly workflow does in the +meantime: run small and stay red, or stay large until the tests are fixed. + +(Written and measured on 2026-09-03 during BL-16804, then deliberately taken back out: the +developer chose to record the plan here rather than carry a red suite. The code is not in the +history, so rebuilding it from this entry is part of the job.) From 6f2a42d2445a47090e0aa193ebde2852d5d416fd Mon Sep 17 00:00:00 2001 From: Hatton Date: Thu, 3 Sep 2026 08:34:52 -0600 Subject: [PATCH 7/8] Say plainly where the below-primary window still gets the wrong DPI Devin's review of the previous commit is right on the geometry. The comment claimed that keeping the off-screen window directly under the primary keeps the primary the nearest monitor, and Windows applies the nearest monitor's scale factor. That claim holds only while no monitor sits below the primary in the same band of x. Stack one monitor under another at a different scale and the lower one is nearest, so the window comes out the wrong size again. Nobody on the team has such a layout, and getting it right for every layout means asking Windows for the nearest monitor's scale factor and scaling the requested size by the ratio, which needs the per-monitor DPI calls the automation-debt entry is about. So this records the limit in both places rather than overstating what the code does. Co-Authored-By: Claude Opus 5 (1M context) --- src/BloomE2E/AUTOMATION-DEBT.md | 22 ++++++++++++++++++---- src/BloomExe/AutomationWindowPlacement.cs | 13 ++++++++++++- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index ce61d338ceb0..bb878139c3c0 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -326,6 +326,17 @@ Whatever the mechanism, the suite needs the same thing from it: a way to say "ru at 1920x1080 at 150%" and have the run either honour it or refuse, rather than silently using the desktop it found. +One piece of this is a known limit in the code already, and it is what the fix direction above +would settle. `AutomationWindowPlacement.GetBoundsOffEveryMonitor` puts an off-screen window +directly below the primary monitor, because the nearest monitor is the one whose scale factor +Windows applies, and on the layouts we have that keeps the primary nearest. It stops being true +when a monitor sits *below* the primary in the same band of x: that lower monitor is then nearest, +and if its scale factor differs the window comes out the wrong size, which is the same bug in a +new layout. Fixing it properly means asking Windows for the nearest monitor's scale factor and +scaling the requested size by the ratio, which needs the per-monitor DPI calls this entry is +about. Nobody on the team has such a layout today, which is why it is written down rather than +fixed. (Devin raised it on PR 8285, 2026-09-03.) + ## Every run takes the developer's window size, so small-screen bugs go unseen A run makes its window as big as the monitor it lands on, so the suite proves the layout only at @@ -346,16 +357,19 @@ in another, a trap that caught this code twice on BL-16804. The work is not the window size, which is about thirty lines in `AutomationWindowPlacement.cs` and `Shell.cs`. The work is the suite going red, which is the point of the change. One full run of the -35 tests at 1024x586 on 2026-09-03 gave **16 passed, 6 failed, 13 did not run**, against 23 passed -and 2 failed at the size of a developer's monitor. Two of the six fail at any size (Test Case ID -349 is BL-16807, Test Case ID 169 is BL-16806), so the small window is what broke these four: +35 tests at 1024x586 on 2026-09-03 gave **16 passed, 6 failed, 13 did not run**, against 23 +passed and 2 failed at the size of a developer's monitor. Two of the six fail at either size, so +they are not the window's doing: Test Case ID 349 (BL-16807) and Test Case ID 606, which times out +after 60 seconds waiting for the publish-to-web steps. The small window is what added these four: - `copy-page.spec.ts:85` (Test Case ID 348), failed in 8 seconds. - `derivative-keeps-template-pages.spec.ts:106` (Test Case ID 72), failed after 48 seconds. - `format-gear-positioning.spec.ts:130` (Test Case ID 356), failed in 335 ms: the Format dialog no longer opened close to its gear, while the test above it in the same file passed. So the small window moved the dialog. -- `upload-required-items.spec.ts:88` (Test Case ID 606), failed after 1.2 minutes. +- `publish-text-languages.spec.ts:416` (Test Case ID 169), failed after 37 seconds. Read this one + with care: it is BL-16806, which is machine-dependent, and it passed in the full-size run of the + same build. So the window may have caused it or may not. Because the suite is serial per file, those 6 failures also stop 13 more tests from running, so the small window costs 7 passes and hides 13 results until the fixes land. diff --git a/src/BloomExe/AutomationWindowPlacement.cs b/src/BloomExe/AutomationWindowPlacement.cs index be2a9f6bc152..41727aea7c1a 100644 --- a/src/BloomExe/AutomationWindowPlacement.cs +++ b/src/BloomExe/AutomationWindowPlacement.cs @@ -215,7 +215,18 @@ public static Rectangle GetBoundsOffEveryMonitor() // 3840x2100 REAL pixels, taller than any monitor on the machine, and the page inside // it laid out at a viewport height no user could ever have. Keeping the window // directly under the primary, aligned with its left edge, keeps the primary the - // nearest monitor, so an off-screen window paints at the size a visible one would. + // nearest monitor on the layouts we have, so an off-screen window paints at the size + // a visible one would. + // + // "On the layouts we have" is the real limit here, and it is worth stating plainly. + // The primary is nearest only while no monitor sits below the primary in the same + // band of x. A machine with one monitor stacked under another, at a different scale + // factor, puts that lower monitor nearest instead, and the size comes out wrong in + // exactly the way described above. Getting it right for every layout means asking + // Windows for the scale factor of whichever monitor ends up nearest and scaling the + // requested size by the ratio, which is more than this code does today. See "No way + // to run the suite at a chosen monitor resolution and scale factor" in + // src/BloomE2E/AUTOMATION-DEBT.md. // // How far down: far enough that the window clears every monitor even after Windows // scales it. This process's idea of the height can be out by the ratio of two scale From 5f230bb5108066ce1ed213ec9d7c7fecfd2d5769 Mon Sep 17 00:00:00 2001 From: Hatton Date: Thu, 3 Sep 2026 10:22:15 -0600 Subject: [PATCH 8/8] Say which runs obey BLOOM_AUTOMATION_MONITOR, and stop claiming otherwise The class comment said a developer who leaves the variable set in their shell still gets an ordinary, visible Bloom when they start one themselves. That is false: ./go.sh passes --automation (scripts/watchBloomExe.mjs), so a Bloom started to work in obeys the variable too. The behaviour is right and stays. It is what the variable is for: the developer sets it once and every Bloom an agent starts stays off the monitor they are working on. Only the comment was wrong. It now also records the one consequence worth knowing, because a reader who meets it without warning loses an afternoon: "headless" hides a ./go.sh Bloom the developer started themselves, and a hidden Bloom looks exactly like one that failed to start. The log line DescribeChoice writes is what settles that. Co-Authored-By: Claude Opus 5 (1M context) --- src/BloomExe/AutomationWindowPlacement.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/BloomExe/AutomationWindowPlacement.cs b/src/BloomExe/AutomationWindowPlacement.cs index 41727aea7c1a..899e415a3b57 100644 --- a/src/BloomExe/AutomationWindowPlacement.cs +++ b/src/BloomExe/AutomationWindowPlacement.cs @@ -13,9 +13,19 @@ namespace Bloom /// "headless", or "0" for no monitor at all every window opens off every monitor /// absent, empty, or anything else Bloom places windows as it always does /// - /// The variable applies ONLY under --automation. A developer who leaves it set in their shell - /// therefore still gets an ordinary, visible Bloom when they start one themselves; only a run - /// that already declared itself automation obeys it. + /// The variable applies ONLY under --automation, and that is a wider set of runs than the test + /// suites: ./go.sh passes --automation as well (scripts/watchBloomExe.mjs), so a Bloom started + /// to work in obeys the variable too. That is deliberate, and it is what the variable is for: + /// the developer sets it once and every Bloom an agent starts stays off the monitor they are + /// working on. Two things follow that are worth knowing before changing this code. + /// + /// First, "headless" therefore hides a ./go.sh Bloom the developer started THEMSELVES, and a + /// hidden Bloom looks exactly like one that failed to start: the launcher reports success and + /// the HTTP server answers, but no window exists anywhere. The log line DescribeChoice writes + /// on every automation start is what settles that, so leave it in place. + /// + /// Second, a Bloom the developer starts with no --automation flag at all is untouched, however + /// the variable is set. /// /// Nothing here changes whether an automation window takes the keyboard focus. It never does, /// wherever it is (see Shell.ShowWithoutActivation and Shell.ReallyComeToFront).