From 3835378aee16f332621e93d7c43bd644be65884e Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Mon, 3 Aug 2026 09:13:22 -0400 Subject: [PATCH] fix(desktop): stop the terminal fade clobbering the app surface's compositor hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `FadeController` borrows `will-change: opacity` on the app surface for the duration of a ⌘J fade and released it by writing the literal string "auto". An inline declaration outranks the stylesheet, so that write does not restore the element to its authored state — it permanently replaces it. `.buzz-huddle-app-surface` carries an authored hint for the huddle drawer transition (components.css:30-31: bottom, border-bottom-left-radius, border-bottom-right-radius, box-shadow). After the first fade of a session the surface computed `will-change: auto` for the rest of the session, so every subsequent drawer open/close ran without the hint it was written to have. Release by removing the inline property instead, which hands the element back to CSS. Verified in a real browser: before, the surface computed "auto" at every stage of a reveal/conceal cycle; after, it computes the authored hint at all three. The fixture previously modelled `style` as a plain object, where "auto" and "absent" are indistinguishable — it scored the clobbering write as a correct release. It now models presence/absence and what the element would compute with the stylesheet included, which is what makes the defect observable. Five tests fail with the production line reverted and the tests kept. Mutation sweep 6/6 killed, including the original defect as M1 and a mutant that removes the wrong property name. This is not a fix for the intermittent conceal flicker; that remains open and unreproduced. Found while instrumenting it. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../features/terminal/fadeController.test.mjs | 54 +++++++++++++++++-- .../src/features/terminal/fadeController.ts | 17 ++++-- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/desktop/src/features/terminal/fadeController.test.mjs b/desktop/src/features/terminal/fadeController.test.mjs index 9a8c28b1a5..ae9214f743 100644 --- a/desktop/src/features/terminal/fadeController.test.mjs +++ b/desktop/src/features/terminal/fadeController.test.mjs @@ -37,8 +37,26 @@ function fixture() { calls.push("cancel"); }, }; + // Models the part of CSSOM this class depends on: an inline declaration is + // PRESENT or ABSENT, and only an absent one lets the stylesheet through. + // A plain `{ willChange: "auto" }` bag cannot tell those apart, so it would + // score a clobbering write as a correct release. + const authored = "bottom, box-shadow"; + const style = { + opacity: "1", + willChange: "", + removeProperty(property) { + assert.equal(property, "will-change"); + this.willChange = ""; + }, + /** What the element would actually compute, stylesheet included. */ + get computedWillChange() { + return this.willChange === "" ? authored : this.willChange; + }, + }; const surface = { - style: { opacity: "1", willChange: "auto" }, + authored, + style, animate(frames, options) { calls.push({ frames, options }); return animation; @@ -77,7 +95,7 @@ test("completed fades retain the timeline but release its compositor hint", asyn assert.equal(controller.toggle(false), "reveal"); completions[0].resolve(); await Promise.resolve(); - assert.equal(surface.style.willChange, "auto"); + assert.equal(surface.style.computedWillChange, surface.authored); assert.equal(controller.toggle(false), "conceal"); assert.equal(calls.includes("cancel"), false); @@ -96,13 +114,41 @@ test("a stale completion cannot release the hint during a reversal", async () => completions[1].resolve(); await Promise.resolve(); - assert.equal(surface.style.willChange, "auto"); + assert.equal(surface.style.computedWillChange, surface.authored); }); test("reduced motion settles immediately without allocating an animation", () => { const { calls, controller, surface } = fixture(); assert.equal(controller.toggle(true), "reveal"); assert.equal(surface.style.opacity, "0"); - assert.equal(surface.style.willChange, "auto"); + assert.equal(surface.style.computedWillChange, surface.authored); assert.deepEqual(calls, []); }); + +test('releasing the fade hint restores the authored will-change, not "auto"', async () => { + const { completions, controller, surface } = fixture(); + + // Before any fade the element is on the stylesheet's hint. + assert.equal(surface.style.computedWillChange, surface.authored); + + controller.toggle(false); + assert.equal(surface.style.willChange, "opacity"); + + completions[0].resolve(); + await Promise.resolve(); + // The inline declaration must be REMOVED. Writing the literal "auto" would + // leave an inline value that outranks the stylesheet and permanently strips + // the surface's authored compositor hint after the first fade. + assert.equal(surface.style.willChange, ""); + assert.equal(surface.style.computedWillChange, surface.authored); +}); + +test('settle also restores the authored hint rather than pinning "auto"', () => { + const { controller, surface } = fixture(); + controller.toggle(false); + assert.equal(surface.style.willChange, "opacity"); + + controller.settle("conceal"); + assert.equal(surface.style.willChange, ""); + assert.equal(surface.style.computedWillChange, surface.authored); +}); diff --git a/desktop/src/features/terminal/fadeController.ts b/desktop/src/features/terminal/fadeController.ts index 91cd5bd729..8434140c6e 100644 --- a/desktop/src/features/terminal/fadeController.ts +++ b/desktop/src/features/terminal/fadeController.ts @@ -14,7 +14,7 @@ type AnimationLike = { }; type SurfaceLike = { - style: Pick; + style: Pick; animate( keyframes: Keyframe[] | PropertyIndexedKeyframes | null, options?: number | KeyframeAnimationOptions, @@ -32,6 +32,17 @@ export class FadeController { this.#surface = surface; } + /** + * The fade owns `will-change` only for as long as it is animating. The app + * surface also carries an authored `will-change` for the huddle drawer + * transition, and an inline declaration outranks the stylesheet — so writing + * the literal `"auto"` does not "clear" the hint, it permanently replaces + * the drawer's. Removing the inline property hands the element back to CSS. + */ + #releaseHint(): void { + this.#surface.style.removeProperty("will-change"); + } + settle(direction: Direction): void { if (this.#animation) { this.#token += 1; @@ -40,7 +51,7 @@ export class FadeController { } this.#direction = direction; this.#surface.style.opacity = direction === "reveal" ? "0" : "1"; - this.#surface.style.willChange = "auto"; + this.#releaseHint(); } toggle(reducedMotion: boolean): Direction { @@ -78,7 +89,7 @@ export class FadeController { void animation.finished .then(() => { if (this.#animation === animation && this.#token === token) { - this.#surface.style.willChange = "auto"; + this.#releaseHint(); } }) .catch(() => {