Skip to content

Commit 582e649

Browse files
authored
native Rendering : fix bug where click trackers are not fired (#12655)
* nativeRendering: fix bug where click trackers are not fired * Cleanup
1 parent e0fe3ac commit 582e649

File tree

6 files changed

+87
-25
lines changed

6 files changed

+87
-25
lines changed

creative/renderers/native/renderer.js

+29-8
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,16 @@ function loadScript(url, doc) {
4545
});
4646
}
4747

48+
function getRenderFrames(node) {
49+
return Array.from(node.querySelectorAll('iframe[srcdoc*="render"]'))
50+
}
51+
52+
function getInnerHTML(node) {
53+
const clone = node.cloneNode(true);
54+
getRenderFrames(clone).forEach(node => node.parentNode.removeChild(node));
55+
return clone.innerHTML;
56+
}
57+
4858
export function getAdMarkup(adId, nativeData, replacer, win, load = loadScript) {
4959
const {rendererUrl, assets, ortb, adTemplate} = nativeData;
5060
const doc = win.document;
@@ -58,21 +68,32 @@ export function getAdMarkup(adId, nativeData, replacer, win, load = loadScript)
5868
return win.renderAd(payload);
5969
});
6070
} else {
61-
return Promise.resolve(replacer(adTemplate ?? doc.body.innerHTML));
71+
return Promise.resolve(replacer(adTemplate ?? getInnerHTML(doc.body)));
6272
}
6373
}
6474

6575
export function render({adId, native}, {sendMessage}, win, getMarkup = getAdMarkup) {
6676
const {head, body} = win.document;
67-
const resize = () => sendMessage(MESSAGE_NATIVE, {
68-
action: ACTION_RESIZE,
69-
height: body.offsetHeight,
70-
width: body.offsetWidth
71-
});
77+
const resize = () => {
78+
// force redraw - for some reason this is needed to get the right dimensions
79+
body.style.display = 'none';
80+
body.style.display = 'block';
81+
sendMessage(MESSAGE_NATIVE, {
82+
action: ACTION_RESIZE,
83+
height: body.offsetHeight,
84+
width: body.offsetWidth
85+
});
86+
}
87+
function replaceMarkup(target, markup) {
88+
// do not remove the rendering logic if it's embedded in this window; things will break otherwise
89+
const renderFrames = getRenderFrames(target);
90+
Array.from(target.childNodes).filter(node => !renderFrames.includes(node)).forEach(node => target.removeChild(node));
91+
target.insertAdjacentHTML('afterbegin', markup);
92+
}
7293
const replacer = getReplacer(adId, native);
73-
head && (head.innerHTML = replacer(head.innerHTML));
94+
replaceMarkup(head, replacer(getInnerHTML(head)));
7495
return getMarkup(adId, native, replacer, win).then(markup => {
75-
body.innerHTML = markup;
96+
replaceMarkup(body, markup);
7697
if (typeof win.postRenderAd === 'function') {
7798
win.postRenderAd({adId, ...native});
7899
}

libraries/creative-renderer-native/renderer.js

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

modules/nativeRendering.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ import {getCreativeRendererSource} from '../src/creativeRenderers.js';
77
function getRenderingDataHook(next, bidResponse, options) {
88
if (isNativeResponse(bidResponse)) {
99
next.bail({
10-
native: getNativeRenderingData(bidResponse, auctionManager.index.getAdUnit(bidResponse))
10+
native: getNativeRenderingData(bidResponse, auctionManager.index.getAdUnit(bidResponse)),
11+
rendererVersion: 2 // 9.28 fixed a rendering bug; this signals to PUC that the native renderer is safe to use
1112
})
1213
} else {
1314
next(bidResponse, options)

src/native.js

+6-3
Original file line numberDiff line numberDiff line change
@@ -436,13 +436,16 @@ function assetsMessage(data, adObject, keys, {index = auctionManager.index} = {}
436436
message: 'assetResponse',
437437
adId: data.adId,
438438
};
439-
let renderData = getRenderingData(adObject).native;
439+
let {native: renderData, rendererVersion} = getRenderingData(adObject);
440440
if (renderData) {
441441
// if we have native rendering data (set up by the nativeRendering module)
442442
// include it in full ("all assets") together with the renderer.
443443
// this is to allow PUC to use dynamic renderers without requiring changes in creative setup
444-
msg.native = Object.assign({}, renderData);
445-
msg.renderer = getCreativeRendererSource(adObject);
444+
Object.assign(msg, {
445+
native: Object.assign({}, renderData),
446+
renderer: getCreativeRendererSource(adObject),
447+
rendererVersion,
448+
})
446449
if (keys != null) {
447450
renderData.assets = renderData.assets.filter(({key}) => keys.includes(key))
448451
}

test/spec/creative/nativeRenderer_spec.js

+45-10
Original file line numberDiff line numberDiff line change
@@ -34,18 +34,24 @@ describe('Native creative renderer', () => {
3434
});
3535
});
3636
describe('otherwise, calls replacer', () => {
37-
let replacer;
37+
let replacer, frame;
3838
beforeEach(() => {
3939
replacer = sinon.stub().returns('markup');
40+
frame = document.createElement('iframe');
41+
document.body.appendChild(frame);
42+
win.document = frame.contentDocument;
4043
});
44+
afterEach(() => {
45+
document.body.removeChild(frame);
46+
})
4147
it('with adTemplate, if present', () => {
4248
return getAdMarkup('123', {adTemplate: 'tpl'}, replacer, win).then((result) => {
4349
expect(result).to.eql('markup');
4450
sinon.assert.calledWith(replacer, 'tpl');
4551
});
4652
});
4753
it('with document body otherwise', () => {
48-
win.document = {body: {innerHTML: 'body'}};
54+
win.document.body.innerHTML = 'body'
4955
return getAdMarkup('123', {}, replacer, win).then((result) => {
5056
expect(result).to.eql('markup');
5157
sinon.assert.calledWith(replacer, 'body');
@@ -186,26 +192,29 @@ describe('Native creative renderer', () => {
186192
});
187193

188194
describe('render', () => {
189-
let getMarkup, sendMessage, adId, nativeData, exc;
195+
let getMarkup, sendMessage, adId, nativeData, exc, frame;
190196
beforeEach(() => {
191197
adId = '123';
192198
nativeData = {}
193199
getMarkup = sinon.stub();
194200
sendMessage = sinon.stub()
195201
exc = sinon.stub();
196-
win.document = {
197-
querySelectorAll() { return [] },
198-
body: {}
199-
}
202+
frame = document.createElement('iframe');
203+
document.body.appendChild(frame);
204+
win.document = frame.contentDocument;
200205
});
201206

207+
afterEach(() => {
208+
document.body.removeChild(frame);
209+
})
210+
202211
function runRender() {
203212
return render({adId, native: nativeData}, {sendMessage, exc}, win, getMarkup)
204213
}
205214

206215
it('replaces placeholders in head, if present', () => {
207216
getMarkup.returns(Promise.resolve(''))
208-
win.document.head = {innerHTML: '##hb_native_asset_id_1##'};
217+
win.document.head.innerHTML = '##hb_native_asset_id_1##';
209218
nativeData.ortb = {
210219
assets: [
211220
{id: 1, data: {value: 'repl'}}
@@ -216,6 +225,14 @@ describe('Native creative renderer', () => {
216225
})
217226
});
218227

228+
it('does not replace iframes with srcdoc that contain "renderer"', () => {
229+
win.document.head.innerHTML = win.document.body.innerHTML = '<iframe srcdoc="renderer"></iframe>';
230+
getMarkup.returns(Promise.resolve(''))
231+
return runRender().then(() => {
232+
expect(Array.from(win.document.querySelectorAll('iframe[srcdoc="renderer"]')).length).to.eql(2);
233+
})
234+
})
235+
219236
it('drops markup on body, and fires imp trackers', () => {
220237
getMarkup.returns(Promise.resolve('markup'));
221238
return runRender().then(() => {
@@ -246,9 +263,27 @@ describe('Native creative renderer', () => {
246263

247264
describe('requests resize', () => {
248265
beforeEach(() => {
266+
const mkNode = () => {
267+
const node = {
268+
innerHTML: '',
269+
childNodes: [],
270+
insertAdjacentHTML: () => {},
271+
style: {},
272+
querySelectorAll: () => [],
273+
cloneNode: () => node
274+
};
275+
return node;
276+
}
277+
win.document = {
278+
head: mkNode(),
279+
body: Object.assign(mkNode(), {
280+
offsetHeight: 123,
281+
offsetWidth: 321
282+
}),
283+
querySelectorAll: () => [],
284+
style: {}
285+
};
249286
getMarkup.returns(Promise.resolve('markup'));
250-
win.document.body.offsetHeight = 123;
251-
win.document.body.offsetWidth = 321;
252287
});
253288

254289
it('immediately, if document is loaded', () => {

test/spec/native_spec.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,8 @@ describe('native.js', function () {
402402
'returns native data': {
403403
renderDataHook(next, bidResponse) {
404404
next.bail({
405-
native: getNativeRenderingData(bidResponse, adUnit)
405+
native: getNativeRenderingData(bidResponse, adUnit),
406+
rendererVersion: 'native-render-version'
406407
});
407408
},
408409
renderSourceHook(next) {
@@ -433,8 +434,9 @@ describe('native.js', function () {
433434
function checkRenderer(message) {
434435
if (withRenderer) {
435436
expect(message.renderer).to.eql('mock-native-renderer')
437+
expect(message.rendererVersion).to.eql('native-render-version');
436438
Object.entries(message).forEach(([key, val]) => {
437-
if (!['native', 'adId', 'message', 'assets', 'renderer'].includes(key)) {
439+
if (!['native', 'adId', 'message', 'assets', 'renderer', 'rendererVersion'].includes(key)) {
438440
expect(message.native[key]).to.eql(val);
439441
}
440442
})

0 commit comments

Comments
 (0)