diff --git a/crates/trusted-server-core/src/integrations/gpt.rs b/crates/trusted-server-core/src/integrations/gpt.rs index e21058a21..f53a21cf0 100644 --- a/crates/trusted-server-core/src/integrations/gpt.rs +++ b/crates/trusted-server-core/src/integrations/gpt.rs @@ -1222,6 +1222,35 @@ mod tests { ); } + #[test] + fn head_inserts_bootstrap_installs_inner_div_slot_handoff() { + let integration = GptIntegration::new(test_config()); + let doc_state = IntegrationDocumentState::default(); + let ctx = IntegrationHtmlContext { + request_host: "edge.example.com", + request_scheme: "https", + origin_host: "example.com", + document_state: &doc_state, + }; + let combined = integration.head_inserts(&ctx).join(""); + assert!( + combined.contains("gptSlotHandoffs"), + "bootstrap should keep late publisher slot handoff state on window.tsjs" + ); + assert!( + combined.contains("__tsSlotHandoffPatched"), + "bootstrap should install idempotent GPT handoff wrappers" + ); + assert!( + combined.contains("return googletag.defineSlot") && combined.contains("actualDivId"), + "bootstrap should define the TS fallback on the actual inner div" + ); + assert!( + !combined.contains("actualDivId + \"-container\""), + "bootstrap must not define a competing outer-container GPT slot" + ); + } + #[test] fn head_inserts_bootstrap_guards_enable_services_with_idempotency_flag() { let config = test_config(); diff --git a/crates/trusted-server-core/src/integrations/gpt_bootstrap.js b/crates/trusted-server-core/src/integrations/gpt_bootstrap.js index cc4c5c00c..0c2697357 100644 --- a/crates/trusted-server-core/src/integrations/gpt_bootstrap.js +++ b/crates/trusted-server-core/src/integrations/gpt_bootstrap.js @@ -42,6 +42,127 @@ pubads.__tsInitialLoadHooked = true; }); + function findSlotByElementId(pubads, elementId) { + var slots = pubads.getSlots ? pubads.getSlots() : []; + return ( + slots.find(function (slot) { + return slot.getSlotElementId() === elementId; + }) || null + ); + } + + function runHandoffInternal(callback) { + var wasInternal = ts.gptSlotHandoffInternal; + ts.gptSlotHandoffInternal = true; + try { + return callback(); + } finally { + ts.gptSlotHandoffInternal = wasInternal; + } + } + + // TS cannot wait an arbitrary amount of time for a framework to define a + // slot: publishers that never define one would render blank. Instead, TS + // defines its fallback on the actual inner div and aliases only a later + // publisher defineSlot() for that exact div to the same GPT slot. + function installSlotHandoff() { + window.googletag.cmd.push(function () { + var tag = window.googletag; + var pubads = tag.pubads && tag.pubads(); + if (!tag.defineSlot || !tag.display || !pubads) return; + + if (!tag.defineSlot.__tsSlotHandoffPatched) { + var originalDefineSlot = tag.defineSlot.bind(tag); + var patchedDefineSlot = function (adUnitPath, formats, elementId) { + var handoff = ts.gptSlotHandoffs && ts.gptSlotHandoffs[elementId]; + if (!ts.gptSlotHandoffInternal && handoff) { + var existingSlot = findSlotByElementId(pubads, elementId); + if (existingSlot) { + if (!handoff.publisherClaimed) { + handoff.publisherClaimed = true; + handoff.suppressPublisherDisplay = true; + handoff.suppressPublisherRefresh = + ts.gptInitialLoadDisabled === true; + ts.prevGptSlots = (ts.prevGptSlots || []).filter( + function (ownedSlot) { + return ownedSlot !== existingSlot; + }, + ); + if ( + handoff.gamUnitPath !== adUnitPath || + JSON.stringify(handoff.formats) !== JSON.stringify(formats) + ) { + ts.log && + ts.log.warn && + ts.log.warn( + "GPT slot handoff: publisher definition differs from TS configuration", + elementId, + ); + } + } + return existingSlot; + } + } + return originalDefineSlot(adUnitPath, formats, elementId); + }; + patchedDefineSlot.__tsSlotHandoffPatched = true; + tag.defineSlot = patchedDefineSlot; + } + + if (!tag.display.__tsSlotHandoffPatched) { + var originalDisplay = tag.display.bind(tag); + var patchedDisplay = function (elementId) { + var handoff = ts.gptSlotHandoffs && ts.gptSlotHandoffs[elementId]; + if ( + !ts.gptSlotHandoffInternal && + handoff && + handoff.suppressPublisherDisplay + ) { + handoff.suppressPublisherDisplay = false; + return; + } + originalDisplay(elementId); + }; + patchedDisplay.__tsSlotHandoffPatched = true; + tag.display = patchedDisplay; + } + + if (!pubads.refresh.__tsSlotHandoffPatched) { + var originalRefresh = pubads.refresh.bind(pubads); + var patchedRefresh = function (requestedSlots) { + if (ts.gptSlotHandoffInternal) { + originalRefresh(requestedSlots); + return; + } + var slots = + requestedSlots || (pubads.getSlots ? pubads.getSlots() : null); + if (!slots) { + originalRefresh(requestedSlots); + return; + } + var suppressed = false; + var remainingSlots = slots.filter(function (slot) { + var handoff = + ts.gptSlotHandoffs && ts.gptSlotHandoffs[slot.getSlotElementId()]; + if (!handoff || !handoff.suppressPublisherRefresh) return true; + handoff.suppressPublisherRefresh = false; + suppressed = true; + return false; + }); + if (!suppressed) { + originalRefresh(requestedSlots); + } else if (remainingSlots.length > 0) { + originalRefresh(remainingSlots); + } + }; + patchedRefresh.__tsSlotHandoffPatched = true; + pubads.refresh = patchedRefresh; + } + }); + } + + installSlotHandoff(); + ts.adInit = function () { var slots = ts.adSlots || []; var bids = ts.bids || {}; @@ -88,15 +209,26 @@ }) || null; var tsOwned = false; if (!s) { - // Use outer container div for TS's slot when publisher hasn't defined - // theirs yet — keeps both slots on separate divs so publisher's - // later defineSlot on the inner div doesn't conflict. - var containerEl = document.getElementById(actualDivId + "-container"); - var slotDivId = containerEl ? containerEl.id : actualDivId; - s = googletag.defineSlot(slot.gam_unit_path, slot.formats, slotDivId); + // Define TS's fallback on the publisher's actual div. The scoped + // handoff wrapper returns this slot if the publisher defines it later. + s = runHandoffInternal(function () { + return googletag.defineSlot( + slot.gam_unit_path, + slot.formats, + actualDivId, + ); + }); if (!s) return; s.addService(googletag.pubads()); tsOwned = true; + ts.gptSlotHandoffs = ts.gptSlotHandoffs || {}; + ts.gptSlotHandoffs[actualDivId] = { + gamUnitPath: slot.gam_unit_path, + formats: slot.formats, + publisherClaimed: false, + suppressPublisherDisplay: false, + suppressPublisherRefresh: false, + }; } Object.entries(slot.targeting || {}).forEach(function (e) { @@ -113,11 +245,9 @@ }); // Keep in sync with TS_INITIAL_TARGETING_KEY in index.ts s.setTargeting("ts_initial", "1"); - // Map both the inner div and the GPT slot's element ID (the - // "-container" div when TS defined the slot there) into divToSlotId. - // This bootstrap fires no beacons and registers no slotRenderEnded - // listener; the map is consumed by the bundle's render bridge (index.ts) - // once it loads, which reports the GPT slot element ID. + // Map the resolved inner div to the slot ID. This bootstrap fires no + // beacons and registers no slotRenderEnded listener; the map is consumed + // by the bundle's render bridge (index.ts) once it loads. divToSlotId[actualDivId] = slot.id; var slotElementId = s.getSlotElementId(); if (slotElementId && slotElementId !== actualDivId) { @@ -143,7 +273,9 @@ // impression. Runs after enableServices(); on SPA navigation services are // already enabled, so this runs unconditionally for new slots. slotsToDisplay.forEach(function (divId) { - googletag.display(divId); + runHandoffInternal(function () { + googletag.display(divId); + }); }); // Reused publisher-owned slots always need a refresh to pick up the // server-side targeting. TS-defined slots are fetched by display() above @@ -161,7 +293,9 @@ // bundle's adInit() in crates/trusted-server-js/lib/src/integrations/gpt/index.ts. ts.adInitRefreshInProgress = true; try { - googletag.pubads().refresh(slotsNeedingRefresh); + runHandoffInternal(function () { + googletag.pubads().refresh(slotsNeedingRefresh); + }); } finally { ts.adInitRefreshInProgress = false; } diff --git a/crates/trusted-server-js/lib/src/core/types.ts b/crates/trusted-server-js/lib/src/core/types.ts index 360e2aa49..cd25133d1 100644 --- a/crates/trusted-server-js/lib/src/core/types.ts +++ b/crates/trusted-server-js/lib/src/core/types.ts @@ -63,6 +63,20 @@ export interface AuctionBidData { debug_bid?: AuctionDebugBidData; } +/** + * Lifecycle state for a GPT slot TS created before its publisher declares it. + * + * Stored on `window.tsjs` so the head bootstrap and the full TSJS bundle share + * one handoff protocol. + */ +export interface GptSlotHandoff { + gamUnitPath: string; + formats: Array<[number, number]>; + publisherClaimed: boolean; + suppressPublisherDisplay: boolean; + suppressPublisherRefresh: boolean; +} + export interface TsjsApi { version: string; que: Array<() => void>; @@ -121,6 +135,10 @@ export interface TsjsApi { * defined slots so they are not left blank. */ gptInitialLoadDisabled?: boolean; + /** Late publisher claims for TS-created GPT slots, keyed by actual div ID. */ + gptSlotHandoffs?: Record; + /** True only while TS calls a GPT function that the handoff wrappers observe. */ + gptSlotHandoffInternal?: boolean; /** Guards SPA pushState hook installation. */ spaHookInstalled?: boolean; } diff --git a/crates/trusted-server-js/lib/src/integrations/gpt/index.ts b/crates/trusted-server-js/lib/src/integrations/gpt/index.ts index ca4689684..8853997c3 100644 --- a/crates/trusted-server-js/lib/src/integrations/gpt/index.ts +++ b/crates/trusted-server-js/lib/src/integrations/gpt/index.ts @@ -1,5 +1,5 @@ import { log } from '../../core/log'; -import type { AuctionSlot, AuctionBidData, TsjsApi } from '../../core/types'; +import type { AuctionSlot, AuctionBidData, GptSlotHandoff, TsjsApi } from '../../core/types'; import { installGptGuard } from './script_guard'; @@ -445,9 +445,143 @@ function installInitialLoadDetector(ts: TsjsApi): void { }); } +interface HandoffPatchedFunction { + __tsSlotHandoffPatched?: boolean; +} + +function findGptSlotByElementId( + pubads: GoogleTagPubAdsService, + elementId: string +): GoogleTagSlot | undefined { + return pubads.getSlots?.().find((slot) => slot.getSlotElementId() === elementId); +} + +function handoffForSlot(ts: TsjsApi, slot: GoogleTagSlot): GptSlotHandoff | undefined { + return ts.gptSlotHandoffs?.[slot.getSlotElementId()]; +} + +function withGptSlotHandoffInternal(ts: TsjsApi, callback: () => T): T { + const wasInternal = ts.gptSlotHandoffInternal; + ts.gptSlotHandoffInternal = true; + try { + return callback(); + } finally { + ts.gptSlotHandoffInternal = wasInternal; + } +} + +/** + * Reuse a TS-created inner-div slot when its publisher defines that div later. + * + * TS cannot wait an arbitrary amount of time for framework hydration: doing so + * would leave placements blank when no publisher slot is ever defined. Instead, + * TS creates its fallback on the publisher's actual div and aliases only a later + * `defineSlot()` for that exact div. The first duplicate publisher request is + * suppressed because TS has already issued the initial request with TS targeting. + */ +function installLatePublisherSlotHandoff(ts: TsjsApi): void { + const win = window as GptWindow; + const cmd = win.googletag?.cmd; + if (!cmd) return; + + cmd.push(() => { + const g = win.googletag; + const pubads = g?.pubads?.(); + if (!g?.defineSlot || !g.display || !pubads) return; + + const defineSlot = g.defineSlot; + if (!(defineSlot as HandoffPatchedFunction).__tsSlotHandoffPatched) { + const originalDefineSlot = defineSlot.bind(g); + const patchedDefineSlot = ( + adUnitPath: string, + formats: Array, + elementId: string + ): GoogleTagSlot | null => { + const handoff = ts.gptSlotHandoffs?.[elementId]; + if (!ts.gptSlotHandoffInternal && handoff) { + const existingSlot = findGptSlotByElementId(pubads, elementId); + if (existingSlot) { + if (!handoff.publisherClaimed) { + handoff.publisherClaimed = true; + handoff.suppressPublisherDisplay = true; + handoff.suppressPublisherRefresh = ts.gptInitialLoadDisabled === true; + ts.prevGptSlots = (ts.prevGptSlots ?? []).filter( + (ownedSlot) => ownedSlot !== existingSlot + ); + if ( + handoff.gamUnitPath !== adUnitPath || + JSON.stringify(handoff.formats) !== JSON.stringify(formats) + ) { + log.warn('GPT slot handoff: publisher definition differs from TS configuration', { + elementId, + tsGamUnitPath: handoff.gamUnitPath, + publisherGamUnitPath: adUnitPath, + }); + } + } + return existingSlot; + } + } + return originalDefineSlot(adUnitPath, formats, elementId); + }; + (patchedDefineSlot as HandoffPatchedFunction).__tsSlotHandoffPatched = true; + g.defineSlot = patchedDefineSlot; + } + + const display = g.display; + if (!(display as HandoffPatchedFunction).__tsSlotHandoffPatched) { + const originalDisplay = display.bind(g); + const patchedDisplay = (elementId: string): void => { + const handoff = ts.gptSlotHandoffs?.[elementId]; + if (!ts.gptSlotHandoffInternal && handoff?.suppressPublisherDisplay) { + handoff.suppressPublisherDisplay = false; + return; + } + originalDisplay(elementId); + }; + (patchedDisplay as HandoffPatchedFunction).__tsSlotHandoffPatched = true; + g.display = patchedDisplay; + } + + const refresh = pubads.refresh; + if (!(refresh as HandoffPatchedFunction).__tsSlotHandoffPatched) { + const originalRefresh = refresh.bind(pubads); + const patchedRefresh = (requestedSlots?: GoogleTagSlot[]): void => { + if (ts.gptSlotHandoffInternal) { + originalRefresh(requestedSlots); + return; + } + + const slots = requestedSlots ?? pubads.getSlots?.(); + if (!slots) { + originalRefresh(requestedSlots); + return; + } + + let suppressed = false; + const remainingSlots = slots.filter((slot) => { + const handoff = handoffForSlot(ts, slot); + if (!handoff?.suppressPublisherRefresh) return true; + handoff.suppressPublisherRefresh = false; + suppressed = true; + return false; + }); + if (!suppressed) { + originalRefresh(requestedSlots); + } else if (remainingSlots.length > 0) { + originalRefresh(remainingSlots); + } + }; + (patchedRefresh as HandoffPatchedFunction).__tsSlotHandoffPatched = true; + pubads.refresh = patchedRefresh; + } + }); +} + export function installTsAdInit(): void { const ts = (window.tsjs ??= {} as TsjsApi); installInitialLoadDetector(ts); + installLatePublisherSlotHandoff(ts); ts.adInit = function () { const slots = ts.adSlots ?? []; // Snapshot bids at adInit() call time — correct for targeting setup. @@ -517,16 +651,23 @@ export function installTsAdInit(): void { if (existingSlot) { gptSlot = existingSlot; } else { - // Use outer container div for TS's slot when publisher hasn't defined - // theirs yet — keeps both slots on separate divs so publisher's - // later defineSlot on the inner div doesn't conflict. - const containerEl = document.getElementById(`${actualDivId}-container`); - const slotDivId = containerEl?.id ?? actualDivId; - const defined = g.defineSlot?.(slot.gam_unit_path, slot.formats, slotDivId); + // Define TS's fallback on the publisher's actual div. A late publisher + // defineSlot() for this div is handed the same slot by the scoped GPT + // wrapper, preventing a competing container-slot request. + const defined = withGptSlotHandoffInternal(ts, () => + g.defineSlot?.(slot.gam_unit_path, slot.formats, actualDivId) + ); if (!defined) return; defined.addService(g.pubads!()); gptSlot = defined; tsOwned = true; + (ts.gptSlotHandoffs ??= {})[actualDivId] = { + gamUnitPath: slot.gam_unit_path, + formats: slot.formats, + publisherClaimed: false, + suppressPublisherDisplay: false, + suppressPublisherRefresh: false, + }; } const slotDivId2 = gptSlot.getSlotElementId?.() ?? actualDivId; @@ -541,9 +682,8 @@ export function installTsAdInit(): void { if (bid[key]) gptSlot.setTargeting(key, String(bid[key]!)); }); gptSlot.setTargeting(TS_INITIAL_TARGETING_KEY, '1'); - // Map both inner div and container div → slot ID so slotRenderEnded - // (which reports the GPT slot's div, i.e. slotDivId/container) can look up - // the slot, while adm injection (which targets the inner div) also works. + // Map the resolved inner div to the slot ID so slotRenderEnded and ADM + // injection address the same, single GPT slot. divToSlotId[actualDivId] = slot.id; if (slotDivId2 !== actualDivId) divToSlotId[slotDivId2] = slot.id; const slotTargetingKeys = Object.keys(slot.targeting ?? {}); @@ -607,7 +747,7 @@ export function installTsAdInit(): void { // called without a matching display call") and misses its impression. // Must run after enableServices(); on SPA navigation services are already // enabled, so this runs unconditionally for any newly-defined slots. - slotsToDisplay.forEach((divId) => g.display?.(divId)); + slotsToDisplay.forEach((divId) => withGptSlotHandoffInternal(ts, () => g.display?.(divId))); // Slots needing an explicit ad request via refresh(). Reused // publisher-owned slots always need one to pick up the just-applied @@ -630,7 +770,7 @@ export function installTsAdInit(): void { // the same slots still go through the wrapper normally. ts.adInitRefreshInProgress = true; try { - g.pubads!().refresh(slotsNeedingRefresh); + withGptSlotHandoffInternal(ts, () => g.pubads!().refresh(slotsNeedingRefresh)); } finally { ts.adInitRefreshInProgress = false; } diff --git a/crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts b/crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts index 4a6368768..f99d90fa7 100644 --- a/crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts +++ b/crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts @@ -145,12 +145,13 @@ describe('installTsAdInit', () => { getSlotElementId: vi.fn().mockReturnValue('div-atf-sidebar'), getTargeting: vi.fn().mockReturnValue([]), }; + const nativeRefresh = vi.fn(); const mockPubads = { enableSingleRequest: vi.fn(), // Publisher has not defined this slot, so TS defines (owns) it. getSlots: vi.fn().mockReturnValue([]), addEventListener: vi.fn(), - refresh: vi.fn(), + refresh: nativeRefresh, }; const defineSlotMock = vi.fn().mockReturnValue(mockSlot); const displayMock = vi.fn(); @@ -184,7 +185,171 @@ describe('installTsAdInit', () => { expect(displayMock).toHaveBeenCalledWith('div-atf-sidebar'); // TS-owned slots are displayed, not refreshed (refresh() no-ops for a slot // that was never displayed). - expect(mockPubads.refresh).not.toHaveBeenCalled(); + expect(nativeRefresh).not.toHaveBeenCalled(); + }); + + it('hands a late publisher definition the TS inner-div slot without a second request', async () => { + type FakeSlot = { + addService(service: unknown): FakeSlot; + setTargeting(key: string, value: string | string[]): FakeSlot; + getSlotElementId(): string; + getTargeting(key?: string): string[]; + }; + const slots = new Map(); + const requests: string[] = []; + const makeSlot = (elementId: string): FakeSlot => ({ + addService: vi.fn().mockReturnThis(), + setTargeting: vi.fn().mockReturnThis(), + getSlotElementId: vi.fn().mockReturnValue(elementId), + getTargeting: vi.fn().mockReturnValue([]), + }); + const pubads = { + enableSingleRequest: vi.fn(), + getSlots: vi.fn(() => Array.from(slots.values())), + addEventListener: vi.fn(), + refresh: vi.fn((requestedSlots?: FakeSlot[]) => { + (requestedSlots ?? Array.from(slots.values())).forEach((slot) => + requests.push(slot.getSlotElementId()) + ); + }), + }; + const nativeDefineSlot = vi.fn( + (_adUnitPath: string, _formats: number[][], elementId: string) => { + const slot = makeSlot(elementId); + slots.set(elementId, slot); + return slot; + } + ); + const nativeDisplay = vi.fn((elementId: string) => requests.push(elementId)); + const destroySlots = vi.fn(); + const googletag = { + cmd: { push: vi.fn((fn: () => void) => fn()) }, + defineSlot: nativeDefineSlot, + display: nativeDisplay, + pubads: vi.fn().mockReturnValue(pubads), + destroySlots, + enableServices: vi.fn(), + }; + (window as TestWindow).googletag = googletag; + (window as TestWindow).tsjs = { + adSlots: [ + { + id: 'atf_sidebar_ad', + gam_unit_path: '/123/atf', + div_id: 'div-atf-sidebar', + formats: [[300, 250]], + targeting: {}, + }, + ], + bids: {}, + }; + + const { installTsAdInit } = await import('../../../src/integrations/gpt/index'); + installTsAdInit(); + (window as TestWindow).tsjs!.adInit!(); + + const publisherDefineSlot = googletag.defineSlot as unknown as ( + adUnitPath: string, + formats: number[][], + elementId: string + ) => FakeSlot; + const publisherDisplay = googletag.display as unknown as (elementId: string) => void; + const publisherSlot = publisherDefineSlot('/123/atf', [[300, 250]], 'div-atf-sidebar'); + publisherSlot.addService(pubads); + publisherDisplay('div-atf-sidebar'); + + expect(nativeDefineSlot).toHaveBeenCalledTimes(1); + expect(nativeDisplay).toHaveBeenCalledTimes(1); + expect(requests).toEqual(['div-atf-sidebar']); + expect((window as TestWindow).tsjs!.prevGptSlots).toEqual([]); + + (window as TestWindow).tsjs!.adSlots = []; + (window as TestWindow).tsjs!.adInit!(); + expect(destroySlots).not.toHaveBeenCalled(); + }); + + it('suppresses only the claimed slot from the first disabled-load publisher refresh', async () => { + type FakeSlot = { + addService(service: unknown): FakeSlot; + setTargeting(key: string, value: string | string[]): FakeSlot; + getSlotElementId(): string; + getTargeting(key?: string): string[]; + }; + const slots = new Map(); + const requests: string[] = []; + let initialLoadDisabled = false; + const makeSlot = (elementId: string): FakeSlot => ({ + addService: vi.fn().mockReturnThis(), + setTargeting: vi.fn().mockReturnThis(), + getSlotElementId: vi.fn().mockReturnValue(elementId), + getTargeting: vi.fn().mockReturnValue([]), + }); + const pubads = { + enableSingleRequest: vi.fn(), + getSlots: vi.fn(() => Array.from(slots.values())), + addEventListener: vi.fn(), + refresh: vi.fn((requestedSlots?: FakeSlot[]) => { + (requestedSlots ?? Array.from(slots.values())).forEach((slot) => + requests.push(slot.getSlotElementId()) + ); + }), + disableInitialLoad: vi.fn(() => { + initialLoadDisabled = true; + }), + }; + const nativeDefineSlot = vi.fn( + (_adUnitPath: string, _formats: number[][], elementId: string) => { + const slot = makeSlot(elementId); + slots.set(elementId, slot); + return slot; + } + ); + const nativeDisplay = vi.fn((elementId: string) => { + if (!initialLoadDisabled) requests.push(elementId); + }); + const googletag = { + cmd: { push: vi.fn((fn: () => void) => fn()) }, + defineSlot: nativeDefineSlot, + display: nativeDisplay, + pubads: vi.fn().mockReturnValue(pubads), + enableServices: vi.fn(), + }; + (window as TestWindow).googletag = googletag; + (window as TestWindow).tsjs = { + adSlots: [ + { + id: 'atf_sidebar_ad', + gam_unit_path: '/123/atf', + div_id: 'div-atf-sidebar', + formats: [[300, 250]], + targeting: {}, + }, + ], + bids: {}, + }; + + const { installTsAdInit } = await import('../../../src/integrations/gpt/index'); + installTsAdInit(); + pubads.disableInitialLoad(); + (window as TestWindow).tsjs!.adInit!(); + + const publisherDefineSlot = googletag.defineSlot as unknown as ( + adUnitPath: string, + formats: number[][], + elementId: string + ) => FakeSlot; + const publisherDisplay = googletag.display as unknown as (elementId: string) => void; + const publisherRefresh = pubads.refresh as unknown as () => void; + const publisherSlot = publisherDefineSlot('/123/atf', [[300, 250]], 'div-atf-sidebar'); + publisherSlot.addService(pubads); + publisherDisplay('div-atf-sidebar'); + slots.set('div-unrelated', makeSlot('div-unrelated')); + publisherRefresh(); + + expect(nativeDefineSlot).toHaveBeenCalledTimes(1); + expect(nativeDisplay).toHaveBeenCalledTimes(1); + expect(requests.filter((elementId) => elementId === 'div-atf-sidebar')).toHaveLength(1); + expect(requests).toContain('div-unrelated'); }); it('refreshes TS-defined slots when the publisher disabled GPT initial load', async () => { @@ -197,12 +362,13 @@ describe('installTsAdInit', () => { getSlotElementId: vi.fn().mockReturnValue('div-atf-sidebar'), getTargeting: vi.fn().mockReturnValue([]), }; + const nativeRefresh = vi.fn(); const mockPubads = { enableSingleRequest: vi.fn(), // Publisher has not defined this slot, so TS defines (owns) it. getSlots: vi.fn().mockReturnValue([]), addEventListener: vi.fn(), - refresh: vi.fn(), + refresh: nativeRefresh, disableInitialLoad: vi.fn(), }; const displayMock = vi.fn(); @@ -240,7 +406,7 @@ describe('installTsAdInit', () => { // The slot is still registered via display(), and additionally refreshed so // it actually requests an ad under disableInitialLoad(). expect(displayMock).toHaveBeenCalledWith('div-atf-sidebar'); - expect(mockPubads.refresh).toHaveBeenCalledWith([mockSlot]); + expect(nativeRefresh).toHaveBeenCalledWith([mockSlot]); }); it('sets adInitRefreshInProgress only for the duration of the internal refresh', async () => { diff --git a/docs/superpowers/plans/2026-07-24-prevent-duplicate-gpt-slot-requests.md b/docs/superpowers/plans/2026-07-24-prevent-duplicate-gpt-slot-requests.md new file mode 100644 index 000000000..4699e3c42 --- /dev/null +++ b/docs/superpowers/plans/2026-07-24-prevent-duplicate-gpt-slot-requests.md @@ -0,0 +1,219 @@ +# Prevent Duplicate GPT Slot Requests — Implementation Plan + +> **Status:** Implemented locally; production-like browser validation remains pending. +> +> **Spec:** `docs/superpowers/specs/2026-07-24-prevent-duplicate-gpt-slot-requests-design.md` + +**Goal:** Ensure one GPT slot and one initial request per configured placement when +TS `adInit()` runs before a publisher later defines the placement's inner GPT div. + +**Architecture:** TS creates its fallback on the resolved inner div and records a +handoff claim. Narrow, idempotent wrappers around GPT's `defineSlot`, `display`, and +`pubads().refresh` alias a matching late publisher definition to that slot and +suppress only the duplicate initial publisher request. A successful handoff transfers +SPA-destruction ownership to the publisher. The head bootstrap and full TSJS bundle +share this runtime protocol through `window.tsjs`. + +**Primary files:** + +- `crates/trusted-server-js/lib/src/core/types.ts` +- `crates/trusted-server-js/lib/src/integrations/gpt/index.ts` +- `crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts` +- `crates/trusted-server-js/lib/test/integrations/gpt/index.test.ts` +- `crates/trusted-server-core/src/integrations/gpt_bootstrap.js` +- `crates/trusted-server-core/src/integrations/gpt.rs` + +## Preconditions + +- [ ] Confirm with the issue owner that the intended late-owner behavior is slot + handoff (publisher receives the existing inner-div slot), not a hydration-delay + policy. +- [ ] Capture representative publisher call sequences for normal initial load and + `disableInitialLoad()` before changing wrappers. The expected sequence is + `defineSlot` → `addService` → `display`; initial-load-disabled pages additionally + call `refresh`. +- [ ] Establish an automated fake-GPT request counter: calling native `display` with + initial load enabled, or native `refresh` with initial load disabled, records a + request. Assertions must use this counter rather than only `getSlots()`. + +## Task 1: Add the shared handoff state and typed GPT wrapper surface + +**Files:** + +- Modify `crates/trusted-server-js/lib/src/core/types.ts` +- Modify `crates/trusted-server-js/lib/src/integrations/gpt/index.ts` + +- [ ] Add a `TsjsApi` property for a div-ID-keyed handoff registry. Each entry must + retain serializable lifecycle flags: TS-created, ownership-transferred, initial + request made, and one-shot publisher display/refresh suppression state. +- [ ] Add only the minimal optional/internal type surface needed for idempotence + markers on GPT functions and `pubads`. Do not weaken the public GPT types with + `any`. +- [ ] Add helper functions in `index.ts` to: + - find a live GPT slot by exact element ID; + - register and retrieve a claim; + - remove a transferred slot from `ts.prevGptSlots`; + - run an internal TS GPT call behind a short-lived guard; + - filter a requested refresh list (including no-argument/global refresh) by the + entries whose one-shot publisher refresh must be suppressed. +- [ ] Keep the registry on `window.tsjs`, not in module scope, so the bootstrap state + survives bundle loading. + +**Focused checks:** + +```bash +cd crates/trusted-server-js/lib +npx vitest run test/integrations/gpt/ad_init.test.ts test/integrations/gpt/index.test.ts +``` + +## Task 2: Install scoped idempotent handoff wrappers + +**File:** `crates/trusted-server-js/lib/src/integrations/gpt/index.ts` + +- [ ] From the GPT command queue, install wrappers once GPT exposes the real methods. + Mark the wrapped functions/service so a later `installTsAdInit()` call or the + bootstrap-to-bundle handoff cannot stack wrappers. +- [ ] `defineSlot` wrapper: + - pass through TS-internal calls and IDs absent from the registry; + - for a late publisher call on a claimed inner div, find and return the existing + slot without calling native `defineSlot`; + - mark ownership transferred and remove that slot from `prevGptSlots` before + returning it; + - log, but do not create a second slot, if publisher arguments differ from the TS + configuration. +- [ ] `display` wrapper: consume the one permitted publisher post-handoff display + call without invoking native `display`; pass every other call through unchanged. +- [ ] `refresh` wrapper: when initial load was disabled, consume the one permitted + post-handoff refresh for each claimed slot. If called with no slot list, expand + `getSlots()`, filter only the claimed slots, and forward the remaining slots + explicitly. Preserve all unrelated refreshes. +- [ ] Ensure wrapper installation precedes the fallback definition path and does not + change existing publisher-owned-slot behavior. + +**Focused checks:** + +```bash +cd crates/trusted-server-js/lib +npx vitest run test/integrations/gpt/ad_init.test.ts +``` + +## Task 3: Change fallback creation to the actual inner div + +**File:** `crates/trusted-server-js/lib/src/integrations/gpt/index.ts` + +- [ ] Delete the `${actualDivId}-container` fallback selection. When no existing + publisher slot is found, call `defineSlot` with `actualDivId`. +- [ ] Register the handoff claim immediately after successful TS definition. +- [ ] Keep `display()` for TS-created slots; with initial load disabled, retain the + single TS `refresh()` that makes the required initial request. +- [ ] Simplify `divToSlotId` and `prevSlotTargetingKeys` to the actual inner div; + remove only mappings that existed exclusively for the container fallback. +- [ ] On SPA navigation, destroy only claims that remain TS-owned. A transferred + claim must participate in stale-targeting cleanup but never be passed to + `destroySlots()`. +- [ ] Retain exact match then prefix-based dynamic-ID lookup; do not interpolate + publisher-provided IDs into CSS selectors. + +## Task 4: Add request-level regression coverage for the full bundle + +**Files:** + +- Modify `crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts` +- Modify `crates/trusted-server-js/lib/test/integrations/gpt/index.test.ts` if the + shared wrapper setup belongs there + +- [ ] Introduce a reusable fake GPT fixture that models slots by element ID and + records native `defineSlot`, `display`, `refresh`, and request events. Its + `getSlots()` result must update when a slot is defined so the test cannot pass by + asserting a stale static array. +- [ ] Add a failing regression test for the critical sequence: + 1. TS finds the inner div and runs `adInit()` before publisher setup; + 2. TS defines/displays the inner div and makes one request; + 3. publisher calls `defineSlot(innerDiv).addService(...); display(innerDiv)`; + 4. assert native `defineSlot` was called once, there is one slot, and there is one + request. +- [ ] Add the same sequence with `disableInitialLoad()`: TS display plus its refresh + makes one request; the publisher's first refresh cannot make a second request. +- [ ] Add a no-argument publisher refresh test containing an unrelated slot. Assert + the claimed slot is suppressed once and the unrelated slot is refreshed. +- [ ] Add an already publisher-owned test proving TS does not install a claim, applies + targeting, and refreshes that slot. +- [ ] Add a no-publisher test proving TS still creates, displays, and requests its + inner-div slot exactly once. +- [ ] Add a SPA handoff test: after late publisher claim, the next `adInit()` does not + destroy the transferred slot, clears old TS keys, and reapplies current-route + targeting. +- [ ] Retain or extend the dynamic prefix-ID test to prove a resolved runtime ID is + the handoff key. + +## Task 5: Mirror the runtime protocol in the head bootstrap + +**Files:** + +- Modify `crates/trusted-server-core/src/integrations/gpt_bootstrap.js` +- Modify `crates/trusted-server-core/src/integrations/gpt.rs` + +- [ ] Port the same actual-inner-div fallback, registry names, lifecycle flags, and + idempotence markers to the plain-JavaScript bootstrap. +- [ ] Use the existing bootstrap `window.tsjs` properties exactly so `index.ts` can + adopt the initial claim after the bundle loads. +- [ ] Ensure its internal definition/display/refresh calls use the same guards as the + bundle; bootstrap must not transfer or suppress its own operations. +- [ ] Extend the `gpt.rs` head-insert tests to assert that the bootstrap contains the + inner-div handoff protocol and no longer contains the container fallback. +- [ ] Add an executable bootstrap behavior test if practical by evaluating the + injected script against the same fake GPT fixture. If the test setup cannot execute + the included asset without duplication, record that limitation and keep the Rust + source-contract assertion plus identical bundle lifecycle tests as the minimum + coverage. + +## Task 6: Validate, inspect, and ship + +- [ ] Run focused request-level tests: + + ```bash + cd crates/trusted-server-js/lib + npx vitest run test/integrations/gpt/ad_init.test.ts test/integrations/gpt/index.test.ts + ``` + +- [ ] Run all TSJS tests and formatting: + + ```bash + cd crates/trusted-server-js/lib + npx vitest run + npm run format + ``` + +- [ ] Run the target-matched Rust tests that cover the embedded bootstrap, followed by + project formatting and linting: + + ```bash + cargo test-axum + cargo fmt --all -- --check + cargo clippy-fastly && cargo clippy-axum && cargo clippy-cloudflare + ``` + +- [ ] Before PR handoff, run the full required CI gates from `CLAUDE.md`, including + Fastly, Axum, Cloudflare, Spin, integration parity, JS build/tests/format, and docs + format. +- [ ] Review the diff specifically for bootstrap/bundle protocol drift and for any + use of container IDs in GPT slot creation. +- [ ] In a controlled production-like browser capture, verify one initial request for + each affected visible placement and independently verify an unrelated placement + remains requestable. +- [ ] Update issue #944 with the ownership-handoff decision, test evidence, and + browser-capture result. + +## Stop conditions + +Stop and return to design review instead of adding heuristics if any of these occur: + +- A publisher relies on a late `defineSlot` with materially different path or size + arguments and cannot accept the existing TS slot. +- The publisher's first initial-load-disabled refresh cannot be identified without + suppressing unrelated legitimate refreshes. +- A cross-bundle bootstrap handoff requires module-local identity that cannot be + represented safely through `window.tsjs`. +- Browser validation shows a second request despite native `defineSlot`/`display`/ + `refresh` suppression; capture the GPT event ordering before choosing another + strategy. diff --git a/docs/superpowers/specs/2026-07-24-prevent-duplicate-gpt-slot-requests-design.md b/docs/superpowers/specs/2026-07-24-prevent-duplicate-gpt-slot-requests-design.md new file mode 100644 index 000000000..770718199 --- /dev/null +++ b/docs/superpowers/specs/2026-07-24-prevent-duplicate-gpt-slot-requests-design.md @@ -0,0 +1,165 @@ +# Prevent Duplicate GPT Slot Requests — Design Specification + +## Problem + +When `tsjs.adInit()` executes before a publisher's framework later calls +`googletag.defineSlot()` for the same placement, TS currently defines and displays a +slot on the outer `-container` element. The publisher subsequently defines and +displays an inner-div slot. These are distinct GPT slots, so they make separate GAM +requests for one visible placement. + +The affected paths are deliberately duplicated today: + +- `crates/trusted-server-js/lib/src/integrations/gpt/index.ts` is the full bundle + implementation used after the TSJS bundle loads. +- `crates/trusted-server-core/src/integrations/gpt_bootstrap.js` is the head-injected + implementation that can make the initial request before the bundle loads. + +A fix must keep both implementations in sync. + +## Goals + +1. A configured placement has at most one initial GPT slot and ad request when TS + runs before a publisher defines its inner div. +2. Apply TS targeting and the `ts_initial=1` marker before that single initial + request. +3. Continue reusing a slot that the publisher has already defined. +4. Keep the TS-only fallback: if the publisher never defines the placement, TS still + displays it and makes exactly one initial request. +5. Preserve `disableInitialLoad()`, SPA targeting cleanup, and the rule that TS does + not destroy genuinely publisher-owned slots. +6. Keep dynamic div-ID prefix resolution intact. + +## Non-goals + +- Deduplicating by GAM ad-unit path. Multiple visible placements may validly share a + path. +- Changing publisher GAM configuration, line items, or refresh policy. +- Delaying the initial TS request while waiting an arbitrary amount of time for + framework hydration. A time-based grace period cannot distinguish a slow + publisher-owned slot from a placement that the publisher will never define. +- General interception of unrelated GPT slots. + +## Decision: one inner-div slot with late-definition handoff + +TS will define its fallback slot on the **actual inner div**, never on its outer +`-container` element. It will record a narrowly scoped handoff claim keyed by that +inner div ID. A `googletag.defineSlot` wrapper then recognizes a later publisher +request for that exact div and returns the existing TS slot rather than invoking +GPT's native `defineSlot` again. + +GPT requires a one-to-one slot-to-div relationship and documents that a slot should +be displayed only once. Sharing the initial inner-div slot therefore avoids both the +competing container slot and an invalid duplicate definition. + +### Lifecycle + +1. **Already publisher-owned** — `getSlots()` finds a slot for the resolved inner + div. TS applies targeting, records it as publisher-owned, and refreshes it as it + does today. +2. **No slot yet** — TS defines a slot on the resolved inner div, applies targeting, + enables services when needed, and displays it. When initial load is disabled, TS + performs its existing one explicit refresh. TS records this slot as TS-owned and + handoff-eligible. +3. **Publisher defines later** — the scoped `defineSlot` wrapper sees the recorded + inner-div claim, returns the existing slot, and transfers ownership: it removes + the slot from TS's future `destroySlots()` set. The publisher's setup continues + against that same slot. +4. **Publisher's first request call** — the wrapper suppresses the duplicate + publisher `display()` call. With `disableInitialLoad()`, it instead suppresses + only the publisher's first refresh for the transferred slot, because TS has + already issued the required initial refresh. For a no-argument/global refresh, + the wrapper must expand `getSlots()`, remove only the one-shot suppressed slots, + and forward the remaining slots explicitly so unrelated slots still refresh. +5. **Later refreshes and SPA navigation** — after the one-shot suppression is + consumed, publisher refreshes are untouched. On navigation, TS clears its + targeting from the shared slot and may reuse it for the next route; it must not + destroy a slot after ownership has transferred. + +The wrapper is not a global deduplicator. It only handles IDs present in TS's +handoff registry and must preserve native `defineSlot`, `display`, and `refresh` +behavior for every other placement. + +## Implementation shape + +### Shared runtime state + +Add a small, serializable `window.tsjs` registry that both initial implementations +can read after the bundle replaces the bootstrap implementation. It is keyed by the +resolved actual div ID and records at least: + +- whether TS created the slot and whether ownership has transferred; +- whether one publisher `display()` or initial-load-disabled `refresh()` remains to + suppress. + +Do not rely only on module-local state: the bootstrap can define the initial slot +before `index.ts` is loaded. Look up the live slot by element ID through +`pubads().getSlots()` when a wrapper needs it. + +Install idempotent markers on the wrapped GPT functions/services so the bootstrap and +bundle do not stack wrappers. Each wrapper must retain and call the original bound +function for non-claimed slots. Internal TS calls need a short-lived guard so the +wrappers do not mistake TS's own `defineSlot`, `display`, or `refresh` for a +publisher handoff. + +### Full bundle + +In `crates/trusted-server-js/lib/src/integrations/gpt/index.ts`: + +- Replace the container fallback with `actualDivId`. +- Add the typed handoff-registry state to `TsjsApi` in + `crates/trusted-server-js/lib/src/core/types.ts`. +- Install the idempotent `defineSlot`, `display`, and `pubads().refresh` handoff + wrappers from the GPT command queue before `adInit()` can create a fallback slot. +- When a late publisher definition is aliased to the existing slot, remove it from + `prevGptSlots` and mark it transferred before returning it. +- Keep targeting cleanup keyed by the real inner div. Remove the old dual + inner/container mappings because the slot element ID is now the inner div. + +### Head bootstrap + +Mirror the same ownership registry and wrappers in +`crates/trusted-server-core/src/integrations/gpt_bootstrap.js`. The bootstrap must +leave the registry and idempotence markers in `window.tsjs` so the full bundle adopts +rather than re-wraps or reclaims the initial slot. + +This duplication is intentional for now: the head bootstrap is needed to apply +server-side targeting before the normal bundle becomes available. The regression +suite must exercise both implementations' observable contract. + +## Compatibility rules and risks + +| Risk | Mitigation | +| -------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Publisher passes a different ad-unit path or sizes in its late `defineSlot` call | Return the existing claimed slot but log a diagnostic. Do not define a second slot. Treat the TS configuration and publisher configuration mismatch as an integration error to resolve separately. | +| Publisher invokes global `refresh()` after `disableInitialLoad()` | Filter the one-shot claimed slot from the expanded slot list and refresh all remaining slots. A no-argument refresh must not be silently dropped. | +| Publisher calls a legitimate refresh without an initial display | The one-shot suppression is consumed only immediately after a successful late handoff. Document and test the standard publisher sequence (`defineSlot` → `addService` → `display`, with `refresh` when initial load is disabled). Escalate unusual publisher lifecycle requirements rather than adding a time heuristic. | +| Publisher-owned slot is destroyed on SPA navigation | Transfer ownership synchronously in the `defineSlot` wrapper and remove the slot from `prevGptSlots`. | +| Bootstrap and bundle diverge | Give both paths the same black-box regression cases; retain a Rust source-contract assertion for bootstrap-specific sentinels. | +| A framework creates the inner element only after `adInit()` | TS still skips an absent element, as it does today; when the publisher owns that later-created slot it will not be duplicated. Supplying TS targeting to such a slot is a separate readiness problem, not part of this duplicate-request fix. | + +## Acceptance criteria + +- A late `defineSlot(innerDiv)` aliases the already-created inner-div TS slot; native + `defineSlot` is not called a second time for that placement. +- Request instrumentation records one initial request for the placement in normal and + initial-load-disabled modes. +- The late publisher `display()` (and its first initial-load-disabled refresh) cannot + create a second request, while unrelated slots retain their normal calls. +- Existing publisher slots are still reused and receive TS targeting. +- A slot that no publisher claims is displayed and requested once by TS. +- A transferred slot is absent from TS's SPA `destroySlots()` argument; targeting is + still cleared and reapplied correctly on the next route. +- Dynamic resolved div IDs work without constructing a CSS selector from the ID. +- Bootstrap and bundle paths pass the same ownership/request assertions. + +## Validation + +1. Add focused Vitest lifecycle tests with a fake GPT that records native + `defineSlot`, `display`, `refresh`, and synthetic request events. +2. Run the focused GPT test files, then the full TSJS Vitest suite and formatter. +3. Run the target-matched Rust test suite so the included bootstrap and its source + assertions compile and pass. +4. In a controlled browser capture, verify that one configured header and one + configured fixed placement each produce one initial slot request, while a distinct + in-content placement remains independently requestable.