Measure the drawer's inert window in a real browser (task 197) #212
Loading…
Reference in a new issue
No description provided.
Delete branch "drawer-inert-after-close"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This is an advance in understanding, not a fix, and task 197 stays open. No production code is touched. Task 197's own wording is that it is "worth doing properly, not worth forcing", and a seventh attempt that narrowed the window while being reported as a fix would be worse than nothing.
Not a repeat of any of the six attempts ADR-0119 records — no relocation of
OverlayFocusTrap.ExitAsync, noInvokeAsyncavoidance, no@key, no same-tick fix. The approach was to stop reasoning about the mechanism and measure whether the confirmedinertwindow actually costs a user a click, in a real browser, at the exact trigger point the existing repro uses.What was measured, in Chromium via Playwright:
MutationObserver, with no Playwright round trip in the timing path, foundinertalreadyfalseat the moment the panel's removal became observable, in all 25 timed runs.A methodological trap, now documented so nobody repeats it: a scripted
element.click()does not respectinertin this Chromium build. Any measurement built on it would report a pass regardless of the defect.A second finding that matters more than the result.
HomeAndDrawerSharedGroupStateTests's repro currently passes 25/25 with bothWaitForAssertionwrappers removed, on unmodifiedmain— but this is not evidence the defect is fixed. Commitcd2eff8(task 263, merged the day before this pass) wired aFakeClosingAnimationDelayinto that test which resolvesModalOverlay's 200ms closing wait synchronously, removing the yield point the old repro depended on to expose the race. Production code is byte-for-byte what ADR-0119 examined. That test currently proves nothing about this race in either direction, and now carries a comment saying so.What ships:
DrawerClickableImmediatelyAfterModalTaskClosesJourneyTests, in the E2E suite — at the level that can actually observeinert, which bUnit cannot.WaitForAssertionworkaround stays, now with a comment explaining why its own repro cannot currently be trusted either way.What this does not establish. Seventy-five trials on a fast, idle machine do not prove the defect is unreachable on a slower device or a loaded one. That is untested, and is stated as untested rather than glossed. Task 197 remains open with these findings recorded on it.
Verified under the pinned SDK 10.0.100: 2,222 unit and integration tests passed, 0 failed; the full E2E suite — 62 including the 2 new — passed in about 2.5 minutes with
PLACEMARK_RUN_E2E_TESTS=trueconfirmed set and the tests confirmed executing rather than silently skipped, each real Chromium journey taking 30–45 seconds rather than sub-second.Verdict: mergeable
Checked against the five things that would actually undermine this investigation, none held up:
Close_PanelStillOpen_ClickOnTheDrawerNeverReachesItdrives the samepage.Mouse.ClickAsyncmechanism at the same computed button coordinates as the positive measurement, against a genuinely open panel. All selectors it and the positive test use (modal-overlay-panel,modal-overlay-close,accordion-add-place,place-form-name) exist in production markup — not a check against a selector that vacuously never matches.element.click()claim is consistent with the shipped test: both tests usepage.Mouse.ClickAsyncthroughout for the timing-sensitive assertions;Locator.ClickAsync()only appears for closing panels, which isn't timing-sensitive.cd2eff8/FakeClosingAnimationDelayexplanation checks out at the code level.ModalOverlay.CloseAsyncdoesawait ClosingDelay.WaitAsync(...)thenawait OnClose.InvokeAsync();FakeClosingAnimationDelay.WaitAsyncreturnsTask.CompletedTaskunlessHold()is called, andawaiton an already-completed task doesn't yield — it removes exactly the yield point the six ADR-0119 attempts relied on to expose the race. Traced the wiring further:Drawer's<aside inert="@(!IsOpen || OverlayFocus.IsOpen)">only clears on a second, separately-scheduled render fired byOverlayFocusTrap.Changed, itself only raised fromModalOverlay.DisposeAsync— genuinely two Blazor dispatch cycles apart from the render that detaches the panel's DOM. That's real production wiring, not stubbed, so the new E2E test's trigger point is tied to a genuine race window rather than something that can only ever pass.AdrFiles_Corrected_HasAMatchingInlineMarkerInBothDirectionspasses (ran it locally against this head).8883668, matches head) —e2ejob checked out the same SHA and loggedPassed! - Failed: 0, Passed: 62, ... Total: 62in 3m10s wall time;buildjob shows the same 62 tests individually[SKIP]ped with noPLACEMARK_RUN_E2E_TESTSset there, and the workflow only sets that env var in thee2ejob. Baselinemain(5cb644a) E2E total is 60 — confirms the 60→62 claim.One minor, non-blocking discrepancy: the PR body claims "2,222 unit and integration tests passed" — CI's actual sum (Domain 39 + Contracts 139 + Architecture 148 + WebUI 916 + Infrastructure 360 + Api 622) is 2,224. Off by two; doesn't change anything about the substance of the investigation.
The stated limit ("75 trials on a fast idle machine ... untested on a slower or loaded device") is honest and matches what was actually measured — nothing here claims the defect doesn't exist, only that it wasn't reproduced under the conditions tried.