-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Support multiple media formats within a single ad unit #1991
Conversation
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.
Awesome work! LGTM
needs a linked docs PR before merge |
@matthewlane |
2d64a45
to
e6520b3
Compare
Added |
6247fe7
to
2808763
Compare
src/constants.json
Outdated
@@ -52,7 +52,8 @@ | |||
"hb_pb", | |||
"hb_size", | |||
"hb_deal", | |||
"hb_source" | |||
"hb_source", | |||
"hb_mediatype" |
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.
@matthewlane can you change this to hb_format
instead?
it's a little bit shorter
@@ -31,7 +30,7 @@ const SOURCE = 'pbjs'; | |||
export const spec = { | |||
code: BIDDER_CODE, | |||
aliases: ['appnexusAst', 'brealtime', 'pagescience', 'defymedia', 'gourmetads', 'matomy', 'featureforward', 'oftmedia', 'districtm'], | |||
supportedMediaTypes: [VIDEO, NATIVE], | |||
supportedMediaTypes: [BANNER, VIDEO, NATIVE], |
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.
@matthewlane If i am not wrong all other adapters supporting video or native also needs to be updated. As of now https://github.com/prebid/Prebid.js/blob/master/modules/rubiconBidAdapter.js#L76 Rubicon will not work for banner
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.
Thanks, looking in to this
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.
Good callout @jaiminpanchal27. I went through all adapters that have supportedMediaTypes
and added 'banner' to those that do support it in addition to 'native' or 'video'. Some adapters only support 'native' or 'video', determined by looking at code and corresponding .md files, and in those cases supportedMediaTypes
are left unchanged
…e of the mediaTypes
…plicitly supporting banner
5409303
to
fb045f8
Compare
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.
LGTM
* 'master' of https://github.com/prebid/Prebid.js: Prebid 1.2.0 Release Use polyfilled includes method (prebid#2061) RockYou Adapter: Added RockYou Adapter supporting Prebid 1.0 (prebid#1977) Optimera Adapter for 1.0. (prebid#1961) Use cross-browser integer check (prebid#2058) Fix skipped test (prebid#2059) Support multiple media formats within a single ad unit (prebid#1991) pre1api module that allows use of deprecated pre1.0 API in Prebid 1.0 (prebid#1976) Colossus SSP header bidding adapter 1.0.0 (prebid#2029) InSkin Bidder Adapter (prebid#2016) Update adapter to prebid v1.0 (prebid#1908) PubMatic 1.0 adapter (prebid#2011)
* Populate ad_types array on all requests * Remove descriptionUrl * Allow a bidder to participate on an adunit if it supports at least one of the mediaTypes * Add hb_mediatype to standard targeting * Determine when to trigger callback based on bid response * Change key to hb_format * Print banner as default when mediaTypes not defined * Update spportedMediaTypes to include banner for adapters that were implicitly supporting banner
Type of change
Description of change
This change supports declaring multiple media formats on a single ad unit, and allowing any bidder that supports at least one of those media formats to participate in that ad unit.
Previously, only one media format per ad type was supported, and only bidders that were compatible with that media format were eligible to participate.
Now, an ad unit can declare multiple media types, and any bidder that supports at least one of those types may take part in that auction. Any bidder that isn't compatible with the specified
mediaTypes
will be dropped from the ad unit. IfmediaTypes
is not specified on an ad unit,banner
is the assumed format and any banner bidder is eligible for inclusion.This PR also adds
hb_format
to standard targeting and sets its value based on the bid response object'smediaType
property.Examples: