PBjs Core: bugfix - accessing array instead of adUnit #10567
Conversation
ChrisHuie
changed the title
src/prebid.js
Outdated
| @@ -525,11 +525,11 @@ pbjsInstance.renderAd = hook('async', function (doc, id, options) { | |||
|
|
|||
| // video module | |||
| if (FEATURES.VIDEO) { | |||
There was a problem hiding this comment.
Could this short-circuit if the bid is not a video bid (bid.mediaType === 'video')? to avoid looking up the adUnit - since in 99% of cases this will be used for display and the lookup takes some CPU time.
Also - could you add some tests - verifying that videoModule.renderBid in invoked when and how you expect it to.
There was a problem hiding this comment.
@dgirardi addressed!
karimMourra
force-pushed
the
bugfix/gro-414
branch
from
7b5b8fa to
86081d9
Compare
| if (FEATURES.VIDEO && bid.mediaType === 'video') { | ||
| // TODO: could the video module implement this as a custom renderer, rather than a special case in here? | ||
| const adUnit = bid && auctionManager.index.getAdUnit(bid); | ||
| const videoModule = getGlobal().videoModule; | ||
| if (adUnit?.video && videoModule) { | ||
| videoModule.renderBid(adUnit.video.divId, bid); | ||
| const divId = adUnit && adUnit.video && adUnit.video.divId; | ||
| if (divId && videoModule) { | ||
| videoModule.renderBid(divId, bid); |
There was a problem hiding this comment.
@dgirardi i ported the changes over to direct.js ; open to creating a custom renderer but not sure what you mean by that . Is it an existing pattern ?
|
@karimMourra assume you see these test errors?
|
|
fixed in #10819 |
Type of change
Bugfix
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other
Description of change
pbjsInstance.adUnits.filterreturns an array yet the code was expecting an actual ad unit. This fix usesauctionManager.index.getAdUnitto get the ad unit and adds a check for thedivIdbefore attempting to render the ad.Integration example has been updated.
Other information
fixes issue #10505