diff --git a/apps/web/src/components/HoverTouchpointOverlay.tsx b/apps/web/src/components/HoverTouchpointOverlay.tsx index 206c7f2ba5..b97655cf2b 100644 --- a/apps/web/src/components/HoverTouchpointOverlay.tsx +++ b/apps/web/src/components/HoverTouchpointOverlay.tsx @@ -12,6 +12,7 @@ import { webTouchpointContext, verifyWebTouchpoint, } from "./touchpoint-component"; +import { watchTouchpointVisibility } from "./touchpoint-lifecycle"; import styles from "./HoverTouchpointOverlay.module.css"; const ENTRY_PLACEMENT = "opend.home.hover-entry"; @@ -280,11 +281,9 @@ export function HoverTouchpointOverlay({ }, ); if (abandon()) return; + // Visibility is decided by the effect below, once React has + // committed `hidden`. Sampling it here races that commit. setReady(true); - requestAnimationFrame(() => { - if (!cancelled && isAuthorized() && !document.hidden && entryElement?.getClientRects().length) - onEntryVisible?.(); - }); } catch (error) { dispose(); if (!cancelled) { @@ -301,19 +300,31 @@ export function HoverTouchpointOverlay({ setReady(false); dispose(); }; - }, [close, dispatchEntryAction, dispatchLayerAction, elementReady, entry, entryActionIds, isAuthorized, layer, layerActionIds, mode, onDiagnostic, onEntryVisible, onLayerVisible]); + }, [close, dispatchEntryAction, dispatchLayerAction, elementReady, entry, entryActionIds, isAuthorized, layer, layerActionIds, mode, onDiagnostic]); useLayoutEffect(() => { if (open && ready) refreshPosition(); }, [open, ready, refreshPosition]); useEffect(() => { - if (!open || !ready) return; - const frame = requestAnimationFrame(() => { - if (!document.hidden && layerRef.current?.getClientRects().length) - onLayerVisible?.(); + const element = entryRef.current; + if (!ready || !element || !onEntryVisible) return; + return watchTouchpointVisibility({ + element, + isCurrent: isAuthorized, + onVisible: onEntryVisible, + onSlow: onDiagnostic, }); - return () => cancelAnimationFrame(frame); - }, [onLayerVisible, open, ready]); + }, [isAuthorized, onDiagnostic, onEntryVisible, ready]); + useEffect(() => { + const element = layerRef.current; + if (!open || !ready || !element || !onLayerVisible) return; + return watchTouchpointVisibility({ + element, + isCurrent: isAuthorized, + onVisible: onLayerVisible, + onSlow: onDiagnostic, + }); + }, [isAuthorized, onDiagnostic, onLayerVisible, open, ready]); useEffect(() => { if (!open || !ready) return; const reposition = () => refreshPosition(); diff --git a/apps/web/src/components/touchpoint-lifecycle.ts b/apps/web/src/components/touchpoint-lifecycle.ts index 39c3bac004..9ae33818db 100644 --- a/apps/web/src/components/touchpoint-lifecycle.ts +++ b/apps/web/src/components/touchpoint-lifecycle.ts @@ -312,6 +312,87 @@ export function useTouchpointLifecycle({ enabled, identity, load, onError }: } +/** + * A host that is laid out but still reports no box has not been committed yet; + * one that reports a box while the page is hidden was never shown. Neither can + * be decided from a single sample, so the warning only reports, never resolves. + */ +export const VISIBILITY_WARNING_MS = 5_000; + +export type TouchpointVisibilityWatch = Readonly<{ + element: HTMLElement; + isCurrent: () => boolean; + onVisible: () => void; + onSlow?: (code: string) => void; + slowAfterMs?: number; +}>; + +/** + * Resolves the first moment a mounted host is really on screen, then stops. + * + * Sampling once cannot answer this. `hidden` is bound to React state, and a + * frame scheduled in the same continuation as that state update can run before + * React commits it — the host is then still `display: none` and reports no box. + * Because the old callers never looked again, that one lost sample permanently + * suppressed the receipt for the whole session. Three sources can change the + * answer, so all three re-check: layout (`ResizeObserver`, which also fires the + * initial observation), page visibility, and the caller's own re-mount. + */ +export function watchTouchpointVisibility({ + element, + isCurrent, + onVisible, + onSlow, + slowAfterMs = VISIBILITY_WARNING_MS, +}: TouchpointVisibilityWatch): () => void { + let recorded = false; + let stopped = false; + let frame: number | undefined; + let observer: ResizeObserver | undefined; + let slowTimer: ReturnType | undefined; + const stop = () => { + if (stopped) return; + stopped = true; + if (frame !== undefined) cancelAnimationFrame(frame); + frame = undefined; + observer?.disconnect(); + if (slowTimer !== undefined) clearTimeout(slowTimer); + document.removeEventListener("visibilitychange", check); + }; + function check() { + if (stopped || recorded || frame !== undefined) return; + frame = requestAnimationFrame(() => { + frame = undefined; + if (stopped || recorded) return; + if ( + !isCurrent() || + document.hidden || + !element.isConnected || + element.hidden || + element.getClientRects().length === 0 + ) + return; + recorded = true; + stop(); + onVisible(); + }); + } + // Layout is the strongest signal but the only optional one: a host without + // `ResizeObserver` must still mount and still report, so its absence costs + // this watch a wake-up source and never the display itself. + observer = + typeof ResizeObserver === "function" ? new ResizeObserver(check) : undefined; + observer?.observe(element); + document.addEventListener("visibilitychange", check); + // A slow host is reported but keeps its watch: a late box still earns its + // receipt, and dropping the watch here would recreate the lost-sample bug. + slowTimer = setTimeout(() => { + if (!recorded && !stopped) onSlow?.("touchpoint_visibility_slow"); + }, slowAfterMs); + check(); + return stop; +} + type MountAdapter = Readonly<{ content: WebTouchpointContent; placementKey: string; @@ -342,9 +423,7 @@ export function mountTouchpoint( elementDisposed = false, verifiedDisposed = false; let verified: Awaited> | undefined; - let frame: number | undefined; - let mounted = false, - recorded = false; + let stopVisibilityWatch: (() => void) | undefined; let observer: MutationObserver | undefined; const current = () => !cancelled && adapter.isCurrent(); const dispose = () => { @@ -357,23 +436,6 @@ export function mountTouchpoint( verified.dispose(); } }; - const recordWhenVisible = () => { - if (!mounted || recorded || frame !== undefined || !adapter.onVisible) - return; - frame = requestAnimationFrame(() => { - frame = undefined; - if ( - !current() || - document.hidden || - !element.isConnected || - element.hidden || - element.getClientRects().length === 0 - ) - return; - recorded = true; - adapter.onVisible?.(); - }); - }; const fail = (code: string) => { emitWebTouchpointDiagnostic({ code }); adapter.onCloseControlChange?.(false); @@ -381,7 +443,6 @@ export function mountTouchpoint( }; adapter.onCloseControlChange?.(null); container.replaceChildren(element); - document.addEventListener("visibilitychange", recordWhenVisible); void (async () => { try { verified = await verifyWebTouchpoint(adapter.content); @@ -440,7 +501,14 @@ export function mountTouchpoint( dispose(); return; } - mounted = true; + const onVisible = adapter.onVisible; + if (onVisible) + stopVisibilityWatch = watchTouchpointVisibility({ + element, + isCurrent: current, + onVisible, + onSlow: (code) => emitWebTouchpointDiagnostic({ code }), + }); if (adapter.onCloseControlChange) { const update = () => { if (current()) @@ -471,7 +539,6 @@ export function mountTouchpoint( if (dialog) observer.observe(dialog, options); } adapter.onReady?.(); - recordWhenVisible(); } catch (error) { if (current()) fail(error instanceof Error ? error.message : "touchpoint_load_failed"); @@ -481,8 +548,7 @@ export function mountTouchpoint( return () => { cancelled = true; observer?.disconnect(); - document.removeEventListener("visibilitychange", recordWhenVisible); - if (frame !== undefined) cancelAnimationFrame(frame); + stopVisibilityWatch?.(); adapter.onCloseControlChange?.(null); dispose(); if (element.parentNode === container) container.replaceChildren(); diff --git a/apps/web/tests/components/TestCampaignHosts.test.tsx b/apps/web/tests/components/TestCampaignHosts.test.tsx index ccbbc8d9c9..69efe32e97 100644 --- a/apps/web/tests/components/TestCampaignHosts.test.tsx +++ b/apps/web/tests/components/TestCampaignHosts.test.tsx @@ -106,15 +106,38 @@ function decision(placementKey: (typeof placements)[number]): TestDecision { } describe("Test decisions at the existing host touchpoints", () => { + /** + * A host reports no box until React commits `hidden` and the package lays + * out. Pinning this to 1 for every element would assert away the very frame + * the receipt is lost in, so tests drive it and wake the observer by hand. + */ + let clientRectCount = 1; + const resizeObserverCallbacks: Array<() => void> = []; + const notifyResizeObservers = () => { + for (const trigger of [...resizeObserverCallbacks]) trigger(); + }; beforeEach(() => { localStorage.clear(); + clientRectCount = 1; + resizeObserverCallbacks.length = 0; vi.stubEnv("NEXT_PUBLIC_CMS_HOST_RELEASE", `sha256:${"a".repeat(64)}`); document.documentElement.lang = "zh-CN"; vi.stubGlobal( "ResizeObserver", class { - observe() {} - disconnect() {} + private trigger?: () => void; + constructor(private readonly callback: ResizeObserverCallback) {} + observe() { + this.trigger = () => + this.callback([], this as unknown as ResizeObserver); + resizeObserverCallbacks.push(this.trigger); + } + disconnect() { + const index = this.trigger + ? resizeObserverCallbacks.indexOf(this.trigger) + : -1; + if (index >= 0) resizeObserverCallbacks.splice(index, 1); + } }, ); (globalThis as HostGlobal).__cmsTestHost = { @@ -133,10 +156,13 @@ describe("Test decisions at the existing host touchpoints", () => { document.createTextNode("Test host content"), ); }); - vi.spyOn(HTMLElement.prototype, "getClientRects").mockReturnValue({ - length: 1, - item: () => null, - } as unknown as DOMRectList); + vi.spyOn(HTMLElement.prototype, "getClientRects").mockImplementation( + () => + ({ + length: clientRectCount, + item: () => null, + }) as unknown as DOMRectList, + ); }); afterEach(() => { clearTestRuntimeSession(); @@ -732,4 +758,56 @@ describe("Test decisions at the existing host touchpoints", () => { .querySelector("opend-touchpoint"), ).not.toBeNull(); }); + + // Delayed visibility itself is covered directly in touchpointVisibility.test.ts: + // at this level a re-mount can supply a second sample, which would hide a + // regression back to sampling once. What this level can still pin down is + // that a host nobody could see never earns a receipt. + it("never accepts a hover entry that never gains a box", async () => { + clientRectCount = 0; + const decisions = new Map( + placements.map((placementKey) => [placementKey, decision(placementKey)]), + ); + setTestRuntimeSession({ + selectionKey: "deployment-four:sha256:four-snapshot:active", + deployment: { + id: context.deploymentId, + activityId: "activity-four", + snapshotHash: "sha256:four-snapshot", + snapshot: { + contentVersionId: "version-four-placement", + manifestHash: "sha256:four-manifest", + artifactHash: "sha256:four-artifact", + placementKeys: [...placements], + }, + }, + context, + decisions, + isAuthorized: () => true, + }); + const fetchMock = vi.fn(async (url: string, _init?: RequestInit) => + url.includes("acceptances") + ? new Response(JSON.stringify({ id: "acceptance" }), { status: 201 }) + : new Response(JSON.stringify({ error: "production_read_forbidden" }), { + status: 404, + }), + ); + vi.stubGlobal("fetch", fetchMock); + render(); + const entry = (await screen.findByTestId( + "cms-hover-overlay-root", + )).querySelector("opend-touchpoint"); + expect(entry).not.toBeNull(); + await waitFor(() => expect(entry).not.toHaveAttribute("hidden")); + const entryAcceptances = () => + fetchMock.mock.calls + .filter(([url]) => url.includes("acceptances")) + .map(([, init]) => JSON.parse(String(init?.body))) + .filter((report) => report.placementKey === "opend.home.hover-entry"); + notifyResizeObservers(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 50)); + }); + expect(entryAcceptances()).toHaveLength(0); + }); }); diff --git a/apps/web/tests/components/touchpointVisibility.test.ts b/apps/web/tests/components/touchpointVisibility.test.ts new file mode 100644 index 0000000000..e1b40546db --- /dev/null +++ b/apps/web/tests/components/touchpointVisibility.test.ts @@ -0,0 +1,172 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { watchTouchpointVisibility } from "../../src/components/touchpoint-lifecycle"; + +/** + * These cover the frame the acceptance receipt used to be lost in. The old + * callers sampled visibility once, right after the state update that removes + * `hidden`; when that sample lost the race with React's commit the host was + * still `display: none`, reported no box, and nothing ever looked again. + */ +describe("watchTouchpointVisibility", () => { + let rectCount = 1; + let hidden = false; + const observerCallbacks: Array<() => void> = []; + const notifyResize = () => { + for (const trigger of [...observerCallbacks]) trigger(); + }; + const settleFrames = async () => { + for (let i = 0; i < 3; i++) + await new Promise((resolve) => requestAnimationFrame(() => resolve(null))); + }; + const host = () => { + const element = document.createElement("div"); + document.body.append(element); + return element; + }; + + beforeEach(() => { + rectCount = 1; + hidden = false; + observerCallbacks.length = 0; + // Registering on `observe`, not on construction, is what makes these + // tests able to tell an observed host from a merely constructed observer. + vi.stubGlobal( + "ResizeObserver", + class { + private trigger?: () => void; + constructor(private readonly callback: ResizeObserverCallback) {} + observe() { + this.trigger = () => + this.callback([], this as unknown as ResizeObserver); + observerCallbacks.push(this.trigger); + } + disconnect() { + const index = this.trigger + ? observerCallbacks.indexOf(this.trigger) + : -1; + if (index >= 0) observerCallbacks.splice(index, 1); + } + }, + ); + vi.spyOn(document, "hidden", "get").mockImplementation(() => hidden); + vi.spyOn(HTMLElement.prototype, "getClientRects").mockImplementation( + () => ({ length: rectCount, item: () => null }) as unknown as DOMRectList, + ); + }); + afterEach(() => { + document.body.replaceChildren(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("waits for a host that only gains its box after the first frame", async () => { + rectCount = 0; + const onVisible = vi.fn(); + const stop = watchTouchpointVisibility({ + element: host(), + isCurrent: () => true, + onVisible, + }); + await settleFrames(); + expect(onVisible).not.toHaveBeenCalled(); + rectCount = 1; + notifyResize(); + await settleFrames(); + expect(onVisible).toHaveBeenCalledTimes(1); + stop(); + }); + + it("reports a host at most once however often its layout changes", async () => { + const onVisible = vi.fn(); + const stop = watchTouchpointVisibility({ + element: host(), + isCurrent: () => true, + onVisible, + }); + await settleFrames(); + notifyResize(); + notifyResize(); + await settleFrames(); + expect(onVisible).toHaveBeenCalledTimes(1); + stop(); + }); + + it("does not report a laid-out host while the page is hidden, and recovers when it returns", async () => { + hidden = true; + const onVisible = vi.fn(); + const stop = watchTouchpointVisibility({ + element: host(), + isCurrent: () => true, + onVisible, + }); + await settleFrames(); + expect(onVisible).not.toHaveBeenCalled(); + hidden = false; + document.dispatchEvent(new Event("visibilitychange")); + await settleFrames(); + expect(onVisible).toHaveBeenCalledTimes(1); + stop(); + }); + + it("never reports a host whose authority lapsed while it was waiting", async () => { + rectCount = 0; + let authorized = true; + const onVisible = vi.fn(); + const stop = watchTouchpointVisibility({ + element: host(), + isCurrent: () => authorized, + onVisible, + }); + await settleFrames(); + authorized = false; + rectCount = 1; + notifyResize(); + await settleFrames(); + expect(onVisible).not.toHaveBeenCalled(); + stop(); + }); + + it("stops watching once released, so a later layout cannot report a released host", async () => { + rectCount = 0; + const onVisible = vi.fn(); + const stop = watchTouchpointVisibility({ + element: host(), + isCurrent: () => true, + onVisible, + }); + stop(); + rectCount = 1; + notifyResize(); + document.dispatchEvent(new Event("visibilitychange")); + await settleFrames(); + expect(onVisible).not.toHaveBeenCalled(); + }); + + it("reports a slow host without giving up on it", async () => { + vi.useFakeTimers(); + try { + rectCount = 0; + const onVisible = vi.fn(); + const onSlow = vi.fn(); + const stop = watchTouchpointVisibility({ + element: host(), + isCurrent: () => true, + onVisible, + onSlow, + slowAfterMs: 1_000, + }); + vi.advanceTimersByTime(1_500); + expect(onSlow).toHaveBeenCalledWith("touchpoint_visibility_slow"); + expect(onVisible).not.toHaveBeenCalled(); + rectCount = 1; + notifyResize(); + await vi.advanceTimersByTimeAsync(100); + expect(onVisible).toHaveBeenCalledTimes(1); + stop(); + } finally { + vi.useRealTimers(); + } + }); +});