Skip to content

Commit b2410cb

Browse files
dgirardidgirardi
authored
Fix bugs / tests with empty string default for missing assets (#178)
* Fix bugs / tests with empty string default for missing assets * Run tests on PRs * Update test browsers to be in line with Prebid.js * Update circleCI image to be in line with the one used for releases
1 parent 99d4c0e commit b2410cb

File tree

4 files changed

+56
-69
lines changed

4 files changed

+56
-69
lines changed

‎.circleci/config.yml

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ jobs:
77
build:
88
docker:
99
# specify the version you desire here
10-
- image: circleci/node:8.9.0
11-
10+
- image: circleci/node:14.18.2-browsers
11+
1212
# Specify service dependencies here if necessary
1313
# CircleCI maintains a library of pre-built images
1414
# documented at https://circleci.com/docs/2.0/circleci-images/
@@ -45,6 +45,6 @@ jobs:
4545
# Run the file with user's access key
4646
./BrowserStackLocal ${BROWSERSTACK_ACCESS_KEY} &
4747
# run tests!
48-
- run:
48+
- run:
4949
name: BrowserStack testing
5050
command: gulp test --browserstack

‎browsers.json

+17-33
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,50 @@
11
{
2-
"bs_edge_16_windows_10": {
2+
"bs_edge_latest_windows_10": {
33
"base": "BrowserStack",
44
"os_version": "10",
55
"browser": "edge",
6-
"browser_version": "16.0",
6+
"browser_version": "latest",
77
"device": null,
88
"os": "Windows"
99
},
10-
"bs_edge_17_windows_10": {
11-
"base": "BrowserStack",
12-
"os_version": "10",
13-
"browser": "edge",
14-
"browser_version": "17.0",
15-
"device": null,
16-
"os": "Windows"
17-
},
18-
"bs_ie_11_windows_10": {
19-
"base": "BrowserStack",
20-
"os_version": "10",
21-
"browser": "ie",
22-
"browser_version": "11.0",
23-
"device": null,
24-
"os": "Windows"
25-
},
26-
"bs_chrome_76_windows_10": {
10+
"bs_chrome_latest_windows_10": {
2711
"base": "BrowserStack",
2812
"os_version": "10",
2913
"browser": "chrome",
30-
"browser_version": "76.0",
14+
"browser_version": "latest",
3115
"device": null,
3216
"os": "Windows"
3317
},
34-
"bs_chrome_77_windows_10": {
18+
"bs_chrome_87_windows_10": {
3519
"base": "BrowserStack",
3620
"os_version": "10",
3721
"browser": "chrome",
38-
"browser_version": "77.0",
22+
"browser_version": "87.0",
3923
"device": null,
4024
"os": "Windows"
4125
},
42-
"bs_firefox_68_windows_10": {
26+
"bs_firefox_latest_windows_10": {
4327
"base": "BrowserStack",
4428
"os_version": "10",
4529
"browser": "firefox",
46-
"browser_version": "68.0",
30+
"browser_version": "latest",
4731
"device": null,
4832
"os": "Windows"
4933
},
50-
"bs_firefox_69_windows_10": {
34+
"bs_safari_latest_mac_bigsur": {
5135
"base": "BrowserStack",
52-
"os_version": "10",
53-
"browser": "firefox",
54-
"browser_version": "69.0",
36+
"os_version": "Big Sur",
37+
"browser": "safari",
38+
"browser_version": "latest",
5539
"device": null,
56-
"os": "Windows"
40+
"os": "OS X"
5741
},
58-
"bs_safari_12.1_mac_mojave": {
42+
"bs_safari_15_catalina": {
5943
"base": "BrowserStack",
60-
"os_version": "Mojave",
44+
"os_version": "Catalina",
6145
"browser": "safari",
62-
"browser_version": "12.1",
46+
"browser_version": "13.1",
6347
"device": null,
6448
"os": "OS X"
6549
}
66-
}
50+
}

‎src/nativeAssetManager.js

+34-31
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,14 @@ export function newNativeAssetManager(win, pubUrl) {
161161
}
162162

163163
function loadMobileAssets(tagData, cb) {
164-
const placeholders = scanForPlaceholders();
164+
const placeholders = scanDOMForPlaceHolders();
165165
if (placeholders.length > 0) {
166166
callback = cb;
167167
requestAssetsFromCache(tagData);
168168
}
169169
}
170170

171-
function pbNativeDataHasValidType() {
171+
function hasPbNativeData() {
172172
return typeof win.pbNativeData !== 'undefined'
173173
}
174174

@@ -180,16 +180,16 @@ export function newNativeAssetManager(win, pubUrl) {
180180
* to retrieve native assets that have a value on the corresponding bid
181181
*/
182182
function loadAssets(adId, cb) {
183-
const placeholders = scanForPlaceholders(adId);
183+
const placeholders = scanDOMForPlaceHolders(adId);
184184

185-
if (pbNativeDataHasValidType() && win.pbNativeData.hasOwnProperty('assetsToReplace')) {
185+
if (hasPbNativeData() && win.pbNativeData.hasOwnProperty('assetsToReplace')) {
186186
win.pbNativeData.assetsToReplace.forEach((asset) => {
187187
const key = (asset.match(/hb_native_/i)) ? asset : NATIVE_KEYS[asset];
188188
if (key) {placeholders.push(key);}
189189
});
190190
}
191191

192-
if (pbNativeDataHasValidType() && win.pbNativeData.hasOwnProperty('requestAllAssets') && win.pbNativeData.requestAllAssets) {
192+
if (hasPbNativeData() && win.pbNativeData.hasOwnProperty('requestAllAssets') && win.pbNativeData.requestAllAssets) {
193193
callback = cb;
194194
cancelMessageListener = requestAllAssets(adId);
195195
} else if (placeholders.length > 0) {
@@ -198,24 +198,29 @@ export function newNativeAssetManager(win, pubUrl) {
198198
}
199199
}
200200

201+
function placeholderFor(key, adId) {
202+
return (adId && !hasPbNativeData()) ? `${key}:${adId}` : ((hasPbNativeData()) ? `##${key}##` : key)
203+
}
204+
205+
function scanForPlaceHolders(adId, ...markupFragments) {
206+
return Object.values(NATIVE_KEYS)
207+
.reduce((found, key) => {
208+
const placeholder = placeholderFor(key, adId);
209+
for (const mkup of markupFragments.filter(Boolean)) {
210+
if (mkup.indexOf(placeholder) >= 0) {
211+
found.push(key);
212+
break;
213+
}
214+
}
215+
return found;
216+
}, []);
217+
}
218+
201219
/*
202220
* Searches the DOM for placeholder values sent in by Prebid Native
203221
*/
204-
function scanForPlaceholders(adId) {
205-
let placeholders = [];
206-
const doc = win.document;
207-
208-
Object.keys(NATIVE_KEYS).forEach(key => {
209-
const placeholderKey = NATIVE_KEYS[key];
210-
const placeholder = (adId && !pbNativeDataHasValidType()) ? `${placeholderKey}:${adId}` : `${placeholderKey}`;
211-
const placeholderIndex = (~doc.body.innerHTML.indexOf(placeholder)) ? doc.body.innerHTML.indexOf(placeholder) : (doc.head.innerHTML && doc.head.innerHTML.indexOf(placeholder));
212-
213-
if (~placeholderIndex) {
214-
placeholders.push(placeholderKey);
215-
}
216-
});
217-
218-
return placeholders;
222+
function scanDOMForPlaceHolders(adId) {
223+
return scanForPlaceHolders(adId, win.document.body.innerHTML, win.document.head.innerHTML);
219224
}
220225

221226
/*
@@ -281,13 +286,12 @@ export function newNativeAssetManager(win, pubUrl) {
281286
if (data.message === 'assetResponse') {
282287
const body = win.document.body.innerHTML;
283288
const head = win.document.head.innerHTML;
284-
const flag = pbNativeDataHasValidType();
285289

286-
if (flag && data.adId !== win.pbNativeData.adId) return;
290+
if (hasPbNativeData() && data.adId !== win.pbNativeData.adId) return;
287291

288292
if (head) win.document.head.innerHTML = replace(head, data);
289293

290-
if ((data.hasOwnProperty('rendererUrl') && data.rendererUrl) || (flag && win.pbNativeData.hasOwnProperty('rendererUrl'))) {
294+
if ((data.hasOwnProperty('rendererUrl') && data.rendererUrl) || (hasPbNativeData() && win.pbNativeData.hasOwnProperty('rendererUrl'))) {
291295
if (win.renderAd) {
292296
const newHtml = (win.renderAd && win.renderAd(data.assets)) || '';
293297

@@ -305,7 +309,7 @@ export function newNativeAssetManager(win, pubUrl) {
305309
requestHeightResize(data.adId, (document.body.clientHeight || document.body.offsetHeight));
306310
});
307311
} else {
308-
loadScript(win, ((flag && win.pbNativeData.hasOwnProperty('rendererUrl') && win.pbNativeData.rendererUrl) || data.rendererUrl), function() {
312+
loadScript(win, ((hasPbNativeData() && win.pbNativeData.hasOwnProperty('rendererUrl') && win.pbNativeData.rendererUrl) || data.rendererUrl), function() {
309313
const newHtml = (win.renderAd && win.renderAd(data.assets)) || '';
310314

311315
win.document.body.innerHTML = body + newHtml;
@@ -314,8 +318,8 @@ export function newNativeAssetManager(win, pubUrl) {
314318
requestHeightResize(data.adId, (document.body.clientHeight || document.body.offsetHeight));
315319
})
316320
}
317-
} else if ((data.hasOwnProperty('adTemplate') && data.adTemplate)||(flag && win.pbNativeData.hasOwnProperty('adTemplate'))) {
318-
const template = (flag && win.pbNativeData.hasOwnProperty('adTemplate') && win.pbNativeData.adTemplate) || data.adTemplate;
321+
} else if ((data.hasOwnProperty('adTemplate') && data.adTemplate)||(hasPbNativeData() && win.pbNativeData.hasOwnProperty('adTemplate'))) {
322+
const template = (hasPbNativeData() && win.pbNativeData.hasOwnProperty('adTemplate') && win.pbNativeData.adTemplate) || data.adTemplate;
319323
const newHtml = replace(template, data);
320324
win.document.body.innerHTML = body + newHtml;
321325
callback && callback();
@@ -336,12 +340,11 @@ export function newNativeAssetManager(win, pubUrl) {
336340
* in the given document.
337341
* If there's no actual value, the placeholder gets replaced by an empty string.
338342
*/
339-
function replace(document, { assets, adId }) {
340-
let html = document;
343+
function replace(html, { assets, adId }) {
344+
assets = assets || [];
341345

342-
scanForPlaceholders().forEach(placeholder => {
343-
const flag = pbNativeDataHasValidType();
344-
const searchString = (adId && !flag) ? `${placeholder}:${adId}` : ((flag) ? '##'+`${placeholder}`+'##' : `${placeholder}`);
346+
scanForPlaceHolders(adId, html).forEach(placeholder => {
347+
const searchString = placeholderFor(placeholder, adId);
345348
const searchStringRegex = new RegExp(searchString, 'g');
346349
const fittingAsset = assets.find(asset => placeholder === NATIVE_KEYS[asset.key]);
347350
html = html.replace(searchStringRegex, fittingAsset ? fittingAsset.value : '');

‎test/spec/nativeAssetManager_spec.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,8 @@ describe('nativeAssetManager', () => {
437437
expect(win.document.body.innerHTML).to.include(`
438438
<a href="http://example.com">Click Here</a>
439439
`);
440-
// cta was not a requested asset so this should stay as is
441-
expect(win.document.body.innerHTML).to.include('<h1>hb_native_cta</h1>');
440+
// cta was not in the response so it should default to an empty string
441+
expect(win.document.body.innerHTML).to.include('<h1></h1>');
442442
utils.sendRequest.restore();
443443
})
444444
});

0 commit comments

Comments
 (0)