-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Core: handling interventions #13818
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.
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
Core: handling interventions #13818
Conversation
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
creative/crossDomain.js
Outdated
PB_LOCATOR | ||
} from './constants.js'; | ||
|
||
const observer = new ReportingObserver((reports) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you put this in the renderer(s) instead? this is supposed to be a thin wrapper that just injects the renderer, because updating it is painful; everyone using it would need to update their creatives, and we'd need to replicate the same logic in PUC as well. Renderers on the other hand are just shipped together with prebid.
creative/crossDomain.js
Outdated
target.postMessage(JSON.stringify(Object.assign({message: type, adId}, payload)), pubDomain, [channel.port2]); | ||
} | ||
|
||
window.sendMessage = sendMessage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this need to be set on window
?
import {ERROR_NO_AD} from './constants.js'; | ||
|
||
export function render({ad, adUrl, width, height, instl}, {mkFrame}, win) { | ||
export function render({ad, adUrl, width, height, instl}, {mkFrame, sendMessage}, win) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can extract this into some shared file within creative/
. The 'browserIntervention' event can also be defined once instead of copied as a string, similar to how other events are imported in creative/constants.js
(the oddity of that file importing just to export is to avoid pulling more of prebid core into the creative).
|
||
export function render({ad, adUrl, width, height, instl}, {mkFrame}, win) { | ||
export function render({ad, adUrl, width, height, instl}, {mkFrame, sendMessage}, win) { | ||
if (window.ReportingObserver) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
window
here is not the window where the creative will be; however, from my testing using the "correct" win.ReportingObserver
doesn't work - so I guess this is OK.
This made me realize however that this likely won't work when the GAM creative is not set up to use safeframe (which is quite common). The logic there:
Lines 326 to 344 in 71139a1
function renderFn(adData) { | |
if (adData.ad) { | |
doc.write(adData.ad); | |
doc.close(); | |
emitAdRenderSucceeded({doc, bid, id: bid.adId}); | |
} else { | |
getCreativeRenderer(bid) | |
.then(render => render(adData, { | |
sendMessage: (type, data) => messageHandler(type, data, bid), | |
mkFrame: createIframe, | |
}, doc.defaultView)) | |
.then( | |
() => emitAdRenderSucceeded({doc, bid, id: bid.adId}), | |
(e) => { | |
fail(e?.reason || AD_RENDER_FAILED_REASON.EXCEPTION, e?.message) | |
e?.stack && logError(e); | |
} | |
); | |
} |
doesn't run this renderer at all when the bid provides ad
; and runs it using a different window setup otherwise. I'd test this by calling pbjs.renderAd
directly (e.g. following https://github.com/prebid/Prebid.js/blob/master/integrationExamples/noadserver/basic_noadserver.html) - we probably need to add the ReportingObserver
logic in core as well.
We're very happy to see the onIntervention bidder method included |
Pull Request Test Coverage Report for Build 17499038656Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk what I'm doing wrong but I can't get Chrome to intervene using the examples you added. Code LGTM.
No idea. I'm attaching the recording of how it works on my side. The only thing that comes to my mind is that Chrome won’t apply the intervention to the same ad again after refreshing the page if it has been done before. So you could try using incognito mode Screen.Recording.2025-09-23.at.15.59.43.mov |
Type of change
Bugfix
Feature
New bidder adapter
Updated bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other
Description of change
Other information
#13560