Skip to content
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

Concert Bid Adapter: adding referrer #8564

Closed
wants to merge 0 commits into from

Conversation

andrew-a-dev
Copy link
Contributor

Type of change

  • Other

Specifically: adding functionality to the existing Concert adapter

Description of change

Adding a ref parameter to the bid calls, using the various "referrer" properties available in Prebid.

(The PR also contains linting-related changes to the file as a whole, to do with ; and whitespace.)

Copy link
Collaborator

@dgirardi dgirardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes LGTM - but I am curious about it if you want to indulge me.

site: bidRequest.params.site || bidderRequest.refererInfo.page
}
site: bidRequest.params.site || bidderRequest.refererInfo.page,
ref: bidderRequest.refererInfo.ref || window.document.referrer
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious about the fallback (because this choice is common enough that I wonder if it should be built into refererInfo). It would be used when prebid is running inside an isolated iframe, and its value would usually be the location of the parent frame - why is that preferable to null? Not knowing how this is used in the backend it looks like pollution of the data to me. (sometimes ref is the referrer, sometimes not, and you don't know which is which).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgirardi You're right, that wasn't properly thought through; we don't want the iframe's referrer. I see the pull request has been closed because of lack of CI, so I'll open another one with an update to the line above. Thanks for calling that out!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no, I just realised I inadvertently closed this PR by messing around with master on our fork. 🤦 Sorry!

@ChrisHuie ChrisHuie changed the title Concert Bid Adapter update: adding referrer Concert Bid Adapter: adding referrer Jun 15, 2022
@patmmccann
Copy link
Collaborator

circleci did not run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants