Let the script-free ad frame ask the browser what colour it should be - #221
Merged
Conversation
…ld be `/api/ads/frame` is the embed for publishers who will not run our JavaScript — a strict CSP, a Tor hidden service, a site that simply does not want a third party's script. Not running a script also means it cannot do the one thing `ad.js` does before it fills anything: read the publisher's background off the DOM and pick a polarity from it. So the frame rendered whatever the slot said, and a slot that says nothing renders dark. A light publisher got a black bar punched into their page, which is the exact failure theme variants were added to prevent. Nothing was broken; the mechanism just had no input on this path. It has one input it never used: the reader's own browser. `prefers-color-scheme` inside the frame is answerable without seeing the parent page at all, because the frame is a real document. This makes 'auto' a rendering mode rather than a synonym for the default — the creative ships both palettes and the media query picks — and makes it the frame's default. Doing that means no theme-dependent colour can be interpolated into the markup any more, or half the document would keep the base palette when the query fires. Every one of them now goes through a CSS custom property declared by `themeStyle`, which emits one `:root` block for a chosen theme and two for auto. That covers the derived values as well as the palette: the hairline, the punch-out ink for the CTA chip, the over-image ink, the text shadow, the scrim and the brand wash all move together or the unit comes apart at the seam. Light is the base and dark rides the media query, for the same reason `ad.js` composites over white: a page with no styling of its own is white whatever the viewer's OS prefers. An explicit light or dark still wins from either side, since both mean somebody looked — the tag measured the page, or a publisher set the slot. Surfaces that cannot honour a media query at all, a MOTD over curl and a feed body in somebody's reader, keep going through `resolveTheme` and collapse to dark. The preview harness grows an auto row, which is how the pair above was checked.
ThreatCrush Security Scan40 finding(s) HIGH/CRITICAL: 3 | MEDIUM: 28 | LOW: 9
Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the light-theme case for
/api/ads/frame, the script-free embed.The problem
/ad.jswalks up the DOM, finds the first painted background, and passes a concretethemetoserveAd./api/ads/framecannot do any of that — not running a script on the publisher's page is the entire point of it. So it rendered whatever the slot said, and a slot that says nothing renders dark.A light publisher got a black bar punched into their page: the exact failure theme variants exist to prevent. Hit on d3vices.com, which embeds the frame precisely because its CSP allows one inline script by hash and
ad.jswould have needed'unsafe-inline'instyle-srcsite-wide.The fix
The frame has one input it never used: the reader's own browser.
prefers-color-schemeis answerable inside the frame without seeing the parent page, because the frame is a real document.So
'auto'becomes a real rendering mode rather than a synonym for the default — the creative ships both palettes in one document and the media query picks — and it becomes the frame's default. One impression, no script, no client hint, no round trip.What that required
No theme-dependent colour can be interpolated into the markup any more, or half the document would keep the base palette when the query fires. They all now go through a CSS custom property declared by
themeStyle, which emits one:rootblock for a chosen theme and two for auto.That covers the derived values, not just the palette — the hairline, the punch-out ink for the CTA chip, the over-image ink, the text shadow, the scrim and the brand wash move together or the unit comes apart at the seam. A test asserts no raw
#rrggbborrgba()survives anywhere after</style>, across every HTML format, with and without a hero image.Light is the base, dark rides the media query — the same reasoning
ad.jsuses when it composites over white: a page with no styling of its own is white whatever the viewer's OS prefers.Precedence
light/darklight/darkautolight/darkauto/ absentauto/ absentautodark, unchangedresolveThemekeeps its old signature and contract for surfaces that cannot honour a media query — a MOTD over curl, a feed body in somebody's reader — and collapsesautoto dark there.resolveThemePrefis the new one./ad.jsis untouched: it still detects and sends an explicit theme, which still wins.Verified
npm run typecheckclean; full suite 1960 passed, 0 failed (11 new tests intests/ads-auto-theme.test.ts).prefers-color-schemevalues. The auto units computergb(246,247,249)under light andrgb(11,13,16)under dark; the pinned light and dark sections are byte-identical in both.scripts/preview-themes.tsgrows an auto row, which is how that was checked.After deploy
d3vices.com needs no change — it already embeds
/api/ads/frame, so it picks this up on deploy.