Skip to content

Select active GPT responsive slots #978

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ChristianPavilonis wants to merge 2 commits into
base: fix/duplicate-gpt-slots
Choose a base branch
from
Open

Select active GPT responsive slots #978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
340d1ef
Select active GPT responsive slots
ChristianPavilonis Jul 29, 2026
ca678fe
Resolve visible responsive GPT slots
ChristianPavilonis Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/trusted-server-core/src/integrations/gpt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1213,7 +1213,7 @@ mod tests {
"bootstrap should scan ID-bearing elements instead of interpolating div_id into CSS"
);
assert!(
combined.contains(".startsWith(slot.div_id)"),
combined.contains("candidate.id.startsWith(divId)"),
Comment thread
ChristianPavilonis marked this conversation as resolved.
"bootstrap should match metacharacter-containing div_id prefixes with startsWith"
);
assert!(
Expand Down
82 changes: 64 additions & 18 deletions crates/trusted-server-core/src/integrations/gpt_bootstrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,66 @@
return target && target.id ? target.id : null;
}

function isElementVisible(element) {
var style = window.getComputedStyle(element);
return (
style.display !== "none" &&
style.visibility !== "hidden" &&
style.visibility !== "collapse"
);
}

function slotElementHasLayout(element) {
if (!isElementVisible(element)) return false;
var elementRect = element.getBoundingClientRect();
if (elementRect.width > 0 && elementRect.height > 0) return true;

var container = document.getElementById(element.id + "-container");
if (!container || !isElementVisible(container)) return false;
var containerRect = container.getBoundingClientRect();
return containerRect.width > 0 && containerRect.height > 0;
}

function findSlotElementByDivId(divId) {
if (!divId) return null;
var exact = document.getElementById(divId);
if (exact) return exact;

var idElements = document.querySelectorAll("[id]");
var prefixMatches = [];
for (var i = 0; i < idElements.length; i++) {
var candidate = idElements[i];
if (
candidate.id.startsWith(divId) &&
!candidate.id.endsWith("-container")
) {
prefixMatches.push(candidate);
}
}
// A unique prefix match may be a lazy slot that has not been sized yet.
// Geometry is only needed to disambiguate multiple responsive siblings.
if (prefixMatches.length === 1) return prefixMatches[0];

var visibleMatches = prefixMatches.filter(isElementVisible);
if (visibleMatches.length === 1) return visibleMatches[0];

var activeMatches = visibleMatches.filter(slotElementHasLayout);
if (activeMatches.length === 1) return activeMatches[0];

if (
prefixMatches.length > 1 &&
ts.log &&
typeof ts.log.warn === "function"
) {
ts.log.warn("GPT slot prefix did not resolve to one active element", {
Comment thread
ChristianPavilonis marked this conversation as resolved.
divId: divId,
prefixMatchCount: prefixMatches.length,
activeMatchCount: activeMatches.length,
});
}
return null;
}

function runHandoffInternal(callback) {
var wasInternal = ts.gptSlotHandoffInternal;
ts.gptSlotHandoffInternal = true;
Expand Down Expand Up @@ -217,24 +277,10 @@
// slot that was never displayed, so these are display()ed instead.
var slotsToDisplay = [];
slots.forEach(function (slot) {
// Resolve actual div ID: exact match first, then safe prefix scan.
// div_id in config may be a stable prefix (e.g. "ad-header-0-") when
// the suffix is dynamically generated by the framework at render time.
var el = document.getElementById(slot.div_id);
if (!el) {
var idElements = document.querySelectorAll("[id]");
for (var i = 0; i < idElements.length; i++) {
var candidate = idElements[i];
if (
slot.div_id &&
candidate.id.startsWith(slot.div_id) &&
!candidate.id.endsWith("-container")
) {
el = candidate;
break;
}
}
}
// Resolve actual div ID: exact match first, then the one active prefix
// match. Responsive publishers may emit several mutually exclusive
// siblings for one stable prefix, so document order is not sufficient.
var el = findSlotElementByDivId(slot.div_id);
if (!el) return;
var actualDivId = el.id;
var b = bids[slot.id] || {};
Expand Down
55 changes: 50 additions & 5 deletions crates/trusted-server-js/lib/src/integrations/gpt/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,50 @@ interface SlotRenderEndedEvent {
slot: GoogleTagSlot;
}

function isElementVisible(element: HTMLElement): boolean {
const style = window.getComputedStyle(element);
return (
style.display !== 'none' && style.visibility !== 'hidden' && style.visibility !== 'collapse'
);
}

function slotElementHasLayout(element: HTMLElement): boolean {
if (!isElementVisible(element)) return false;
const elementRect = element.getBoundingClientRect();
if (elementRect.width > 0 && elementRect.height > 0) return true;

const container = document.getElementById(`${element.id}-container`);
if (!container || !isElementVisible(container)) return false;
const containerRect = container.getBoundingClientRect();
return containerRect.width > 0 && containerRect.height > 0;

@aram356 aram356 Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

❓ question — Fail-closed drops the impression entirely when no candidate has reserved height.

slotElementHasLayout requires width > 0 && height > 0 on the element or its -container. An empty ad div has height 0 until an ad renders, unless the publisher reserves space with min-height. In the container-based responsive-hiding pattern this PR targets (inactive containers display:none), every inner element still passes the element-visibility check (computed display does not inherit from ancestors), so resolution always falls through to geometry — and with no reserved height, activeMatches is empty and the slot is skipped entirely, where the old code served the first match.

Is total no-fill acceptable for publishers that don't reserve slot heights? A width-only container check would disambiguate without requiring reserved height, since a display:none container has a 0×0 rect while the active one keeps its block width:

const containerRect = container.getBoundingClientRect();
return containerRect.width > 0;

Same applies to the mirrored slotElementHasLayout in crates/trusted-server-core/src/integrations/gpt_bootstrap.js.

}

function findSlotElementByDivId(divId: string): HTMLElement | null {
if (!divId) return null;
Comment thread
ChristianPavilonis marked this conversation as resolved.
const exact = document.getElementById(divId);
if (exact) return exact;

return (
Array.from(document.querySelectorAll<HTMLElement>('[id]')).find(
(el) => el.id.startsWith(divId) && !el.id.endsWith('-container')
) ?? null
const prefixMatches = Array.from(document.querySelectorAll<HTMLElement>('[id]')).filter(
(element) => element.id.startsWith(divId) && !element.id.endsWith('-container')
);
// A unique prefix match may be a lazy slot that has not been sized yet.
// Geometry is only needed to disambiguate multiple responsive siblings.
if (prefixMatches.length === 1) return prefixMatches[0] ?? null;

const visibleMatches = prefixMatches.filter(isElementVisible);
if (visibleMatches.length === 1) return visibleMatches[0] ?? null;

@aram356 aram356 Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤔 thinking — Element-level visibility can't see container-hidden siblings.

isElementVisible reads the element's own computed style, and display doesn't inherit — an element inside a display:none container still counts as visible. When no sibling is actually active and exactly one is hidden only at container level, this visibleMatches.length === 1 shortcut returns that hidden element instead of failing closed, defining a GPT slot in an invisible subtree (unviewable impression).

element.checkVisibility?.() checks ancestor display for exactly this case and could be layered in with the current heuristic as fallback:

function isElementVisible(element: HTMLElement): boolean {
  if (typeof element.checkVisibility === 'function') return element.checkVisibility();
  const style = window.getComputedStyle(element);
  // existing checks…
}


const activeMatches = visibleMatches.filter(slotElementHasLayout);
if (activeMatches.length === 1) return activeMatches[0] ?? null;

if (prefixMatches.length > 1) {
log.warn('GPT slot prefix did not resolve to one active element', {

@aram356 aram356 Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

♻️ refactor — Ambiguity warning fires on every animation frame during the SPA slot wait.

waitForSlotElements' allPresent() calls findSlotElementByDivId per slot per mutation-batched frame for up to 2 s. An ambiguous prefix emits this log.warn on every call — potentially ~120 duplicate console warns per navigation. Move the warn to the adInit resolution site, or add a warn-once guard keyed on divId.

divId,
prefixMatchCount: prefixMatches.length,
activeMatchCount: activeMatches.length,
});
}
return null;
}

function candidateSlotRoots(divId: string): HTMLElement[] {

@aram356 aram356 Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤔 thinking — Render bridge re-resolves the prefix at message time and can disagree with adInit.

candidateSlotRoots re-runs the geometry-sensitive resolver when the Prebid Request message arrives. If layout shifted since adInit (hydration, rotation), or the prefix has become ambiguous by then, resolution returns a different element or null, slotIdForMessageSource misses the iframe, and the TS creative and win/billing beacons are silently dropped.

adInit already records the resolved ID in ts.divToSlotId — deriving candidate roots from that map would keep both sites consistent with the element adInit actually chose.

Expand Down Expand Up @@ -869,16 +904,26 @@ function waitForSlotElements(slots: AuctionSlot[], signal: AbortSignal): Promise

return new Promise<void>((resolve) => {
let settled = false;
let animationFrame: number | undefined;
const finish = (): void => {
if (settled) return;
settled = true;
if (animationFrame !== undefined) cancelAnimationFrame(animationFrame);
observer.disconnect();
clearTimeout(timer);
signal.removeEventListener('abort', finish);
resolve();
};
const observer = new MutationObserver(() => {
if (allPresent()) finish();
if (animationFrame !== undefined) return;
if (typeof requestAnimationFrame === 'undefined') {
if (allPresent()) finish();
return;
}
animationFrame = requestAnimationFrame(() => {

@aram356 aram356 Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤔 thinking — rAF gating always costs the full 2 s in background tabs.

requestAnimationFrame doesn't fire in hidden tabs, so a background-tab SPA navigation never runs the observer check and applies bids only at the SPA_SLOT_WAIT_MS timeout. Minor, but a document.visibilityState === 'hidden' branch that checks allPresent() directly would restore prompt application.

animationFrame = undefined;
if (allPresent()) finish();
});
});
observer.observe(document.documentElement, { childList: true, subtree: true });
const timer = setTimeout(finish, SPA_SLOT_WAIT_MS);
Expand Down
163 changes: 163 additions & 0 deletions crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,44 @@ type TestWindow = Window & {
tsjs?: any;
};

function appendResponsiveSlotElement(

@aram356 aram356 Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⛏ nitpick — appendResponsiveSlotElement takes five positional booleans.

Call sites like (id, true, false, true, false) are hard to read; an options object ({ containerHasLayout, elementHidden, … }) would make the parameterized cases self-describing. Test-only.

id: string,
containerHasLayout: boolean,
elementHidden = false,
elementHasLayout = false,
containerVisible = containerHasLayout
): HTMLDivElement {
const container = document.createElement('div');
container.id = `${id}-container`;
container.dataset.responsiveSlotTest = 'true';
container.style.display = containerVisible ? 'block' : 'none';
container.getBoundingClientRect = () =>
({
width: containerHasLayout ? 320 : 0,
height: containerHasLayout ? 100 : 0,
}) as DOMRect;

const element = document.createElement('div');
element.id = id;
element.style.display = elementHidden ? 'none' : 'block';
element.getBoundingClientRect = () =>
({
width: elementHasLayout ? 300 : 0,
height: elementHasLayout ? 250 : 0,
}) as DOMRect;
container.appendChild(element);
document.body.appendChild(container);
return element;
}

function runGptBootstrap(): void {

@aram356 aram356 Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🏕 camp site — Reuse this helper in the older bootstrap test.

The pre-existing handoff test ('runs the embedded bootstrap handoff for a hydrated publisher ID', ~line 516) still inlines the identical readFileSync + window.eval that runGptBootstrap() wraps.

const bootstrap = readFileSync(
resolve(process.cwd(), '../../trusted-server-core/src/integrations/gpt_bootstrap.js'),
'utf8'
);
window.eval(bootstrap);
}

describe('installTsAdInit', () => {
beforeEach(() => {
vi.resetModules();
Expand All @@ -79,6 +117,7 @@ describe('installTsAdInit', () => {
document.getElementById('div-atf-sidebar')?.remove();
document.getElementById('ad-header-0-_r_1_')?.remove();
document.getElementById("ad'prefix-real")?.remove();
document.querySelectorAll('[data-responsive-slot-test]').forEach((element) => element.remove());
});

it('reads window.tsjs.bids synchronously and applies bid targeting before refresh', async () => {
Expand Down Expand Up @@ -1257,6 +1296,130 @@ describe('installTsAdInit', () => {
expect(mockPubads.refresh).toHaveBeenCalled();
});

it.each([
Comment thread
ChristianPavilonis marked this conversation as resolved.
{ implementation: 'runtime', activeIndexes: [2], publisherOwned: true, selectedIndex: 2 },
{ implementation: 'runtime', activeIndexes: [], selectedIndex: null },
{ implementation: 'runtime', activeIndexes: [], elementLayoutIndexes: [1], selectedIndex: 1 },
{ implementation: 'runtime', activeIndexes: [0, 2], selectedIndex: null },
{
implementation: 'runtime',
activeIndexes: [2, 3],
hiddenElementIndexes: [2],
selectedIndex: 3,
},
{
implementation: 'runtime',
activeIndexes: [],
hiddenElementIndexes: [0, 1, 3],
visibleContainerIndexes: [2],
selectedIndex: 2,
},
{ implementation: 'runtime', activeIndexes: [2], divId: '', selectedIndex: null },
{ implementation: 'bootstrap', activeIndexes: [2], publisherOwned: true, selectedIndex: 2 },
{ implementation: 'bootstrap', activeIndexes: [], selectedIndex: null },
{ implementation: 'bootstrap', activeIndexes: [], elementLayoutIndexes: [1], selectedIndex: 1 },
{ implementation: 'bootstrap', activeIndexes: [0, 2], selectedIndex: null },
{
implementation: 'bootstrap',
activeIndexes: [2, 3],
hiddenElementIndexes: [2],
selectedIndex: 3,
},
{
implementation: 'bootstrap',
activeIndexes: [],
hiddenElementIndexes: [0, 1, 3],
visibleContainerIndexes: [2],
selectedIndex: 2,
},
{ implementation: 'bootstrap', activeIndexes: [2], divId: '', selectedIndex: null },
] as const)(
'$implementation resolves responsive matches $activeIndexes to $selectedIndex',
async (testCase) => {
const { implementation, activeIndexes, selectedIndex } = testCase;
const hiddenElementIndexes =
'hiddenElementIndexes' in testCase ? testCase.hiddenElementIndexes : [];
const elementLayoutIndexes =
'elementLayoutIndexes' in testCase ? testCase.elementLayoutIndexes : [];
const visibleContainerIndexes =
'visibleContainerIndexes' in testCase ? testCase.visibleContainerIndexes : activeIndexes;
const divId = 'divId' in testCase ? testCase.divId : 'ad-responsive-';
const publisherOwned = 'publisherOwned' in testCase && testCase.publisherOwned;
const elements = ['a', 'b', 'c', 'd'].map((suffix, index) =>
appendResponsiveSlotElement(
`ad-responsive-${suffix}`,
(activeIndexes as readonly number[]).includes(index),
(hiddenElementIndexes as readonly number[]).includes(index),
(elementLayoutIndexes as readonly number[]).includes(index),
(visibleContainerIndexes as readonly number[]).includes(index)
)
);
const selectedElement = selectedIndex === null ? undefined : elements[selectedIndex];
const mockSlot = {
addService: vi.fn().mockReturnThis(),
setTargeting: vi.fn().mockReturnThis(),
getSlotElementId: vi.fn().mockReturnValue(selectedElement?.id ?? elements[0]!.id),
getTargeting: vi.fn().mockReturnValue([]),
};
const nativeRefresh = vi.fn();
const mockPubads = {
enableSingleRequest: vi.fn(),
getSlots: vi.fn().mockReturnValue(publisherOwned ? [mockSlot] : []),
addEventListener: vi.fn(),
refresh: nativeRefresh,
};
const defineSlot = vi.fn().mockReturnValue(mockSlot);
const nativeDisplay = vi.fn();
(window as TestWindow).googletag = {
cmd: { push: vi.fn((fn: () => void) => fn()) },
defineSlot,
display: nativeDisplay,
pubads: vi.fn().mockReturnValue(mockPubads),
enableServices: vi.fn(),
};
(window as TestWindow).tsjs = {
adSlots: [
{
id: 'responsive_slot',
gam_unit_path: '/123/responsive',
div_id: divId,
formats: [[300, 250]],
targeting: {},
},
],
bids: {},
};

if (implementation === 'runtime') {
const { installTsAdInit } = await import('../../../src/integrations/gpt/index');
installTsAdInit();
} else {
runGptBootstrap();
}
(window as TestWindow).tsjs!.adInit!();

if (selectedElement) {
if (publisherOwned) {
expect(defineSlot).not.toHaveBeenCalled();
expect(nativeRefresh).toHaveBeenCalledWith([mockSlot]);
} else {
expect(defineSlot).toHaveBeenCalledWith(
'/123/responsive',
[[300, 250]],
selectedElement.id
);
expect(nativeDisplay).toHaveBeenCalledWith(selectedElement.id);
}
expect((window as TestWindow).tsjs!.divToSlotId).toEqual({
[selectedElement.id]: 'responsive_slot',
});
} else {
expect(defineSlot).not.toHaveBeenCalled();
expect((window as TestWindow).tsjs!.divToSlotId).toEqual({});
}
}
);

it('resolves dynamic div prefixes without interpolating div_id into a CSS selector', async () => {
const dynamicDiv = document.createElement('div');
dynamicDiv.id = "ad'prefix-real";
Expand Down
Loading
Loading