From 6ad1cb5ed88fe8bbc1f3d761625182f4ce93d09e Mon Sep 17 00:00:00 2001 From: glevitzky Date: Thu, 7 Mar 2019 09:20:35 -0500 Subject: [PATCH] Enables 'msz' and 'psz' parameters on doubleclick ad requests. (#21159) * Initial * stash * Remove unrelated test page. * Move function to utils and some minor clenaup. * Added tests. * Fixes & test updates. * Fix * Experiment logic. * Test regexp fix, lint fix, dep-check fix, and check-type fix. * Remove stray comment. * Minor tweak + testing non-presence of eid. * Use assertElement over cast. * Fix test desc + minor test regexp fix. * Use url encoded commas in regexp. * PR feedback. * Refactor so that getContainerWidth is executed at most once per request. --- ads/google/a4a/test/test-utils.js | 128 ++++++++++++++++++ ads/google/a4a/utils.js | 50 +++++++ build-system/dep-check-config.js | 1 + examples/fluid.amp.html | 6 +- .../0.1/amp-ad-network-doubleclick-impl.js | 38 +++++- .../test-amp-ad-network-doubleclick-impl.js | 30 ++++ 6 files changed, 249 insertions(+), 4 deletions(-) diff --git a/ads/google/a4a/test/test-utils.js b/ads/google/a4a/test/test-utils.js index 11a249bd32ae2..d3035b4430c7f 100644 --- a/ads/google/a4a/test/test-utils.js +++ b/ads/google/a4a/test/test-utils.js @@ -26,6 +26,7 @@ import { extractAmpAnalyticsConfig, extractHost, getAmpRuntimeTypeParameter, + getContainerWidth, getCorrelator, getCsiAmpAnalyticsVariables, getEnclosingContainerTypes, @@ -988,3 +989,130 @@ describes.realWin('#groupAmpAdsByType', {amp: true}, env => { }); }); }); + +describes.realWin('#getContainerWidth', {amp: true}, env => { + let doc, win; + beforeEach(() => { + win = env.win; + doc = win.document; + }); + + function createResource( + config, layout, tagName = 'amp-ad', parent = doc.body) { + config['layout'] = layout; + const element = createElementWithAttributes(doc, tagName, config); + parent.appendChild(element); + return element; + } + + it('should return the fixed width for FIXED layout', () => { + const element = createResource({width: 300, height: 250}, 'fixed'); + expect(getContainerWidth(win, element)).to.equal(300); + }); + + it('should return 0 for FIXED layout and invalid width', () => { + allowConsoleError(() => { + const element = createResource({width: 'auto', height: 250}, 'fixed'); + expect(getContainerWidth(win, element)).to.equal(0); + }); + }); + + it('should return 0 for NODISPLAY layout', () => { + const element = createResource({width: 500}, 'nodisplay'); + expect(getContainerWidth(win, element)).to.equal(0); + }); + + it('should return 0 for FLEX_ITEM layout', () => { + const element = createResource({width: 500}, 'flex-item'); + expect(getContainerWidth(win, element)).to.equal(0); + }); + + it('should return 0 for invalid layout', () => { + allowConsoleError(() => { + const element = createResource({width: 500}, 'qwerty'); + expect(getContainerWidth(win, element)).to.equal(0); + }); + }); + + it('should return the max-width, if present, for FILL layout', () => { + const element = createResource({maxWidth: 300}, 'fill'); + expect(getContainerWidth(win, element)).to.equal(300); + }); + + it('should return parent\'s fixed width for FILL layout', () => { + const parent = document.createElement('div'); + parent.setAttribute('width', 300); + parent.setAttribute('layout', 'fixed'); + doc.body.appendChild(parent); + const element = createResource({} /* config */, 'fill', 'amp-ad', parent); + expect(getContainerWidth(win, element)).to.equal(300); + }); + + it('should return the max-width, if present, for FIXED_HEIGHT layout', + () => { + const element = createResource({height: 300}, 'fixed-height'); + element.style.maxWidth = '250px'; + expect(getContainerWidth(win, element)).to.equal(250); + }); + + it('should return parent\'s fixed width for FIXED_HEIGHT layout', () => { + const parent = document.createElement('div'); + parent.setAttribute('width', 300); + parent.setAttribute('layout', 'fixed'); + doc.body.appendChild(parent); + const element = createResource( + {height: 250}, 'fixed-height', 'amp-ad', parent); + expect(getContainerWidth(win, element)).to.equal(300); + }); + + it('should return the max-width, if present, for FLUID layout', () => { + const element = createResource({height: 300}, 'fluid'); + element.style.maxWidth = '250px'; + expect(getContainerWidth(win, element)).to.equal(250); + }); + + it('should return parent\'s fixed width for FLUID layout', () => { + const parent = document.createElement('div'); + parent.setAttribute('width', 300); + parent.setAttribute('layout', 'fixed'); + doc.body.appendChild(parent); + const element = createResource( + {height: 250}, 'fluid', 'amp-ad', parent); + expect(getContainerWidth(win, element)).to.equal(300); + }); + + it('should return the max-width, if present, for RESPONSIVE layout', () => { + const element = createResource({height: 200, width: 200}, 'responsive'); + element.style.maxWidth = '250px'; + expect(getContainerWidth(win, element)).to.equal(250); + }); + + it('should return parent\'s fixed width for RESPONSIVE layout', () => { + const parent = document.createElement('div'); + parent.setAttribute('width', 300); + parent.setAttribute('layout', 'fixed'); + doc.body.appendChild(parent); + const element = createResource( + {height: 250, width: 250}, 'responsive', 'amp-ad', parent); + expect(getContainerWidth(win, element)).to.equal(300); + }); + + it('should return the viewport width for CONTAINER layout', () => { + const element = createResource({} /* config */, 'container'); + sandbox.stub(Services.viewportForDoc(element), 'getSize') + .returns({width: 300}); + expect(getContainerWidth(win, element)).to.equal(300); + }); + + it('should return -1 width for non-fixed layouts, maxDepth = 1', () => { + ['fill', 'fixed-height', 'fluid', 'responsive'].forEach(layout => { + const parent = document.createElement('div'); + parent.setAttribute('width', 300); + parent.setAttribute('layout', 'fixed'); + doc.body.appendChild(parent); + const element = createResource( + {height: 250}, layout, 'amp-ad', parent); + expect(getContainerWidth(win, element, 1)).to.equal(-1); + }); + }); +}); diff --git a/ads/google/a4a/utils.js b/ads/google/a4a/utils.js index 372fe5d41f979..ba2c1b88207f0 100644 --- a/ads/google/a4a/utils.js +++ b/ads/google/a4a/utils.js @@ -16,8 +16,10 @@ import {CONSENT_POLICY_STATE} from '../../../src/consent-state'; import {DomFingerprint} from '../../../src/utils/dom-fingerprint'; +import {Layout} from '../../../src/layout'; import {Services} from '../../../src/services'; import {buildUrl} from './shared/url-builder'; +import {computedStyle} from '../../../src/style'; import {dev, devAssert} from '../../../src/log'; import {dict} from '../../../src/utils/object'; import { @@ -960,3 +962,51 @@ export function getAmpRuntimeTypeParameter(win) { const art = getBinaryTypeNumericalCode(getBinaryType(win)); return isCdnProxy(win) && art != '0' ? art : null; } + +/** + * Returns the fixed size of the given element, or the fixed size of its nearest + * ancestor that has a fixed size, if the given element has none. + * @param {!Window} win + * @param {?Element} element + * @param {number=} maxDepth The maximum number of ancestors to check. + * @return {number} The width of the given element, or of the nearest ancestor + * with a fixed size, if the given element has none. + */ +export function getContainerWidth(win, element, maxDepth = 100) { + let el = element; + let depth = maxDepth; + // Find the first ancestor with a fixed size. + while (el && depth--) { + const layout = el.getAttribute('layout'); + switch (layout) { + case Layout.FIXED: + return parseInt(el.getAttribute('width'), 10) || 0; + case Layout.RESPONSIVE: + case Layout.FILL: + case Layout.FIXED_HEIGHT: + case Layout.FLUID: + // The above layouts determine the width of the element by the + // containing element, or by CSS max-width property. + const maxWidth = parseInt(computedStyle(win, el).maxWidth, 10); + if (maxWidth || maxWidth == 0) { + return maxWidth; + } + el = el.parentElement; + break; + case Layout.CONTAINER: + // Container layout allows the container's size to be determined by + // the children within it, so in principle we can grow as large as the + // viewport. + const viewport = Services.viewportForDoc(dev().assertElement(element)); + return viewport.getSize().width; + case Layout.NODISPLAY: + case Layout.FLEX_ITEM: + return 0; + default: + // If no layout is provided, we must use getComputedStyle. + return parseInt(computedStyle(win, el).width, 10) || 0; + } + } + return -1; +} + diff --git a/build-system/dep-check-config.js b/build-system/dep-check-config.js index 69ffc0487e4ca..d2e4520b5beed 100644 --- a/build-system/dep-check-config.js +++ b/build-system/dep-check-config.js @@ -165,6 +165,7 @@ exports.rules = [ 'ads/google/a4a/**->src/experiments.js', 'ads/google/a4a/**->src/services.js', 'ads/google/a4a/utils.js->src/service/variable-source.js', + 'ads/google/a4a/utils.js->src/layout.js', // alp handler needs to depend on src files 'ads/alp/handler.js->src/dom.js', 'ads/alp/handler.js->src/config.js', diff --git a/examples/fluid.amp.html b/examples/fluid.amp.html index 1a7efa2376a7f..5010eb324cf01 100644 --- a/examples/fluid.amp.html +++ b/examples/fluid.amp.html @@ -21,9 +21,9 @@

AMP Static Image DFP Reservation.

Tests a static image ad in an AMP page.

-
- +
+
- + diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js b/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js index 393262b95eeae..9353eb7e4c1a2 100644 --- a/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js +++ b/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js @@ -34,6 +34,7 @@ import { ValidAdContainerTypes, addCsiSignalsToAmpAnalyticsConfig, extractAmpAnalyticsConfig, + getContainerWidth, getCsiAmpAnalyticsConfig, getCsiAmpAnalyticsVariables, getEnclosingContainerTypes, @@ -119,6 +120,15 @@ const DOUBLECLICK_SRA_EXP_BRANCHES = { SRA_NO_RECOVER: '21062235', }; +/** @const {string} */ +const FLEXIBLE_AD_SLOTS_EXP = 'flexAdSlots'; + +/** @const @enum{string} */ +const FLEXIBLE_AD_SLOTS_BRANCHES = { + CONTROL: '21063173', + EXPERIMENT: '21063174', +}; + /** * Map of pageview tokens to the instances they belong to. * @private {!Object} @@ -267,6 +277,9 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A { * @private {boolean} */ this.shouldSandbox_ = false; + + /** @private {boolean} */ + this.sendFlexibleAdSlotParams_ = false; } /** @@ -363,10 +376,19 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A { branches: Object.keys(DOUBLECLICK_SRA_EXP_BRANCHES).map( key => DOUBLECLICK_SRA_EXP_BRANCHES[key]), }, + [FLEXIBLE_AD_SLOTS_EXP]: { + isTrafficEligible: () => true, + branches: Object.values(FLEXIBLE_AD_SLOTS_BRANCHES), + }, }); const setExps = this.randomlySelectUnsetExperiments_(experimentInfoMap); Object.keys(setExps).forEach(expName => setExps[expName] && this.experimentIds.push(setExps[expName])); + if (setExps[FLEXIBLE_AD_SLOTS_EXP] && + setExps[FLEXIBLE_AD_SLOTS_EXP] == + FLEXIBLE_AD_SLOTS_BRANCHES.EXPERIMENT) { + this.sendFlexibleAdSlotParams_ = true; + } } /** @@ -472,6 +494,17 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A { this.win['ampAdGoogleIfiCounter']++; const pageLayoutBox = this.isSinglePageStoryAd ? this.element.getPageLayoutBox() : null; + let psz = null; + let msz = null; + if (this.sendFlexibleAdSlotParams_) { + const parentWidth = getContainerWidth( + this.win, this.element.parentElement); + let slotWidth = getContainerWidth( + this.win, this.element, 1 /* maxDepth */); + slotWidth = slotWidth == -1 ? parentWidth : slotWidth; + psz = `${parentWidth}x-1`; + msz = `${slotWidth}x-1`; + } return Object.assign({ 'iu': this.element.getAttribute('data-slot'), 'co': this.jsonTargeting && @@ -487,6 +520,10 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A { 'frc': Number(this.fromResumeCallback) || null, 'fluid': this.isFluidRequest_ ? 'height' : null, 'fsf': this.forceSafeframe ? '1' : null, + // Both msz/psz send a height of -1 because height expansion is + // disallowed in AMP. + 'msz': msz, + 'psz': psz, 'scp': serializeTargeting( (this.jsonTargeting && this.jsonTargeting['targeting']) || null, (this.jsonTargeting && @@ -1406,7 +1443,6 @@ AMP.extension(TAG, '0.1', AMP => { AMP.registerElement(TAG, AmpAdNetworkDoubleclickImpl); }); - /** @visibleForTesting */ export function resetSraStateForTesting() { sraRequests = null; diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js b/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js index c4d96259ae303..1cbf56ccac6ea 100644 --- a/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js +++ b/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js @@ -810,6 +810,36 @@ describes.realWin('amp-ad-network-doubleclick-impl', realWinConfig, env => { impl.getAdUrl(CONSENT_POLICY_STATE.UNKNOWN_NOT_REQUIRED).then(url => { expect(url).to.not.match(/(\?|&)npa=(&|$)/); })); + + it('should include msz/psz if in experiment', () => { + sandbox.stub(impl, 'randomlySelectUnsetExperiments_').returns( + {flexAdSlots: '21063174'}); + impl.setPageLevelExperiments(); + return impl.getAdUrl().then(url => { + expect(url).to.match(/(\?|&)msz=[0-9]+x-1(&|$)/); + expect(url).to.match(/(\?|&)psz=[0-9]+x-1(&|$)/); + expect(url).to.match(/(=|%2C)21063174(%2C|&|$)/); + }); + }); + + it('should not include msz/psz if not in flexAdSlots control', () => { + sandbox.stub(impl, 'randomlySelectUnsetExperiments_').returns( + {flexAdSlots: '21063173'}); + impl.setPageLevelExperiments(); + return impl.getAdUrl().then(url => { + expect(url).to.not.match(/(\?|&)msz=/); + expect(url).to.not.match(/(\?|&)psz=/); + expect(url).to.match(/(=|%2C)21063173(%2C|&|$)/); + }); + }); + + it('should not include msz/psz if not in flexAdSlots experiment', () => { + return impl.getAdUrl().then(url => { + expect(url).to.not.match(/(\?|&)msz=/); + expect(url).to.not.match(/(\?|&)psz=/); + expect(url).to.not.match(/(=|%2C)2106317(3|4)(%2C|&|$)/); + }); + }); }); describe('#getPageParameters', () => {