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

ShowHeroes adapter - expanded outstream support #4222

Merged
merged 13 commits into from
Oct 15, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 97 additions & 5 deletions modules/shBidAdapter.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as utils from '../src/utils';
import { config } from '../src/config';
import { Renderer } from '../src/Renderer';
import { registerBidder } from '../src/adapters/bidderFactory';
import { VIDEO, BANNER } from '../src/mediaTypes';

Expand All @@ -12,6 +13,13 @@ const STAGE_VL = 'https://video-library.stage.showheroes.com';
const BIDDER_CODE = 'showheroes-bs';
const TTL = 300;

function getEnvURLs(isStage) {
return {
pubTag: isStage ? STAGE_PUBLISHER_TAG : PROD_PUBLISHER_TAG,
vlHost: isStage ? STAGE_VL : PROD_VL
}
}

export const spec = {
code: BIDDER_CODE,
aliases: ['showheroesBs'],
Expand All @@ -23,6 +31,9 @@ export const spec = {
const pageURL = validBidRequests[0].params.contentPageUrl || bidderRequest.refererInfo.referer;
const isStage = !!validBidRequests[0].params.stage;
const isBanner = !!validBidRequests[0].mediaTypes.banner;
const isOutstream = utils.deepAccess(validBidRequests[0], 'mediaTypes.video.context') === 'outstream';
const isCustomRender = utils.deepAccess(validBidRequests[0], 'params.outstreamOptions.customRender');
const outstreamOptions = utils.deepAccess(validBidRequests[0], 'params.outstreamOptions');

let adUnits = validBidRequests.map((bid) => {
const vpaidMode = utils.getBidIdParameter('vpaidMode', bid.params);
Expand Down Expand Up @@ -57,6 +68,7 @@ export const spec = {
type: streamType,
bidId: bid.bidId,
mediaType: isBanner ? BANNER : VIDEO,
Copy link
Collaborator

Choose a reason for hiding this comment

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

out of curiosity:
will video ads still be displayed if the media type is banner and video?

Copy link
Contributor

Choose a reason for hiding this comment

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

Our player will work and show video or display ad as banner in an iframe, in this case.

context: context,
playerId: utils.getBidIdParameter('playerId', bid.params),
auctionId: bidderRequest.auctionId,
bidderCode: BIDDER_CODE,
Expand All @@ -67,6 +79,7 @@ export const spec = {
width: sizes[0],
height: sizes[1]
},
bidRequest: bidderRequest,
};
});

Expand All @@ -78,8 +91,9 @@ export const spec = {
'user': [],
'meta': {
'pageURL': encodeURIComponent(pageURL),
'vastCacheEnabled': (!!config.getConfig('cache') && !isBanner) || false,
'vastCacheEnabled': (!!config.getConfig('cache') && !isBanner && !outstreamOptions) || false,
'isDesktop': utils.getWindowTop().document.documentElement.clientWidth > 700,
'xmlAndTag': !!(isOutstream && isCustomRender) || false,
'stage': isStage || undefined
},
'requests': adUnits,
Expand Down Expand Up @@ -133,6 +147,14 @@ function createBids(bidRes, reqData) {

bidRes.bids.forEach(function (bid) {
const reqBid = bidMap[bid.bidId];
let currentBidRequest;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be simplified to

Suggested change
let currentBidRequest;
const currentBidRequest = reqBid.bidRequest.bids.find(bid2 => bid.bidId === bid2.bidId);

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @muuki88! Changed :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice 😎

for (let i in reqBid.bidRequest.bids) {
if (bid.bidId === reqBid.bidRequest.bids[i].bidId) {
currentBidRequest = reqBid.bidRequest.bids[i];
break;
}
}

let bidUnit = {};
bidUnit.cpm = bid.cpm;
bidUnit.requestId = bid.bidId;
Expand All @@ -154,25 +176,95 @@ function createBids(bidRes, reqData) {
}
if (reqBid.mediaType === BANNER) {
bidUnit.ad = getBannerHtml(bid, reqBid, reqData);
} else if (reqBid.context === 'outstream') {
const renderer = Renderer.install({
id: bid.bidId,
url: '//',
config: {
playerId: reqBid.playerId,
width: bid.video.width,
height: bid.video.height,
vastUrl: bid.vastTag,
vastXml: bid.vastXml,
debug: reqData.debug,
isStage: !!reqData.meta.stage,
customRender: utils.getBidIdParameter('customRender', currentBidRequest.params.outstreamOptions),
slot: utils.getBidIdParameter('slot', currentBidRequest.params.outstreamOptions),
iframe: utils.getBidIdParameter('iframe', currentBidRequest.params.outstreamOptions),
}
});
renderer.setRender(outstreamRender);
bidUnit.renderer = renderer;
}
bids.push(bidUnit);
});

return bids;
}

function outstreamRender(bid) {
const embedCode = createOutstreamEmbedCode(bid);
if (typeof bid.renderer.config.customRender === 'function') {
bid.renderer.config.customRender(bid, embedCode);
} else {
try {
const inIframe = utils.getBidIdParameter('iframe', bid.renderer.config);
if (inIframe && window.document.getElementById(inIframe).nodeName === 'IFRAME') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How should this work with DFP?

The iframe option is hard, because the iframe ID is assigned by gpt.js, which can change the naming scheme at any time, breaking the integration.

The slot option doesn't work either as DFP creatives are always delivered in an iframe and the standard prebid creative doesn't have any div elements.

From other implementations there are better options:

inFrame flag

If you really need to know if you are in an iFrame or not use a flag instead. Then append the necessary code to the body and head. Scripts, divs, whatever you want. Not need to break out and search for the iFrame.

slot_id

If you want to break out of the iFrame, then try to do something like window.top.document.getElementById(..). I guess prebid has some helper functions that prevent illegal access if the creative was not delivered in a friendly iframe.

Copy link
Contributor

@veranevera veranevera Oct 1, 2019

Choose a reason for hiding this comment

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

These options are not for DFP integration. They need for our publishers, which want to set our outstream directly on their page. They can use id of existing iframe or slot id (can be used any html element).

If you would like to integrate our outstream with DFP, please, give me a link with an example of your bidding. We will try to found a solution.

Copy link
Collaborator

@muuki88 muuki88 Oct 1, 2019

Choose a reason for hiding this comment

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

tl;dr IMHO the slot_id and iframe parameter are unnecessary for an integration that relies on an Ad Server such as DFP that delivers creatives in iframes. From my limited experience it's a safe guess that the player was delivered in an iframe through and ad server. Especially as a Prebid module.

These options are not for DFP integration. They need for our publishers, which want to set our outstream directly on their page. They can use id of existing iframe or slot id (can be used any html element).

I'm not sure how prevalent DFP is as an Ad Server, but I guess there's a fair amount of publisher that use DFP as an Ad Server along with Prebid.

Your solution is what is documented as Option 2: Serving without an ad server in the outstream-video-ads documentation. We haven't heard of any publisher doing this.

If you would like to integrate our outstream with DFP, please, give me a link with an example of your bidding. We will try to found a solution.

This is not necessarily DFP relevant. Delivering ad creatives in an iFrame is IMHO a good practice to avoid ad fraud and illegal page access by creatives.

My point here is that the implementation is unnecessary complex and hinders the integration with any Ad Server that delivers in iFrames. At least this is how I understand the code 😉 . Please correct me if I missed something obvious.

Here's the example page from prebid outstream working examples section:

http://prebid.org/dev-docs/show-outstream-video-ads.html

The appNexus outstream renders everything inside the iframe and only adds a small div for handling the expansion of the video. As mentioned in the initial issue this is how the appNexus adapter does it:
AppNexus Adpater newRenderer and AppNexus Adapter oustreamRenderer

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @muuki88! We created a demo page how we can work with DFP http://test.showheroes.com/schema/sh-prebid-outstream-google
Please, take a look. We will return "banner" type and you can use "renderAd" function inside DFP. In this case, no need to use the frame wrapper for custom frames and slots.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @veranevera 😃

Thanks a lot for the example page 👍 The banner solutions sounds good to me. Still unsure about the use case of the other options in Prebid, but you know what you are doing 😁

I'll give it a try and keep you posted. I saw that you were using 1x1 as a size, so the player resizes itself, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

@muuki88 we are looking forward 💃
The player from the demo is responsive :) do you need fixed sizes?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope. Responsive is perfect 😄

const iframe = window.document.getElementById(inIframe);
let framedoc = iframe.contentDocument || (iframe.contentWindow && iframe.contentWindow.document);
framedoc.body.appendChild(embedCode);
return;
}

const slot = utils.getBidIdParameter('slot', bid.renderer.config);
if (slot && window.document.getElementById(slot)) {
window.document.getElementById(slot).appendChild(embedCode);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
window.document.getElementById(slot).appendChild(embedCode);
window.document.getElementById(slot).appendChild(embedCode);
return;

} else if (slot) {
utils.logError('[ShowHeroes][renderer] Error: spot not found');
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
}
}
// Served in an iframe by an ad server
window.document.body.appendChild(embedCode);

The last case would be to just insert the embedCode inside the iframe.

} catch (err) {
utils.logError('[ShowHeroes][renderer] Error:' + err.message)
}
}
}

function createOutstreamEmbedCode(bid) {
const isStage = utils.getBidIdParameter('isStage', bid.renderer.config);
const urls = getEnvURLs(isStage);

const fragment = window.document.createDocumentFragment();

const script = window.document.createElement('script');
script.type = 'text/javascript';
script.src = urls.pubTag;
script.onload = function () {
window.ShowheroesTag=this;
Copy link
Collaborator

@robertrmartinez robertrmartinez Sep 26, 2019

Choose a reason for hiding this comment

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

/home/circleci/Prebid.js/modules/shBidAdapter.js
  241:25  error  Infix operators must be spaced  space-infix-ops

Please update this so circleCi passes

window.ShowheroesTag = this;

};
script.setAttribute('data-player-host', urls.vlHost);

const spot = window.document.createElement('div');
spot.setAttribute('class', 'showheroes-spot');
spot.setAttribute('data-player', utils.getBidIdParameter('playerId', bid.renderer.config));
spot.setAttribute('data-debug', utils.getBidIdParameter('debug', bid.renderer.config));
spot.setAttribute('data-ad-vast-tag', utils.getBidIdParameter('vastUrl', bid.renderer.config));
spot.setAttribute('data-stream-type', 'outstream');

fragment.appendChild(spot);
fragment.appendChild(script);
return fragment;
}

function getBannerHtml (bid, reqBid, reqData) {
const isStage = !!reqData.meta.stage;
const pubTag = isStage ? STAGE_PUBLISHER_TAG : PROD_PUBLISHER_TAG;
const vlHost = isStage ? STAGE_VL : PROD_VL;
const urls = getEnvURLs(isStage);
return `<html>
<head></head>
<body>
<script async src="${pubTag}"
<script async src="${urls.pubTag}"
data-canvas=""
data-noad-passback-listener=""
onload="window.ShowheroesTag=this"
data-player-host="${vlHost}"></script>
data-player-host="${urls.vlHost}"></script>
<div class="showheroes-spot"
data-debug="${reqData.debug ? '1' : ''}"
data-player="${reqBid.playerId}"
Expand Down
36 changes: 36 additions & 0 deletions modules/shBidAdapter.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,42 @@ Module that connects to ShowHeroes demand source to fetch bids.
}
]
},
{
code: 'video',
mediaTypes: {
video: {
playerSize: [640, 480],
context: 'outstream',
}
},
bids: [
{
bidder: "showheroes-bs",
Copy link
Collaborator

@muuki88 muuki88 Oct 8, 2019

Choose a reason for hiding this comment

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

During my testing I found that this bidder code causes issue with DFP as - is not a valid character for the PATTERN:key macro (see ad manager docs).

I used the alias showheroesBs instead, which works.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this something we want to fix / update?

params: {
playerId: '0151f985-fb1a-4f37-bb26-cfc62e43ec05',
vpaidMode: true, // by default is 'false'
outstreamOptions: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add a note here that this is optional

Copy link
Contributor

Choose a reason for hiding this comment

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

@muuki88 right, thanks! Will do :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome 🍸

// Required for the default outstream renderer, one of
iframe: 'iframe_id',
slot: 'slot_id'

// Custom outstream rendering function
customRender: function(bid, embedCode) {
// Example with embedCode
someContainer.appendChild(embedCode);

// bid config data
var vastUrl = bid.renderer.config.vastUrl;
var vastXML = bid.renderer.config.vastXML;
var videoWidth = bid.renderer.config.width;
var videoHeight = bid.renderer.config.height;
var playerId = bid.renderer.config.playerId;
},
}
}
}
]
},
{
code: 'banner',
mediaTypes: {
Expand Down