-
Notifications
You must be signed in to change notification settings - Fork 76
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
adServerDomain
setup in creatives appears to be useless
#181
Comments
I think it used to be used on the Prebid.js side of things to help handle the postMessage request. But it seems that a while ago the logic was changed on the Prebid.js side through this PR: It's likely it was just not cleaned up on the PUC side, and now (as you inferred) doesn't do much and can probably be removed. |
Thanks for the detective work! That PR was merged in 3.22; is that old enough to be OK to break with a PUC update? |
I'm not sure what you mean by break in this instance? Do you mean people that would still be using latest PUC (as that's how the creatives are setup) while also using a version of 3.x Prebid.js; as that would stop supplying the adserverDomain from updated PUC to that old version of Prebid.js? |
Yes - with how we distribute PUC, we are implicitly promising unlimited backwards compatibility. |
I think we need to review this with the larger group. |
Agreed. Brainstorming ideas on how to escape this trap, we could try to tie breaking changes to Prebid major versions; the transition will take forever, but we could for example ask pubs to set up
and start a new PUC branch (and release tag) for each new Prebid major version. |
In at least some of our docs we ask publishers to set up PUC-GAM creatives with
adServerDomain
:PUC includes it as part of the message payload for Prebid, but the latter does not use it:
prebid-universal-creative/src/renderingManager.js
Lines 144 to 151 in 68d81fc
From my testing, including or removing it makes no difference. I believe it should be removed together with the code around it, and the docs updated.
The text was updated successfully, but these errors were encountered: