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

iOS Referrer fix #996

Merged
merged 5 commits into from
Mar 7, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
31 changes: 18 additions & 13 deletions src/prebid.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/** @module $$PREBID_GLOBAL$$ */

import { getGlobal } from './prebidGlobal';
import {flatten, uniques, isGptPubadsDefined, adUnitsFilter} from './utils';
import {flatten, uniques, isGptPubadsDefined, adUnitsFilter, isSrcdocSupported} from './utils';
import { videoAdUnit, hasNonVideoBidder } from './video';
import 'polyfill';
import {parse as parseURL, format as formatURL} from './url';
Expand Down Expand Up @@ -72,7 +72,7 @@ $$PREBID_GLOBAL$$.adUnits = $$PREBID_GLOBAL$$.adUnits || [];

/**
* Command queue that functions will execute once prebid.js is loaded
* @param {function} cmd Annoymous function to execute
* @param {function} cmd Anonymous function to execute
* @alias module:$$PREBID_GLOBAL$$.que.push
*/
$$PREBID_GLOBAL$$.que.push = function (cmd) {
Expand Down Expand Up @@ -277,8 +277,9 @@ $$PREBID_GLOBAL$$.allBidsAvailable = function () {
};

/**
* This function will render the ad (based on params) in the given iframe document passed through. Note that doc SHOULD NOT be the parent document page as we can't doc.write() asynchrounsly
* @param {object} doc document
* This function will render the ad (based on params) in the given iframe document passed through.
* Note that doc SHOULD NOT be the parent document page as we can't doc.write() asynchronously
* @param {HTMLDocument} doc document
* @param {string} id bid id to locate the ad
* @alias module:$$PREBID_GLOBAL$$.renderAd
*/
Expand All @@ -299,15 +300,19 @@ $$PREBID_GLOBAL$$.renderAd = function (doc, id) {
var width = adObject.width;
var url = adObject.adUrl;
var ad = adObject.ad;

if (doc===document || adObject.mediaType === 'video') {
utils.logError('Error trying to write ad. Ad render call ad id ' + id + ' was prevented from writing to the main document.');
if (doc === document || adObject.mediaType === 'video') {
utils.logError(`Error trying to write ad. Ad render call ad id ${id} was prevented from writing to the main document.`);
} else if (ad) {
doc.write(ad);
doc.close();
if (isSrcdocSupported(doc)) {
doc.defaultView.frameElement.srcdoc = ad;
Copy link
Member

Choose a reason for hiding this comment

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

It looks like srcdoc completely wipes out any other content present in the iframe. This is causing DFP to fail to report on viewable impressions. I will be reverting this change until we can find a proper solution.

Copy link
Member

Choose a reason for hiding this comment

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

@ckbo3hrk fyi

Copy link
Contributor Author

@ckbo3hrk ckbo3hrk Apr 18, 2017

Choose a reason for hiding this comment

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

@mkendall07, I'd like to investigate this bug. Could you please provide me a sample dfp tag to reproduce this issue ?

Copy link
Member

@mkendall07 mkendall07 Apr 18, 2017

Choose a reason for hiding this comment

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

@ckbo3hrk
Here is an example:
Working (filter by "activeview" in network)
[working updating links]
Not working:

} else {
doc.write(ad);
doc.close();
}
setRenderSize(doc, width, height);
} else if (url) {
doc.write('<IFRAME SRC="' + url + '" FRAMEBORDER="0" SCROLLING="no" MARGINHEIGHT="0" MARGINWIDTH="0" TOPMARGIN="0" LEFTMARGIN="0" ALLOWTRANSPARENCY="true" WIDTH="' + width + '" HEIGHT="' + height + '"></IFRAME>');
doc.write(`<IFRAME SRC="${url}" FRAMEBORDER="0" SCROLLING="no" MARGINHEIGHT="0" MARGINWIDTH="0" TOPMARGIN="0" LEFTMARGIN="0" ALLOWTRANSPARENCY="true" WIDTH="${width}" HEIGHT="${height}"></IFRAME>`);
doc.close();
setRenderSize(doc, width, height);
} else {
Expand Down Expand Up @@ -483,7 +488,7 @@ $$PREBID_GLOBAL$$.offEvent = function (event, handler, id) {
/**
* Add a callback event
* @param {String} eventStr event to attach callback to Options: "allRequestedBidsBack" | "adUnitBidsBack"
* @param {Function} func function to execute. Paramaters passed into the function: (bidResObj), [adUnitCode]);
* @param {Function} func function to execute. Parameters passed into the function: (bidResObj), [adUnitCode]);
* @alias module:$$PREBID_GLOBAL$$.addCallback
* @returns {String} id for callback
*/
Expand Down Expand Up @@ -586,7 +591,7 @@ $$PREBID_GLOBAL$$.loadScript = function (tagSrc, callback, useCache) {
};

/**
* Will enable sendinga prebid.js to data provider specified
* Will enable sending a prebid.js to data provider specified
* @param {object} config object {provider : 'string', options : {}}
*/
$$PREBID_GLOBAL$$.enableAnalytics = function (config) {
Expand Down Expand Up @@ -679,7 +684,7 @@ $$PREBID_GLOBAL$$.buildMasterVideoTagFromAdserverTag = function (adserverTag, op

/**
* Set the order bidders are called in. If not set, the bidders are called in
* the order they are defined wihin the adUnit.bids array
* the order they are defined within the adUnit.bids array
* @param {string} order - Order to call bidders in. Currently the only possible value
* is 'random', which randomly shuffles the order
*/
Expand Down
2 changes: 1 addition & 1 deletion src/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export function parse(url) {
protocol: (parsed.protocol || '').replace(/:$/, ''),
hostname: parsed.hostname,
port: +parsed.port,
pathname: parsed.pathname,
pathname: parsed.pathname.replace(/^(?!\/)/,'/'),
search: parseQS(parsed.search || ''),
hash: (parsed.hash || '').replace(/^#/, ''),
host: parsed.host
Expand Down
9 changes: 9 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -592,3 +592,12 @@ export function shuffle(array) {
export function adUnitsFilter(filter, bid) {
return filter.includes(bid && bid.placementCode || bid && bid.adUnitCode);
}

/**
* Check if parent iframe of passed document supports content rendering via 'srcdoc' property
* @param {HTMLDocument} doc document to check support of 'srcdoc'
*/
export function isSrcdocSupported(doc) {
//Firefox is excluded due to https://bugzilla.mozilla.org/show_bug.cgi?id=1265961
return !!doc.defaultView && 'srcdoc' in doc.defaultView.frameElement && !/firefox/i.test(navigator.userAgent);
}
2 changes: 1 addition & 1 deletion test/spec/url_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('helpers.url', () => {
});

it('extracts the pathname', () => {
expect(['/pathname/', 'pathname/']).to.include(parsed.pathname);
expect(parsed).to.have.property('pathname', '/pathname/');
});

it('extracts the search query', () => {
Expand Down