Skip to content

test(browser): hold motion when it commits, not on its start event - #459

Merged
kalvinnchau merged 1 commit into
mainfrom
larry/webkit-flake-classes
Sep 30, 2026
Merged

kalvinnchau merged 1 commit into
mainfrom
larry/webkit-flake-classes

Conversation

@loganj

@loganj loganj commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

🤖

Summary

  • Two browser tests sometimes failed on Linux WebKit CI and then passed when the same commit ran again. The sidebar test failed with "Missing sidebar transition" (sidenav-polish.spec.mjs:337). The mention test at mentions.spec.mjs:594 checks a short reveal animation in the same way, so it can fail for the same reason.
  • Both tests freeze a short CSS motion (about 220ms) so they can check the middle of it. Until now they froze it when a "motion started" browser event arrived (transitionrun or animationstart). WebKit can send that event one frame late. If that frame is slow, the motion has already finished, so there is nothing left to freeze and the test fails. In the sidebar, other small transitions inside the sidebar (the resize handle) also send transitionrun up to the same listener.
  • Now each test freezes the motion when the page changes the attribute that starts it: aria-hidden on the sidebar, data-reveal on the mention qualifier. The browser creates the motion as part of that change, so the test holds it before any frame can run. No product code changes.
  • Fix flaky WebKit menu focus browser test #409 fixed the same kind of failure in menu-dismiss.spec.mjs in the same way.

Details

  • getAnimations() makes the browser compute styles right away. When the test calls it inside the attribute-change callback, the new transitions already exist and can be paused.
  • The late event does not happen on demand on macOS. To check the fix, I made the old event arrive after the motion had finished. The old tests then failed on both Chromium and WebKit, and the new tests passed. Without that change, both the old and new versions passed 20 repeated WebKit runs locally, so Linux CI is the real check.
  • Both changed files pass in full on Chromium and WebKit locally.
  • Not included: three tests in tests/fixtures/design-system/viewer.spec.ts also freeze motion from transitionrun. They filter by target element, so the bubbling problem does not affect them, but a late event still can. They have not been seen failing in CI, so I did not change them here.

sidenav-polish:337 failed on Linux WebKit CI with "Missing sidebar
transition". It paused the 220ms sidebar transition from a transitionrun
listener. WebKit can dispatch that event in a later frame, after the
transition has finished, and the resize help's descendant transitions
also bubble transitionrun to the same listener. #409 fixed the same class
in menu-dismiss.

Pause the transitions from a MutationObserver on the toggle's aria-hidden
change. getAnimations() flushes style, so the transitions exist and are
held before any frame can advance them. mentions.spec.mjs had the same
shape with animationstart for the qualifier reveal; it now holds the
reveal when data-reveal renders.

Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
@loganj
loganj marked this pull request as ready for review September 30, 2026 16:43
@loganj
loganj requested review from a team, comp615 and wesbillman as code owners September 30, 2026 16:43

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No actionable source findings: the observers hold real motion at its DOM commit while retaining interpolation, clipping, reduced-motion, and recovery assertions.

Hosted CI passed on attempt 2 (the first attempt had cancellations); the affected files passed in both engines, but repeat-run flake elimination is not established.

Source-only: I ran no tests/apps, and did not establish comparative timing improvements or native acceptance; Windows validation was skipped.

Star Lord’s automated review via Wes’s account — head 161cc909d4708236d4f426ea70ff2b583ac06ec3, base 9ab4a1792b0d2cc60c9734baf1dc45911ee8bc83; COMMENT only, not approval.

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-only review: no blockers.

Both flake fixes replace start-event listeners (transitionrun / animationstart) with a MutationObserver on the attribute/style commit, matching the menu-dismiss #409 pattern. The sidenav observer on aria-hidden is exactly the style change that creates the max-width/clip-path transitions, and nav.getAnimations() excludes descendant transitions, so both old failure modes (bubbled descendant transitionrun, late dispatch after the 220ms transition finished) are closed. The mentions observer holds the reveal when data-reveal renders and pauses only matching animationName, preserving the qualifierReveals.length === 2 hold guarantee.

Deferred check: Linux WebKit CI green on this head is the evidence the mutation-microtask getAnimations() flush holds; a green run here should complete it.

@kalvinnchau
kalvinnchau merged commit a04aae5 into main Sep 30, 2026
30 of 37 checks passed
@kalvinnchau
kalvinnchau deleted the larry/webkit-flake-classes branch September 30, 2026 17:24
TheSentinel454 pushed a commit that referenced this pull request Sep 30, 2026
* origin/main: (27 commits)
  Let plugin pages publish NIP-AR artifacts and embed the host thread view (#434)
  test(app): migrate entity-navigation test off removed buzz://open locator API (#463)
  Show agent activity in navigation (#423)
  test(browser): hold motion when it commits, not on its start event (#459)
  fix(navigation): ignore unknown query parameters on Buzz links and remove the buzz://open locator (#457)
  feat(design-system): distinguish controls on floating surfaces (#429)
  feat(native): add community extras and media preparation (#450)
  Clone inventory identities through reviewed text and fresh identity creation (#289)
  feat(communities): add right-click actions to the community rail (#400)
  fix(messages): keep a send reveal pending until its scroll runs (#454)
  fix(messages): reserve a stable scrollbar gutter on the channel feed (#451)
  fix(sidebar): list plugin pages as sidebar rows via an opt-in primary flag (#401)
  feat(channels): surface canvas content in channel settings (#426)
  fix(profiles): remove redundant presence status row (#394)
  test(browser): count live retries once the page handles startup controls (#443)
  feat(composer): host-owned resource links for the Projects picker (#445)
  feat: support native read state and recent channel activity (#444)
  feat(native): serve relay media and uploads in packaged builds (#433)
  feat(channels): suggest joined channels in the composer (#446)
  feat: support native agent activity, library, memories, and community resolution (#441)
  ...

Signed-off-by: Codex <noreply@openai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants