diff --git a/patches/README.md b/patches/README.md new file mode 100644 index 00000000..b7403138 --- /dev/null +++ b/patches/README.md @@ -0,0 +1,101 @@ +# Virtua 0.51.0 macOS WebKit correction boundary + +The application imports the React ESM entry (`virtua` → `lib/index.js`) from +`src/features/messages/ChannelTimeline.tsx`. Only that entry's element scroller is +patched; CommonJS, window scrolling, and other-framework exports are untouched. +Keep the dependency pinned to 0.51.0 and review the patch plus version-coupled +installed-bundle tests before upgrading or adding a different import. + +## Failure and chosen boundary + +In a system WKWebView, native momentum can overwrite an instant programmatic +scroll correction. An isolated reproduction adapted from +[WebKit 262287](https://bugs.webkit.org/show_bug.cgi?id=262287) sustains missing +pixels even after DOM geometry covers the viewport. The same failure reproduces +with the production ChannelTimeline and fixed messages when history is prepended. +This does not attribute every disappearing-content incident to this mechanism. + +The previous Mac deferred-store/extent patch is **superseded**. A pause in native +momentum can outlast Virtua's 150ms inferred-idle timer: the deferred correction +then runs before native momentum actually ends, and the timeline stays blank. +One of two paused draft trials failed; this was timing-dependent, not a +universally failing sequence. Increasing an idle timeout is not the remedy. + +Instead, at each **nonzero automatic correction** on Mac WebKit: + +1. Temporarily set only the corrected overflow axis to `hidden !important`. +2. Apply Virtua's original relative correction or absolute edge target. +3. Restore the prior declaration value and priority in the next task. + +This extends the mechanism already used by +[Virtua's iOS driver](https://github.com/inokawa/virtua/blob/0.51.0/src/core/driver.ts#L203-L248), +without changing the iOS branch. Overlapping interventions cancel/restore their +predecessor before capturing the original declaration. Disposal restores +immediately; an observably changed later declaration is not overwritten. + +**Tradeoff:** a correction can stop the remaining trackpad coast. Several size +corrections can therefore reduce inertial travel more than one prepend. A fresh +gesture must continue to work; sustained real-history/media acceptance must assess +whether repeated braking is acceptable. No wheel ownership, permanent scrolling +CSS, forced layout, alternate store sizing, or new scroll scheduler is introduced. + +The platform predicate requires MacIntel and Apple vendor, excluding Virtua's iOS +detector (including desktop-mode iPad). Chrome/Firefox, non-Mac WebKit and iOS keep +existing policy. Store/layout/observer timing and imperative smooth/instant +navigation remain stock. Scheduler-driven reveal/restore/bottom navigation is a +separate acceptance path, not implicitly repaired by the automatic-correction fix. +Native reveal controls showed one/two transient blank interior source frames +before immediate recovery, despite valid sampled DOM coverage. This remaining +imperative-path flicker is not the sustained automatic-correction failure; the +patch does not claim to fix it. +The stale source-map directive is removed because the generated map is unpatched. + +## Automated checks + +```sh +bin/pnpm install --frozen-lockfile +bin/pnpm typecheck +bin/pnpm exec vitest run src/features/messages/virtua-compensation.test.mjs \ + src/features/messages/ChannelTimeline.test.tsx +bin/pnpm test:browser history-loading.spec.mjs image-scroll.spec.mjs initial-position.spec.mjs \ + --project chromium --project webkit --no-deps --workers=1 +``` + +The 13 driver/store/observer contracts evaluate the installed React ESM, not a +copied implementation. They cover active and inferred-idle corrections, zero +jumps, positive/negative measurements, absolute edges, horizontal RTL, overlapping +restoration, CSS priority, disposal/remount, later declarations, platform controls, +and unchanged imperative instant/smooth calls. Copied-bundle mutation controls +must fail when the production invocation, overlap/dispose restoration, priority, +or iPad exclusion is removed, or an inferred-scrolling gate is introduced. +These tests use a fake viewport; they cannot establish native painted pixels. + +## Native acceptance and limits + +Use an isolated system WKWebView, fixed data, no live identity, and phase-bearing +native wheel/momentum input confined to that window. Freeze/hash the source, +installed bundle and runner before each run. Start with a settled production +ChannelTimeline, prepend during momentum, then repeat with a pause longer than +150ms before the final momentum event. Compare stock, the superseded draft, and +the replacement. Capture source video frames separately from DOM/anchor traces. + +Require both painted content and the correct identified reading anchor. Repeat +with a fresh reversed gesture, a second prepend at the bottom, a fresh upward +gesture, positive/negative row measurement changes, and explicitly gated local +images. Check restoration and continued movement, not just final scrollTop. +Exercise imperative reveal/restore/bottom controls separately. A pending image +placeholder and bottom rubberband/fractional edge gaps are not missing tiles. + +Observed on macOS 26.6.2: stock and a paused old-draft trial sustain blank message +pixels; the replacement preserves content and correction anchors in the matched +native trials. Repeated native gestures and local images continue to paint, with +the braking tradeoff above. Ordinary Chromium/WebKit browser history, image and +initial-position journeys provide additional behavioral coverage, **not equivalent +native momentum/compositor evidence**. + +The fixed-data native fixture does not fetch real older history. Three-second +movies are not four seconds of pixel evidence just because DOM traces run longer; +movie and JavaScript clocks are not synchronized. These bounded results are not +signed-package/cross-platform acceptance or verification of the original user's +sustained real-message incident. Keep the change draft until that acceptance is +completed; do not restart a running app without coordination. diff --git a/patches/virtua@0.51.0.patch b/patches/virtua@0.51.0.patch new file mode 100644 index 00000000..a1d36783 --- /dev/null +++ b/patches/virtua@0.51.0.patch @@ -0,0 +1,55 @@ +diff --git a/lib/index.js b/lib/index.js +--- a/lib/index.js ++++ b/lib/index.js +@@ -7,1 +7,5 @@ + import { flushSync as f } from "react-dom"; ++ ++// WebKit can restore a stale native scroll offset after an instant correction. ++// Interrupt only the corrected axis; observer-inferred idle is not native idle. ++const isMacWebKit = () => typeof navigator !== "undefined" && !b() && navigator.platform === "MacIntel" && navigator.vendor === "Apple Computer, Inc."; +@@ -249,7 +253,22 @@ + o && o(); + } ]; + }, E = (e, t) => { +- let n, o, r = $(), i = !1; ++ let n, o, r = $(), i = !1, restoreOverflow; ++ const interruptMomentum = () => { ++ // Repeated corrections in one task share the original declaration. ++ restoreOverflow && restoreOverflow(); ++ const style = n.style, property = t ? "overflow-x" : "overflow-y"; ++ const value = style.getPropertyValue(property), priority = style.getPropertyPriority(property); ++ style.setProperty(property, "hidden", "important"); ++ const restore = () => { ++ S(timer); ++ if (style.getPropertyValue(property) === "hidden" && style.getPropertyPriority(property) === "important") { ++ value ? style.setProperty(property, value, priority) : style.removeProperty(property); ++ } ++ restoreOverflow = void 0; ++ }, timer = p(restore); ++ restoreOverflow = restore; ++ }; + const s = t ? "scrollLeft" : "scrollTop", l = t ? "left" : "top", c = t ? "overflowX" : "overflowY", [f, a] = O(e, () => r[0], (e, t) => { + n.scrollTo({ + [l]: T(e, i), +@@ -266,7 +285,8 @@ + return { + D(f, u = f.parentElement) { + h.A(n = u), t && (i = "rtl" === getComputedStyle(u).direction), o = M(e, u, t, () => T(u[s], i), (t, n, o) => { +- if (o) { ++ if (isMacWebKit()) interruptMomentum(); ++ else if (o) { + const e = u.style, t = e[c]; + e[c] = "hidden", p(() => { + e[c] = t; +@@ -283,7 +303,7 @@ + }), r[1](!0); + }, + _() { +- h.X(), o && o.X(), r[1](!1), r = $(); ++ restoreOverflow && restoreOverflow(), h.X(), o && o.X(), r[1](!1), r = $(); + }, + P: (e, t) => (d.set(e, t), h.A(e), () => { + d.delete(e), h.B(e); +@@ -799,2 +819,1 @@ + export { G as VList, j as Virtualizer, U as WindowVirtualizer, N as experimental_VGrid }; +-//# sourceMappingURL=index.js.map diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 55a44e61..855889e9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4,6 +4,9 @@ settings: autoInstallPeers: true excludeLinksFromLockfile: false +patchedDependencies: + virtua@0.51.0: 14ff685d9bc68b34d1a7c6b0042d6000e4b07a9746088fc3842c89c4a995ca9d + importers: .: @@ -76,7 +79,7 @@ importers: version: 4.0.1 virtua: specifier: 0.51.0 - version: 0.51.0(react-dom@19.2.8(react@19.2.8))(react@19.2.8) + version: 0.51.0(patch_hash=14ff685d9bc68b34d1a7c6b0042d6000e4b07a9746088fc3842c89c4a995ca9d)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) devDependencies: '@biomejs/biome': specifier: 2.5.12 @@ -2955,7 +2958,7 @@ snapshots: '@types/unist': 3.0.3 vfile-message: 4.0.3 - virtua@0.51.0(react-dom@19.2.8(react@19.2.8))(react@19.2.8): + virtua@0.51.0(patch_hash=14ff685d9bc68b34d1a7c6b0042d6000e4b07a9746088fc3842c89c4a995ca9d)(react-dom@19.2.8(react@19.2.8))(react@19.2.8): optionalDependencies: react: 19.2.8 react-dom: 19.2.8(react@19.2.8) diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml new file mode 100644 index 00000000..6bab0058 --- /dev/null +++ b/pnpm-workspace.yaml @@ -0,0 +1,2 @@ +patchedDependencies: + virtua@0.51.0: patches/virtua@0.51.0.patch diff --git a/src/features/messages/virtua-compensation.test.mjs b/src/features/messages/virtua-compensation.test.mjs new file mode 100644 index 00000000..696f24e5 --- /dev/null +++ b/src/features/messages/virtua-compensation.test.mjs @@ -0,0 +1,251 @@ +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; + +// Exercise the installed React ESM's actual store, observer and element driver. +// Names are deliberately version-coupled: review this extraction on upgrade. +const source = readFileSync( + fileURLToPath(import.meta.resolve("virtua")), + "utf8", +); +const start = source.indexOf("var {min:"); +const end = source.indexOf("}, W = (e, t) => {"); +if (start < 0 || end < 0) + throw new Error("Review Virtua driver extraction after version change"); +const helperStart = source.indexOf("const isMacWebKit ="); +const helper = helperStart >= 0 ? source.slice(helperStart, start) : ""; +const core = source.slice(start, end + 1); + +beforeEach(() => vi.useFakeTimers()); +afterEach(() => vi.useRealTimers()); + +function setup({ + platform = "MacIntel", + vendor = "Apple Computer, Inc.", + touch = 0, + agent = "Macintosh", + horizontal = false, + direction = "ltr", + offset = 1300, +} = {}) { + const { + store: createStore, + layout, + driver: createDriver, + } = new Function( + "navigator", + "getComputedStyle", + `${helper}${core};return {store:y,layout:R,driver:E};`, + )({ platform, vendor, maxTouchPoints: touch, userAgent: agent }, () => ({ + direction, + })); + const store = createStore(layout(20, 100)); + const declarations = new Map(); + const style = { + getPropertyValue: (name) => declarations.get(name)?.[0] ?? "", + getPropertyPriority: (name) => declarations.get(name)?.[1] ?? "", + setProperty: (name, value, priority = "") => + declarations.set(name, [value, priority]), + removeProperty: (name) => declarations.delete(name), + }; + const viewport = new EventTarget(); + const calls = []; + const axis = horizontal ? "overflow-x" : "overflow-y"; + const key = horizontal ? "scrollLeft" : "scrollTop"; + const option = horizontal ? "left" : "top"; + Object.assign(viewport, { + style, + offsetParent: {}, + scrollTop: 0, + scrollLeft: 0, + ownerDocument: { + defaultView: { + ResizeObserver: class { + observe() {} + unobserve() {} + disconnect() {} + }, + }, + }, + }); + viewport[key] = direction === "rtl" ? -offset : offset; + for (const method of ["scrollTo", "scrollBy"]) + viewport[method] = (options) => { + calls.push({ + method, + options, + overflow: style.getPropertyValue(axis), + priority: style.getPropertyPriority(axis), + }); + viewport[key] = + (method === "scrollBy" ? viewport[key] : 0) + options[option]; + }; + store.W(4, 500); // measured viewport + store.W(1, offset); // observed native scrolling + const driver = createDriver(store, horizontal); + driver.D({}, viewport); + return { + store, + driver, + viewport, + style, + axis, + calls, + prepend(length = 40) { + store.W(5, [length, true]); + driver.J(); + }, + }; +} + +it("interrupts the actual correction before its relative DOM write, even after inferred idle", () => { + const c = setup(); + c.store.W(2); // observer idle is NOT native momentum completion + c.prepend(); + expect(c.calls).toEqual([ + { + method: "scrollBy", + options: { top: 2000, behavior: "instant" }, + overflow: "hidden", + priority: "important", + }, + ]); + expect(c.viewport.scrollTop).toBe(3300); + expect(c.store.t()).toBe(4000); // no deferred extent/store policy + expect(c.store.u(39)).toBe(3900); + c.driver.J(); + expect(c.calls).toHaveLength(1); + vi.runAllTimers(); + expect(c.style.getPropertyValue(c.axis)).toBe(""); + expect(vi.getTimerCount()).toBe(0); +}); + +it("does not defer while scrolling or interrupt without a correction", () => { + const c = setup(); + c.driver.J(); + expect(c.calls).toHaveLength(0); + expect(vi.getTimerCount()).toBe(0); + c.prepend(); + expect(c.calls).toHaveLength(1); + expect(c.store.t()).toBe(4000); + expect(c.store.L()[0]).toBe(0); +}); + +it("preserves absolute edge correction and RTL axis normalization", () => { + for (const config of [ + { offset: 1500 }, + { horizontal: true, direction: "rtl", offset: 1500 }, + ]) { + const c = setup(config); + c.prepend(); + expect(c.calls).toEqual([ + { + method: "scrollTo", + options: { + [config.horizontal ? "left" : "top"]: config.horizontal + ? -3500 + : 3500, + behavior: "instant", + }, + overflow: "hidden", + priority: "important", + }, + ]); + c.driver._(); + } +}); + +it("restores exact value and priority after overlapping corrections without touching the other axis", () => { + const c = setup(); + c.style.setProperty("overflow-y", "scroll", "important"); + c.style.setProperty("overflow-x", "clip", ""); + c.prepend(); + c.prepend(60); + expect(c.calls.map((call) => call.overflow)).toEqual(["hidden", "hidden"]); + expect(vi.getTimerCount()).toBe(1); + expect(c.style.getPropertyValue("overflow-x")).toBe("clip"); + vi.runAllTimers(); + expect(c.style.getPropertyValue("overflow-y")).toBe("scroll"); + expect(c.style.getPropertyPriority("overflow-y")).toBe("important"); +}); + +it("restores immediately on dispose and permits a fresh lifecycle", () => { + const c = setup(); + c.style.setProperty(c.axis, "auto"); + c.prepend(); + c.driver._(); + expect(c.style.getPropertyValue(c.axis)).toBe("auto"); + expect(c.style.getPropertyPriority(c.axis)).toBe(""); + expect(vi.getTimerCount()).toBe(0); + c.driver.D({}, c.viewport); + c.prepend(60); + expect(c.calls.at(-1).overflow).toBe("hidden"); + c.driver._(); + expect(c.style.getPropertyValue(c.axis)).toBe("auto"); +}); + +it("does not overwrite a later style owner", () => { + const c = setup(); + c.prepend(); + c.style.setProperty(c.axis, "clip", "important"); + vi.runAllTimers(); + expect(c.style.getPropertyValue(c.axis)).toBe("clip"); + expect(c.style.getPropertyPriority(c.axis)).toBe("important"); +}); + +it("interrupts positive and negative resize compensation at the same production driver", () => { + for (const size of [50, 150]) { + const c = setup(); + c.store.W(3, [[0, size]]); + c.driver.J(); + expect(c.calls).toEqual([ + { + method: "scrollBy", + options: { top: size - 100, behavior: "instant" }, + overflow: "hidden", + priority: "important", + }, + ]); + c.driver._(); + } +}); + +it("does not change the imperative scheduler's smooth or instant scrolling policy", async () => { + for (const smooth of [false, true]) { + const c = setup(); + c.store.W( + 3, + Array.from({ length: 20 }, (_, index) => [index, 100]), + ); + await c.driver.V(() => 900, smooth); + await vi.advanceTimersByTimeAsync(0); + expect(c.calls.at(-1).overflow).toBe(""); + expect(c.calls.at(-1).options).toEqual({ + top: 900, + behavior: smooth ? "smooth" : "instant", + }); + c.driver._(); + vi.clearAllTimers(); + } +}); + +for (const [name, config, deferred] of [ + ["macOS Chrome", { vendor: "Google Inc." }, false], + ["macOS Firefox", { vendor: "" }, false], + ["Linux WebKit", { platform: "Linux x86_64" }, false], + ["desktop-mode iPad", { touch: 5 }, true], + ["iPhone", { platform: "iPhone", agent: "iPhone", touch: 5 }, true], +]) { + it(`preserves existing ${name} policy`, () => { + const c = setup(config); + c.prepend(); + expect(c.calls).toHaveLength(deferred ? 0 : 1); + if (deferred) { + c.store.W(2); + c.driver.J(); + } + expect(c.calls.at(-1).overflow).toBe(""); + expect(c.store.t()).toBe(4000); + c.driver._(); + }); +} diff --git a/tests/browser/emoji.spec.mjs b/tests/browser/emoji.spec.mjs index 827b6c8d..c7809b0a 100644 --- a/tests/browser/emoji.spec.mjs +++ b/tests/browser/emoji.spec.mjs @@ -677,10 +677,32 @@ test("community picker uses keyboard, proxy thumbnails, event-local history and await expect(region.getByRole("alert")).toContainText( "Fixture catalog offline", ); - await page.evaluate(() => window.emojiFixture.fail(false)); - await page - .getByRole("button", { name: "Retry emoji", exact: true }) - .click(); + await page.evaluate(() => { + window.emojiFixture.fail(false); + window.emojiFixture.holdCatalog(); + }); + try { + await page + .getByRole("button", { name: "Retry emoji", exact: true }) + .click(); + await expect + .poll(() => page.evaluate(() => window.emojiFixture.status("a"))) + .toBe("loading"); + // A usable Unicode-only picker during loading is not the recovered mount. + await expect(search).toHaveAttribute("data-buzz-search-ready", "true"); + await expect( + page.locator('em-emoji-picker [data-id="buzz-custom"]'), + ).toHaveCount(0); + } finally { + await page.evaluate(() => window.emojiFixture.releaseCatalog()); + } + await expect + .poll(() => page.evaluate(() => window.emojiFixture.status("a"))) + .toBe("ready"); + await expect( + page.locator('em-emoji-picker [data-id="buzz-custom"]'), + ).toHaveCount(1); + await expect(search).toHaveAttribute("data-buzz-search-ready", "true"); await search.fill("party"); await expect(insert).toBeVisible(); await draft().fill(":broken: readable"); diff --git a/tests/browser/layout.spec.mjs b/tests/browser/layout.spec.mjs index 9ddd1ecb..a10d1dec 100644 --- a/tests/browser/layout.spec.mjs +++ b/tests/browser/layout.spec.mjs @@ -48,7 +48,16 @@ async function link(page, app, target) { }), ); app.append("primary", "alpha", `Please review ${target}`); - await page.getByRole("link", { name: target, exact: true }).click(); + const trigger = page.getByRole("link", { name: target, exact: true }); + await expect(trigger).toBeVisible(); + await trigger.scrollIntoViewIfNeeded(); + // Appending and bringing an offscreen link into view can both scroll Virtua. + // These are panel-layout checks, not clicks during an in-flight correction. + await settle(page); + await expect( + page.getByRole("region", { name: "Channel message history" }).locator("ol"), + ).toHaveCSS("pointer-events", "auto"); + await trigger.click(); await expect( panel(page).getByRole("heading", { name: "A useful change" }), ).toBeVisible(); diff --git a/tests/fixtures/emoji.tsx b/tests/fixtures/emoji.tsx index e4feef05..c7b92f11 100644 --- a/tests/fixtures/emoji.tsx +++ b/tests/fixtures/emoji.tsx @@ -39,6 +39,8 @@ const sessions = ["a", "b"].map((community) => { let time = 1, fail = false; let live!: LiveCallbacks; + let catalogRead: Promise | undefined; + let releaseCatalogRead: (() => void) | undefined; const makeSet = (empty = false) => signed(member, { kind: 30030, @@ -90,7 +92,7 @@ const sessions = ["a", "b"].map((community) => { async query(filters) { if (filters[0]?.kinds?.includes(30030)) { report.reads.push(community); - await new Promise((resolve) => setTimeout(resolve, 80)); + await catalogRead; if (fail) throw new Error("Fixture catalog offline"); return [catalog]; } @@ -152,6 +154,17 @@ const sessions = ["a", "b"].map((community) => { fail(value: boolean) { fail = value; }, + holdCatalog() { + if (catalogRead) throw new Error("Catalog read already held"); + catalogRead = new Promise((resolve) => { + releaseCatalogRead = resolve; + }); + }, + releaseCatalog() { + releaseCatalogRead?.(); + catalogRead = undefined; + releaseCatalogRead = undefined; + }, }; }); Object.assign(window, { @@ -161,6 +174,8 @@ Object.assign(window, { remove: () => sessions[0]?.replace(true), fail: (value: boolean) => sessions[0]?.fail(value), refresh: () => sessions[0]?.session.emoji.refresh(), + holdCatalog: () => sessions[0]?.holdCatalog(), + releaseCatalog: () => sessions[0]?.releaseCatalog(), status: (community: string) => sessions .find((item) => item.community === community) diff --git a/tests/integration/hooks.test.mjs b/tests/integration/hooks.test.mjs index c1ad9893..ce70a866 100644 --- a/tests/integration/hooks.test.mjs +++ b/tests/integration/hooks.test.mjs @@ -18,6 +18,9 @@ const root = fileURLToPath(new URL("../../", import.meta.url)); const env = Object.fromEntries( Object.entries(process.env).filter(([key]) => !key.startsWith("GIT_")), ); +// Borrow the installed tools without letting pnpm repair the shared symlink target. +// This fixture intentionally has no workspace/patch config of its own. +env.pnpm_config_verify_deps_before_run = "false"; // These commits are disposable probe fixtures, never commits in the source checkout. env.GIT_CONFIG_NOSYSTEM = "1"; env.GIT_CONFIG_GLOBAL = "/dev/null"; @@ -76,7 +79,12 @@ function fixture(t) { return { dir, sibling, run, git, write, read, install, commit }; } -test("installed hook formats and safely fixes staged files without including other work", (t) => { +test("installed hook formats without rewriting borrowed dependencies or other work", (t) => { + const dependencies = () => + [".modules.yaml", "virtua/lib/index.js"].map((file) => + readFileSync(path.join(root, "node_modules", file), "utf8"), + ); + const installed = dependencies(); const f = fixture(t); f.write("nested/space name.ts", "export const answer={value:42}\n"); f.write("safe.ts", "export function count(){let value=1; return value;}\n"); @@ -85,6 +93,7 @@ test("installed hook formats and safely fixes staged files without including oth f.write("untracked.ts", "export const doNotAdd={value:1}\n"); const result = f.commit(); assert.equal(result.status, 0, result.stdout + result.stderr); + assert.deepEqual(dependencies(), installed, "borrowed dependencies changed"); assert.equal( f.git("show", "HEAD:nested/space name.ts"), "export const answer = { value: 42 };\n",