-
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
Topics Fpd Module: Add RtbHouse topics network #10107
Conversation
Addition of RTB House's Topics API iframe support
modules/topicsFpdModule.js
Outdated
@@ -19,6 +19,9 @@ const bidderIframeList = { | |||
bidders: [{ | |||
bidder: 'pubmatic', | |||
iframeURL: 'https://ads.pubmatic.com/AdServer/js/topics/topics_frame.html' | |||
}, { | |||
bidder: 'rtbhouse', | |||
iframeURL: 'https://topics.creativecdn.com/tapi.html' |
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.
this frame doesnt post to top.
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.
take a look at view-source:https://ads.pubmatic.com/AdServer/js/topics/topics_frame.html
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.
Appreciate the early feedback @patmmccann !
Apologies for too-early post - did get roll back to WIP, but you were faster :-)
Will update in the next few days
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.
If you study the latest pr from @jlquaccia , it might make sense to branch off that and avoid the iframe altogether
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 for the pointer!
I could be wrong here, but iframe could make sense as the call would be appropriated to the iframe's domain, rather than to the publisher's domain like in 1-party-context javascript call on pub domain
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.
Should be good to go
re-assigning to @jlquaccia who is much more familiar with navigating these particular test failures. Thanks! |
@@ -15,10 +15,13 @@ let LOAD_TOPICS_INITIALISE = false; | |||
const HAS_DEVICE_ACCESS = hasDeviceAccess(); | |||
|
|||
const bidderIframeList = { | |||
maxTopicCaller: 1, | |||
maxTopicCaller: 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.
@jlquaccia hopefully this change of default is fine with you
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.
@piwanczak know this has already been merged, but yea seems alright to me. Thanks for the heads up! I took some time off last week, so was just getting around to seeing this PR today.
* Update topicsFpdModule.md Addition of RTB House's Topics API iframe support * chore: make linter happy - fixing typo * m: post-review fixes * chore: changed domains * chore: changed domains * make tests happy --------- Co-authored-by: Przemyslaw Iwanczak <przemyslaw.iwanczak@rtbhouse.com>
* Update topicsFpdModule.md Addition of RTB House's Topics API iframe support * chore: make linter happy - fixing typo * m: post-review fixes * chore: changed domains * chore: changed domains * make tests happy --------- Co-authored-by: Przemyslaw Iwanczak <przemyslaw.iwanczak@rtbhouse.com>
Addition of RTB House's Topics API iframe support
Type of change
Bugfix
Feature
New 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