-
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
Added adapter for Conversant Media #708
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.
Please update the createBid
calls to include the bid object as second parameter.
responseNurl = conversantBid.nurl || ''; | ||
|
||
// Our bid! | ||
bid = bidfactory.createBid(1); |
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.
pass the bid object as 2nd argument to sync bid request and response IDs. See #509
} else { | ||
//0 price bid | ||
//indicate that there is no bid for this placement | ||
bid = bidfactory.createBid(2); |
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.
pass the bid object as 2nd argument to sync bid request and response IDs. See #509
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.
Looks good, thanks for updates and tests, functionality is working.
@msahagu2 |
|
||
'site': { | ||
'id': siteId, | ||
'mobile': document.querySelector('meta[name="viewport"][content*="width=device-width"]') !== null ? 1 : 0, |
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.
is the presence of this meta tag a reliable way to detect mobile clients? Our responsive site at washingtonian.com includes this meta tag for everyone.
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.
The mobile field is there to determine if the site's layout can be optimized when on mobile clients. Not necessarily that they are mobile
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.
I see, thank you @msahagu2! 👍
n = navigator; | ||
|
||
// production endpoint | ||
var conversantUrl = '//media.msg.dotomi.com/s2s/header?callback=$$PREBID_GLOBAL$$.conversantResponse'; |
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.
@msahagu2 can you request an Access-Control-Allow-Origin: *
header for this endpoint?
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.
Do you know what ad sizes are requested? We're currently working on a fix for the headers of our 204 responses
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.
I am told by T. Rex at Conversant that the site_id numeric value must be passed as a string (buh?) so can you catch this case in the adapter @msahagu2
Type of change
Description of change
Added adapter for Conversant Media