Keep the search view closed when its focus comes back late
tests / feature (8.4) (push) Successful in 1m49s
tests / feature (8.5) (push) Successful in 1m51s
tests / browser (chrome, chromium) (push) Successful in 7m34s
tests / browser (firefox, firefox) (push) Failing after 14m19s
tests / browser (safari, webkit) (push) Failing after 13m16s

On a slow machine Escape could close the full-screen search and have it
open again for good. A close hands focus back to the input — Escape does,
and so does the view's focus trap as it lets go — and `focused()` told
that return from someone coming to search by a 250ms wall-clock window.
Both returns run on frames, and a runner painting a few frames a second
took longer than that, so the returning focus opened the view again. A
`returning` flag now covers the close until the hand-back has actually
run, on the same frame, and the constant is gone.

`hold()` times the full-screen layout from the exit's own duration token,
the one Alpine's x-transition holds `display` for, instead of waiting a
frame or two for the exit's transitions to appear: `getAnimations()` is
empty both before an engine creates them and after they end, and a loaded
engine can leave a second between frames.

The browser tests were racing the same slow machine, reproduced in a
Linux container like the runner, with its Playwright Firefox and
WebKitGTK and two cores kept busy:

- The browser plugin retries every script and action with a one-second
  attempt until the budget runs out. An in-page sleep longer than that
  only ever passed on the last attempt, which is where the 47-50s tests
  came from, and a script that clicks was run again against a page that
  had moved on. `onceInPage()` runs such a script once however often it
  is retried; the long sleeps are plain retried conditions now.
- Under load WebKitGTK paints no frame while a tight setTimeout loop
  runs, so a sample loop saw the start value and then nothing.
  `caughtMidExit()` samples on animation frames, for 2.5s.
- "No animations running" is also true before an opening transition
  exists, so a close could be sampled from a scrim at 4% opacity.
  `settled()` waits two frames before it asks.

The workflow no longer uploads failure screenshots: Gitea's artifact
service timed out on every attempt, two minutes per red run, and the job
logs are readable without it.

Feature 1159 passed. Browser 299 passed on Chrome, Firefox and WebKit on
macOS, and on Firefox and WebKitGTK in the Linux container under load,
twice each.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Andreas Reinhold / reini
2026-09-18 09:49:17 +02:00
co-authored by Claude Opus 5
parent 8da8b1fa20
commit e7885ce1eb
8 changed files with 270 additions and 188 deletions
+39 -42
View File
@@ -24,19 +24,15 @@
* transition has run, while the header bar fades out with it (search.css). Without it the bar went
* back to its resting form, or to nothing behind the icon, on the first frame of the exit, and the
* fading view dropped to the docked layout. The focus trap lets go at the close itself, not at the
* end of the exit, so its return of focus still lands inside RETURN_GUARD_MS.
* end of the exit, so `returning` (close()) covers its return of focus like the field's own.
*/
import { upTo } from './breakpoints.js'
import { ms } from './util.js'
const FOCUSABLE = 'a[href], button:not([disabled]), input:not([disabled]), select:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])'
const CHOOSES = 'a[href], button:not([disabled]), [data-md-list-open]'
// A close hands focus back to the input: Escape does, and so does the full-screen view's focus trap
// when it lets go, a moment later. Focus arriving this soon after a close is that, not someone
// coming to search.
const RETURN_GUARD_MS = 250
// A Livewire morph replaces the results in several mutations; wait for the batch to end before
// counting, so the live region speaks once.
const SETTLE_MS = 120
@@ -48,7 +44,8 @@ document.addEventListener('alpine:init', () => {
leaving: false,
leavings: 0,
compact: false,
closedAt: -Infinity,
// A close is handing focus back; see close().
returning: false,
announcement: '',
// What is in the field, which is what tells suggestions from results.
query: '',
@@ -126,64 +123,64 @@ document.addEventListener('alpine:init', () => {
},
focused() {
if (performance.now() - this.closedAt > RETURN_GUARD_MS) {
if (!this.returning) {
this.show()
}
},
/**
* `refocus` puts focus back where the view came from: the icon button, or the field itself.
*
* Either way a close hands focus back — the full-screen view's focus trap returns it as it
* lets go, whether this asked for it or not — and that focus must not read as someone
* coming to search, or the view would open again on its way out. `returning` covers it
* until the hand-back has run, rather than for a fixed stretch of wall-clock time after
* the close: both returns are scheduled on frames, and a runner painting a handful of
* frames a second takes longer over one than any such guard would allow, which left the
* view open for good.
*/
close(refocus = false) {
const fromFullScreen = this.open && this.fullScreen
const back = refocus ? (this.$refs.trigger ?? this.$refs.input) : null
this.open = false
this.closedAt = performance.now()
this.returning = true
if (fromFullScreen) {
this.hold()
}
if (refocus) {
// Back to whatever opened the view: the icon button, or the field itself. A frame
// after the tick, as `expand()` waits: the icon button is `x-show`n, and Alpine only
// shows it on that frame, so a focus in the tick reaches a hidden button, which
// Firefox and WebKit refuse (in Chrome the trap's own return had covered for it).
const back = this.$refs.trigger ?? this.$refs.input
this.$nextTick(() => requestAnimationFrame(() => back.focus()))
}
// A frame after the tick, as `expand()` waits: the icon button is `x-show`n, and Alpine
// only shows it on that frame, so a focus in the tick reaches a hidden button, which
// Firefox and WebKit refuse (in Chrome the trap's own return had covered for it).
this.$nextTick(() => requestAnimationFrame(() => {
back?.focus()
this.returning = false
}))
},
/**
* Keeps the full-screen layout for the length of the view's exit: until the animations the
* closed state starts, a frame on, have finished (at once when none run); reopening, which
* counts `leavings` up, lets a pending end go by.
* Keeps the full-screen layout for the length of the view's exit the duration search.css
* gives it, which is also the one Alpine's `x-transition` holds its `display` for, so the
* two end together. Reopening, which counts `leavings` up, lets a pending end go by, and
* reduced motion zeroes the token, which ends the hold on the next task, as it should.
*
* The duration rather than the exit's own animations: `getAnimations()` is empty both
* before an engine has created the transitions and after they have finished, and nothing
* on the element tells the two apart. Waiting a frame or two for them to appear only moves
* the guess — a loaded engine can leave a second between two frames, and holding for what
* it had not started yet ended the full-screen layout at once, mid-exit.
*/
hold() {
const leaving = ++this.leavings
this.leaving = true
// A frame on, the closed state has met the style and the exit's transitions exist, as
// the rail's own settle() reads them. One frame is not always enough: an engine that
// starts them on its next refresh tick would show none here, and holding for nothing
// would end the full-screen layout at once, mid-exit. So an empty list is asked again
// on the following frame before it counts as "nothing to wait for".
const hold = (frame) => requestAnimationFrame(() => {
const animations = this.$refs.view?.getAnimations() ?? []
if (animations.length === 0 && frame === 0) {
hold(1)
return
setTimeout(() => {
if (leaving === this.leavings) {
this.leaving = false
}
Promise.allSettled(animations.map((animation) => animation.finished)).then(() => {
if (leaving === this.leavings) {
this.leaving = false
}
})
})
hold(0)
}, ms(this.$refs.view, '--md-sys-motion-spatial-fast-duration') ?? 0)
},
clear() {