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

pbjs.getAllWinningBids did not count Secure Creatives #1187

Closed
canyousayyes opened this issue May 10, 2017 · 3 comments
Closed

pbjs.getAllWinningBids did not count Secure Creatives #1187

canyousayyes opened this issue May 10, 2017 · 3 comments

Comments

@canyousayyes
Copy link
Contributor

Type of issue

Bug

Description

When the ads are served by X-Domain, the pbjs.getAllWinningBids() didn't include those bid objects.

While in usual pbjs.renderAd, they are included.

Steps to reproduce

  1. Config the line items to serve ads in X-Domain.

  2. Load the page and try to make a bid to win.
    (This may be tricky. In my local environment, I usually update the bidCpmAdjustment to set a very high CPM value, so that the bidder can win)

  3. After the ad content is fetched, check the result of pbjs.getAllWinningBids() in browser console.

Expected results

The bid object is shown in pbjs.getAllWinningBids()
The event BID_WON is emitted.

Actual results

The bid object did not shown in pbjs.getAllWinningBids().
The event BID_WON is emitted, though.

Platform details

Prebid.js v0.23.0
MacOS 10.12.4
Chrome 58.0.3029.96 (64-bit)
node v6.9.2
npm 4.2.0

Other information

My suspect is that pbjs.renderAd handled the _winningBids
https://github.com/prebid/Prebid.js/blob/master/src/prebid.js#L304

But the secure creatives is not.
https://github.com/prebid/Prebid.js/blob/master/src/secure-creatives.js

I am not very experienced in Prebid.js, but if you can confirm that this is the cause, I can try to make a pull request to fix it.

@mkendall07
Copy link
Member

Yeah, I believe this is a bug. Thanks for the report, feel free to submit a PR!

@canyousayyes
Copy link
Contributor Author

Thanks, I have submitted the PR in #1192 :)

@ptomasroos
Copy link
Contributor

This is now fixed, Should be closed

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

No branches or pull requests

3 participants