From b692bf32880910cd52273cb41935098b86fb6725 Mon Sep 17 00:00:00 2001 From: Caleb Cordry Date: Fri, 22 Nov 2019 17:04:46 -0800 Subject: [PATCH 001/436] =?UTF-8?q?=F0=9F=90=9BAllow=20zero=20vals=20in=20?= =?UTF-8?q?validateData=20(#25747)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- 3p/3p.js | 3 ++- test/unit/test-3p.js | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/3p/3p.js b/3p/3p.js index a5b3eb0ae8cfa..7385178b7fea5 100644 --- a/3p/3p.js +++ b/3p/3p.js @@ -232,7 +232,8 @@ export function validateData(data, mandatoryFields, opt_optionalFields) { allowedFields = allowedFields.concat(field); } else { userAssert( - data[field], + // Allow zero values for height, width etc. + data[field] != null, 'Missing attribute for %s: %s.', data.type, field diff --git a/test/unit/test-3p.js b/test/unit/test-3p.js index e4fd3f56c1158..f2a6f69cd1cd4 100644 --- a/test/unit/test-3p.js +++ b/test/unit/test-3p.js @@ -126,6 +126,17 @@ describe('3p', () => { }); }); + it('should allow mandatory fields to have 0 as a value', () => { + validateData( + { + width: 0, + height: 0, + type: 'cats', + }, + ['height', 'width'] + ); + }); + it('should check mandatory fields with alternative options', () => { validateData( { From 1030cb9ca425e7345b62b7ebd05eb6df1f16adba Mon Sep 17 00:00:00 2001 From: Micajuine Ho Date: Mon, 25 Nov 2019 09:06:22 -0800 Subject: [PATCH 002/436] Support consentRequired and deprecate promptIfUnknownForGeo (#25696) * added experiment flag * added configs * changes to prod/canary config * cleaning up * more clean up * merge logic in consent-config * adding in userAssert for consentRequired * Update canary-config.json * Update prod-config.json * Update amp-consent.js * cleaning up * deprecate promptIfUnknownForGeo * consent intialize promise * config merge then validate * check if geoService can't identify * verify consentRequired from endpoint * removed consentState to string * adding comment * bug fixing and geoGroupUnknown * adding in getConsentRequiredPromiseLegacy_() * Update amp-consent.js * requested changes * suggested changes * suggested fixes & fixing unit tests * unit tests, better userAssert messages * adding tests * Suggested changes to tests + bug fixes * starting to write tests for promptunknowngeo * unit tests * remove function*, cleaning up * sandbox change * a little more cleaning up * cleaning * cleaning up logic * fixing tests and promises * testing * bug fix * unit test for cr * test for non remote consentRequired * migrating unit tests * Cleaning code, adding thorough test --- extensions/amp-consent/0.1/amp-consent.js | 24 +++- extensions/amp-consent/0.1/consent-config.js | 23 ++++ .../amp-consent/0.1/test/test-amp-consent.js | 116 ++++++++++------ .../0.1/test/test-consent-config.js | 125 ++++++++++++++++-- 4 files changed, 233 insertions(+), 55 deletions(-) diff --git a/extensions/amp-consent/0.1/amp-consent.js b/extensions/amp-consent/0.1/amp-consent.js index 099854dd87671..8140f451e6785 100644 --- a/extensions/amp-consent/0.1/amp-consent.js +++ b/extensions/amp-consent/0.1/amp-consent.js @@ -439,11 +439,25 @@ export class AmpConsent extends AMP.BaseElement { * @return {!Promise} */ getConsentRequiredPromise_() { - userAssert( - this.consentConfig_['checkConsentHref'] || - this.consentConfig_['promptIfUnknownForGeoGroup'], - 'neither checkConsentHref nor promptIfUnknownForGeoGroup is defined' - ); + if (!isExperimentOn(this.win, 'amp-consent-geo-override')) { + return this.getConsentRequiredPromiseLegacy_(); + } + if (typeof this.consentConfig_['consentRequired'] === 'boolean') { + return Promise.resolve(this.consentConfig_['consentRequired']); + } else { + return this.getConsentRemote_().then(consentInfo => { + const remoteResponse = consentInfo['consentRequired']; + return typeof remoteResponse === 'boolean' ? remoteResponse : true; + }); + } + } + + /** + * Returns a promise that resolve when amp-consent knows + * if the consent is required. + * @return {!Promise} + */ + getConsentRequiredPromiseLegacy_() { let consentRequiredPromise = null; if (this.consentConfig_['promptIfUnknownForGeoGroup']) { const geoGroup = this.consentConfig_['promptIfUnknownForGeoGroup']; diff --git a/extensions/amp-consent/0.1/consent-config.js b/extensions/amp-consent/0.1/consent-config.js index e91a4f786b5bd..15132acf2c877 100644 --- a/extensions/amp-consent/0.1/consent-config.js +++ b/extensions/amp-consent/0.1/consent-config.js @@ -151,6 +151,24 @@ export class ConsentConfig { } } } + + // TODO(micajuineho): delete promptIfUnknownForGeoGroup, once we migrate fully + // Migrate to geoOverride + const group = config['promptIfUnknownForGeoGroup']; + if (typeof group === 'string') { + config['consentRequired'] = false; + config['geoOverride'] = { + [group]: { + 'consentRequired': true, + }, + }; + } else if ( + config['consentRequired'] === undefined && + config['checkConsentHref'] + ) { + config['consentRequired'] = 'remote'; + } + return this.mergeGeoOverride_(config).then(mergedConfig => this.validateMergedGeoOverride_(mergedConfig) ); @@ -192,6 +210,11 @@ export class ConsentConfig { * @return {!JsonObject} */ validateMergedGeoOverride_(mergedConfig) { + userAssert( + mergedConfig['consentRequired'] !== undefined, + '`consentRequired` is required', + TAG + ); if (mergedConfig['consentRequired'] === 'remote') { userAssert( mergedConfig['checkConsentHref'], diff --git a/extensions/amp-consent/0.1/test/test-amp-consent.js b/extensions/amp-consent/0.1/test/test-amp-consent.js index 839a704888412..abb662095267f 100644 --- a/extensions/amp-consent/0.1/test/test-amp-consent.js +++ b/extensions/amp-consent/0.1/test/test-amp-consent.js @@ -54,6 +54,10 @@ describes.realWin( 'https://response1/': '{"promptIfUnknown": true}', 'https://response2/': '{}', 'https://response3/': '{"promptIfUnknown": false}', + 'https://geo-override-check/': '{"consentRequired": false}', + 'https://geo-override-check2/': '{"consentRequired": true}', + 'http://www.origin.com/r/1': '{}', + 'https://invalid.response.com/': '{"consentRequired": 3}', }; xhrServiceMock = { @@ -103,20 +107,16 @@ describes.realWin( let consentElement; it('get consent/policy/postPromptUI config', async () => { - consentElement = createConsentElement( - doc, - dict({ - 'consents': { - 'test': { - 'checkConsentHref': '/override', - }, - }, - 'clientConfig': { - 'test': 'ABC', - }, - 'postPromptUI': 'test', - }) - ); + const config = dict({ + 'consentInstanceId': 'test', + 'checkConsentHref': '/override', + 'consentRequired': true, + 'clientConfig': { + 'test': 'ABC', + }, + 'postPromptUI': 'test', + }); + consentElement = createConsentElement(doc, config); const postPromptUI = document.createElement('div'); postPromptUI.setAttribute('id', 'test'); consentElement.appendChild(postPromptUI); @@ -126,16 +126,7 @@ describes.realWin( expect(ampConsent.postPromptUI_).to.not.be.null; expect(ampConsent.consentId_).to.equal('test'); - expect(ampConsent.consentConfig_).to.deep.equal( - dict({ - 'consentInstanceId': 'test', - 'checkConsentHref': '/override', - 'postPromptUI': 'test', - 'clientConfig': { - 'test': 'ABC', - }, - }) - ); + expect(ampConsent.consentConfig_).to.deep.equal(config); expect(Object.keys(ampConsent.policyConfig_)).to.have.length(4); expect(ampConsent.policyConfig_['default']).to.be.ok; @@ -149,11 +140,8 @@ describes.realWin( consentElement = createConsentElement( doc, dict({ - 'consents': { - 'XYZ': { - 'checkConsentHref': '/r/1', - }, - }, + 'checkConsentHref': '/r/1', + 'consentInstanceId': 'XYZ', }) ); const ampConsent = new AmpConsent(consentElement); @@ -210,6 +198,51 @@ describes.realWin( }); }); + describe('geo-override server communication', () => { + let ampConsent; + beforeEach(() => { + toggleExperiment(win, 'amp-consent-geo-override'); + }); + + it('sends post request to server when consentRequired is remote', async () => { + const remoteConfig = { + 'consentInstanceId': 'abc', + 'consentRequired': 'remote', + 'checkConsentHref': 'https://geo-override-check/', + }; + ampConsent = getAmpConsent(doc, remoteConfig); + await ampConsent.buildCallback(); + await macroTask(); + expect(await ampConsent.getConsentRequiredPromise_()).to.be.false; + }); + + it('resolves consentRequired to remote response with old format', async () => { + const remoteConfig = { + 'consents': { + 'oldConsent': { + 'checkConsentHref': 'https://geo-override-check2/', + }, + }, + }; + ampConsent = getAmpConsent(doc, remoteConfig); + await ampConsent.buildCallback(); + await macroTask(); + expect(await ampConsent.getConsentRequiredPromise_()).to.be.true; + }); + + it('fallsback to true with invalide remote reponse', async () => { + const remoteConfig = { + 'consentInstanceId': 'abc', + 'consentRequired': 'remote', + 'checkConsentHref': 'https://invalid.response.com/', + }; + ampConsent = getAmpConsent(doc, remoteConfig); + await ampConsent.buildCallback(); + await macroTask(); + expect(await ampConsent.getConsentRequiredPromise_()).to.be.true; + }); + }); + describe('amp-geo integration', () => { let defaultConfig; let ampConsent; @@ -230,9 +263,7 @@ describes.realWin( ampConsent = new AmpConsent(consentElement); ISOCountryGroups = ['unknown', 'testGroup']; await ampConsent.buildCallback(); - return ampConsent.getConsentRequiredPromise_().then(isRequired => { - expect(isRequired).to.be.true; - }); + expect(await ampConsent.getConsentRequiredPromise_()).to.be.true; }); it('not in geo group', async () => { @@ -240,9 +271,7 @@ describes.realWin( ampConsent = new AmpConsent(consentElement); ISOCountryGroups = ['unknown']; await ampConsent.buildCallback(); - return ampConsent.getConsentRequiredPromise_().then(isRequired => { - expect(isRequired).to.be.false; - }); + expect(await ampConsent.getConsentRequiredPromise_()).to.be.false; }); it('geo override promptIfUnknown', async () => { @@ -261,9 +290,7 @@ describes.realWin( doc.body.appendChild(consentElement); ampConsent = new AmpConsent(consentElement); await ampConsent.buildCallback(); - return ampConsent.getConsentRequiredPromise_().then(isRequired => { - expect(isRequired).to.be.false; - }); + expect(await ampConsent.getConsentRequiredPromise_()).to.be.false; }); }); @@ -551,3 +578,16 @@ export function createConsentElement(doc, config, opt_type) { consentElement.appendChild(scriptElement); return consentElement; } + +/** + * Create an element and return AmpConsent object. + * @param {Document} doc + * @param {!JsonObject} config + * @param {string=} opt_type + * @return {Element} + */ +export function getAmpConsent(doc, config) { + const consentElement = createConsentElement(doc, config); + doc.body.appendChild(consentElement); + return new AmpConsent(consentElement); +} diff --git a/extensions/amp-consent/0.1/test/test-consent-config.js b/extensions/amp-consent/0.1/test/test-consent-config.js index 9ce29c1c1a9d3..46c429d99d7fc 100644 --- a/extensions/amp-consent/0.1/test/test-consent-config.js +++ b/extensions/amp-consent/0.1/test/test-consent-config.js @@ -54,6 +54,7 @@ describes.realWin('ConsentConfig', {amp: 1}, env => { dict({ 'consentInstanceId': 'ABC', 'checkConsentHref': 'https://response1', + 'consentRequired': 'remote', }) ); }); @@ -69,11 +70,12 @@ describes.realWin('ConsentConfig', {amp: 1}, env => { 'consentInstanceId': '_ping_', 'checkConsentHref': '/get-consent-v1', 'promptUISrc': '/test/manual/diy-consent.html', + 'consentRequired': 'remote', }) ); }); - it('support deprecated config format', () => { + it('converts deprecated format to new format', async () => { appendConfigScriptElement( doc, element, @@ -96,10 +98,15 @@ describes.realWin('ConsentConfig', {amp: 1}, env => { 'postPromptUI': 'test', }) ); + env.sandbox.stub(Services, 'geoForDocOrNull').returns( + Promise.resolve({ + isInCountryGroup() { + return false; + }, + }) + ); const consentConfig = new ConsentConfig(element); - return expect( - consentConfig.getConsentConfigPromise() - ).to.eventually.deep.equal( + expect(await consentConfig.getConsentConfigPromise()).to.deep.equal( dict({ 'consentInstanceId': 'ABC', 'promptIfUnknownForGeoGroup': 'eea', @@ -107,6 +114,7 @@ describes.realWin('ConsentConfig', {amp: 1}, env => { 'clientConfig': { 'test': 'ABC', }, + 'consentRequired': false, 'uiConfig': { 'overlay': true, }, @@ -115,7 +123,55 @@ describes.realWin('ConsentConfig', {amp: 1}, env => { ); }); - it('merge inline config w/ cmp config', () => { + it('converts deprecated format to with consentRequired true', async () => { + appendConfigScriptElement( + doc, + element, + dict({ + 'consents': { + 'ABC': { + 'promptIfUnknownForGeoGroup': 'eea', + 'checkConsentHref': '/href', + 'clientConfig': { + 'test': 'error', + }, + }, + }, + 'clientConfig': { + 'test': 'ABC', + }, + 'uiConfig': { + 'overlay': true, + }, + 'postPromptUI': 'test', + }) + ); + env.sandbox.stub(Services, 'geoForDocOrNull').returns( + Promise.resolve({ + isInCountryGroup() { + return GEO_IN_GROUP.IN; + }, + }) + ); + const consentConfig = new ConsentConfig(element); + expect(await consentConfig.getConsentConfigPromise()).to.deep.equal( + dict({ + 'consentInstanceId': 'ABC', + 'promptIfUnknownForGeoGroup': 'eea', + 'checkConsentHref': '/href', + 'clientConfig': { + 'test': 'ABC', + }, + 'consentRequired': true, + 'uiConfig': { + 'overlay': true, + }, + 'postPromptUI': 'test', + }) + ); + }); + + it('merge inline config w/ cmp config', async () => { appendConfigScriptElement( doc, element, @@ -137,14 +193,20 @@ describes.realWin('ConsentConfig', {amp: 1}, env => { 'postPromptUI': 'test', }) ); + env.sandbox.stub(Services, 'geoForDocOrNull').returns( + Promise.resolve({ + isInCountryGroup() { + return false; + }, + }) + ); element.setAttribute('type', '_ping_'); const consentConfig = new ConsentConfig(element); - return expect( - consentConfig.getConsentConfigPromise() - ).to.eventually.deep.equal( + expect(await consentConfig.getConsentConfigPromise()).to.deep.equal( dict({ 'consentInstanceId': '_ping_', 'checkConsentHref': '/override', + 'consentRequired': false, 'promptUISrc': '/test/manual/diy-consent.html', 'promptIfUnknownForGeoGroup': 'eea', 'postPromptUI': 'test', @@ -277,6 +339,44 @@ describes.realWin('ConsentConfig', {amp: 1}, env => { 'consentRequired': true, }); }); + + it('should have remote consentRequired if checkConsentHref', async () => { + geoConfig = { + 'consentInstanceId': 'abc', + 'checkConsentHref': 'https://example.com/check-consent', + }; + appendConfigScriptElement(doc, element, geoConfig); + const consentConfig = new ConsentConfig(element); + expect(await consentConfig.getConsentConfigPromise()).to.deep.equal({ + 'consentInstanceId': 'abc', + 'checkConsentHref': 'https://example.com/check-consent', + 'consentRequired': 'remote', + }); + }); + + it('should convert promptIfUnknownForGeoGroup', async () => { + geoConfig = { + 'consentInstanceId': 'abc', + 'promptIfUnknownForGeoGroup': 'na', + }; + env.sandbox.stub(Services, 'geoForDocOrNull').returns( + Promise.resolve({ + isInCountryGroup(geoGroup) { + if (geoGroup === 'na') { + return GEO_IN_GROUP.IN; + } + return GEO_IN_GROUP.NOT_IN; + }, + }) + ); + appendConfigScriptElement(doc, element, geoConfig); + const consentConfig = new ConsentConfig(element); + expect(await consentConfig.getConsentConfigPromise()).to.deep.equal({ + 'consentInstanceId': 'abc', + 'consentRequired': true, + 'promptIfUnknownForGeoGroup': 'na', + }); + }); }); it('assert valid config', async () => { @@ -370,23 +470,24 @@ describes.realWin('ConsentConfig', {amp: 1}, env => { ).to.throw(multiScriptError); }); - it('remove not supported policy', () => { + it('remove not supported policy', async () => { toggleExperiment(win, 'multi-consent', false); appendConfigScriptElement( doc, element, dict({ 'consentInstanceId': 'ABC', + 'checkConsentHref': 'example.com/', 'policy': { 'ABC': undefined, }, }) ); const consentConfig = new ConsentConfig(element); - return expect( - consentConfig.getConsentConfigPromise() - ).to.eventually.deep.equal({ + expect(await consentConfig.getConsentConfigPromise()).to.deep.equal({ 'consentInstanceId': 'ABC', + 'checkConsentHref': 'example.com/', + 'consentRequired': 'remote', 'policy': {}, }); }); From 9007d40f3036ee270ba01852c34e2f7aba460bf4 Mon Sep 17 00:00:00 2001 From: Ugo Stephant Date: Mon, 25 Nov 2019 21:56:19 +0100 Subject: [PATCH 003/436] =?UTF-8?q?=F0=9F=90=9Bamp-access-poool:=20Fix=20m?= =?UTF-8?q?issing=20usage=20of=20mutateElement=20(#25761)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(amp-access-poool): correctly mutate element before attributes modifications * fix(amp-access-poool): fix possible type issues --- examples/article-access-poool.amp.html | 7 +++++++ extensions/amp-access-poool/0.1/poool-impl.js | 17 +++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/examples/article-access-poool.amp.html b/examples/article-access-poool.amp.html index ddfbd7978d183..8d56762d45c3d 100644 --- a/examples/article-access-poool.amp.html +++ b/examples/article-access-poool.amp.html @@ -261,6 +261,13 @@

Lorem Ipsum

Nullam in libero nisi.

+ + +

Sed pharetra semper fringilla. Nulla fringilla, neque eget varius suscipit, mi turpis congue odio, quis dignissim nisi diff --git a/extensions/amp-access-poool/0.1/poool-impl.js b/extensions/amp-access-poool/0.1/poool-impl.js index ba756a5f6b132..b38a954a8ff83 100644 --- a/extensions/amp-access-poool/0.1/poool-impl.js +++ b/extensions/amp-access-poool/0.1/poool-impl.js @@ -64,6 +64,9 @@ export class PooolVendor { /** @private {!../../amp-access/0.1/amp-access-source.AccessSource} */ this.accessSource_ = accessSource; + /** @const @private {!../../../src/service/resources-interface.ResourcesInterface} */ + this.resources_ = Services.resourcesForDoc(this.ampdoc); + /** @private {string} */ this.accessUrl_ = ACCESS_CONFIG['authorization']; @@ -211,12 +214,22 @@ export class PooolVendor { const articlePreview = this.ampdoc .getRootNode() .querySelector('[poool-access-preview]'); - articlePreview.setAttribute('amp-access-hide', ''); + + if (articlePreview) { + this.resources_.mutateElement(articlePreview, () => { + articlePreview.setAttribute('amp-access-hide', ''); + }); + } const articleContent = this.ampdoc .getRootNode() .querySelector('[poool-access-content]'); - articleContent.removeAttribute('amp-access-hide'); + + if (articleContent) { + this.resources_.mutateElement(articleContent, () => { + articleContent.removeAttribute('amp-access-hide'); + }); + } resetStyles(this.iframe_, ['transform']); } From 89f394e3eae9ddf7ad924a3e47487c31de75426f Mon Sep 17 00:00:00 2001 From: Angie Lin Date: Mon, 25 Nov 2019 13:01:38 -0800 Subject: [PATCH 004/436] Turn SwG gpay 100% in canary and 10% in prod. (#25762) In both instances, we want 100% native buy flow. --- build-system/global-configs/canary-config.json | 1 + build-system/global-configs/prod-config.json | 1 + 2 files changed, 2 insertions(+) diff --git a/build-system/global-configs/canary-config.json b/build-system/global-configs/canary-config.json index 84d6573feb3eb..eb4f2dedca206 100644 --- a/build-system/global-configs/canary-config.json +++ b/build-system/global-configs/canary-config.json @@ -33,6 +33,7 @@ "fixed-elements-in-lightbox": 1, "flexAdSlots": 0.05, "swg-gpay-api": 1, + "swg-gpay-native": 1, "hidden-mutation-observer": 1, "ios-fixed-no-transfer": 1, "layoutbox-invalidate-on-scroll": 1, diff --git a/build-system/global-configs/prod-config.json b/build-system/global-configs/prod-config.json index 7ba7c68354f39..09f94a67c5a6b 100644 --- a/build-system/global-configs/prod-config.json +++ b/build-system/global-configs/prod-config.json @@ -31,6 +31,7 @@ "fix-inconsistent-responsive-height-selection": 0, "fixed-elements-in-lightbox": 1, "flexAdSlots": 0.05, + "swg-gpay-api": 0.1, "swg-gpay-native": 1, "hidden-mutation-observer": 1, "ios-fixed-no-transfer": 0, From 0859e3853a7f378fd5af07f40cb58c72508fe69d Mon Sep 17 00:00:00 2001 From: Micajuine Ho Date: Mon, 25 Nov 2019 13:12:14 -0800 Subject: [PATCH 005/436] add machtedGeoGroups to request (#25699) * added experiment flag * added configs * changes to prod/canary config * cleaning up * more clean up * merge logic in consent-config * adding in userAssert for consentRequired * Update canary-config.json * Update prod-config.json * Update amp-consent.js * cleaning up * deprecate promptIfUnknownForGeo * consent intialize promise * config merge then validate * check if geoService can't identify * verify consentRequired from endpoint * removed consentState to string * adding comment * bug fixing and geoGroupUnknown * adding const vbs, refactoring, adding matchedgroup * add this.matchedGeoGroups_, & in request * adding in getConsentRequiredPromiseLegacy_() * Update amp-consent.js * Update consent-config.js * requested changes * suggested changes * getMatchedGeoGroups method * getMatchedGeoGroups() in amp-consent * suggested fixes & fixing unit tests * unit tests, better userAssert messages * adding tests * Suggested changes to tests + bug fixes * starting to write tests for promptunknowngeo * unit tests * remove function*, cleaning up * sandbox change * a little more cleaning up * cleaning * cleaning up * cleaning up logic * fixing tests and promises * testing * bug fix * unit test for cr * test for non remote consentRequired * migrating unit tests * Cleaning code, adding thorough test * suggested changes * adding tests * adding tests * nit changes from last pr * suggested changes + tests --- extensions/amp-consent/0.1/amp-consent.js | 24 ++++--- extensions/amp-consent/0.1/consent-config.js | 14 +++- .../amp-consent/0.1/test/test-amp-consent.js | 64 +++++++++++++++++++ 3 files changed, 91 insertions(+), 11 deletions(-) diff --git a/extensions/amp-consent/0.1/amp-consent.js b/extensions/amp-consent/0.1/amp-consent.js index 8140f451e6785..76f4a42703489 100644 --- a/extensions/amp-consent/0.1/amp-consent.js +++ b/extensions/amp-consent/0.1/amp-consent.js @@ -100,6 +100,9 @@ export class AmpConsent extends AMP.BaseElement { /** @private {?string} */ this.consentId_ = null; + + /** @private {?string} */ + this.matchedGeoGroup_ = null; } /** @override */ @@ -115,19 +118,20 @@ export class AmpConsent extends AMP.BaseElement { 'amp-consent should have an id' ); - const config = new ConsentConfig(this.element); + const configManager = new ConsentConfig(this.element); - return config.getConsentConfigPromise().then(validatedConfig => { + return configManager.getConsentConfigPromise().then(validatedConfig => { + this.matchedGeoGroup_ = configManager.getMatchedGeoGroup(); this.initialize_(validatedConfig); }); } /** * - * @param {!JsonObject} config + * @param {!JsonObject} validatedConfig */ - initialize_(config) { - this.consentConfig_ = config; + initialize_(validatedConfig) { + this.consentConfig_ = validatedConfig; // ConsentConfig has verified that there's one and only one consent instance this.consentId_ = this.consentConfig_['consentInstanceId']; @@ -444,12 +448,11 @@ export class AmpConsent extends AMP.BaseElement { } if (typeof this.consentConfig_['consentRequired'] === 'boolean') { return Promise.resolve(this.consentConfig_['consentRequired']); - } else { - return this.getConsentRemote_().then(consentInfo => { - const remoteResponse = consentInfo['consentRequired']; - return typeof remoteResponse === 'boolean' ? remoteResponse : true; - }); } + return this.getConsentRemote_().then(consentInfo => { + const remoteResponse = consentInfo['consentRequired']; + return typeof remoteResponse === 'boolean' ? remoteResponse : true; + }); } /** @@ -546,6 +549,7 @@ export class AmpConsent extends AMP.BaseElement { 'consentStateValue': getConsentStateValue(storedInfo['consentState']), 'consentString': storedInfo['consentString'], 'isDirty': !!storedInfo['isDirty'], + 'matchedGeoGroup': this.matchedGeoGroup_, }); if (this.consentConfig_['clientConfig']) { request['clientConfig'] = this.consentConfig_['clientConfig']; diff --git a/extensions/amp-consent/0.1/consent-config.js b/extensions/amp-consent/0.1/consent-config.js index 15132acf2c877..b1be829c5097d 100644 --- a/extensions/amp-consent/0.1/consent-config.js +++ b/extensions/amp-consent/0.1/consent-config.js @@ -42,6 +42,9 @@ export class ConsentConfig { /** @private {!Window} */ this.win_ = toWin(element.ownerDocument.defaultView); + /** @private {?string} */ + this.matchedGeoGroup_ = null; + /** @private {?Promise} */ this.configPromise_ = null; } @@ -57,6 +60,15 @@ export class ConsentConfig { return this.configPromise_; } + /** + * Returns the matched geoGroup. Call after getConsentConfigPromise + * has resolved. + * @return {?string} + */ + getMatchedGeoGroup() { + return this.matchedGeoGroup_; + } + /** * Convert the inline config to new format * @param {!JsonObject} config @@ -190,12 +202,12 @@ export class ConsentConfig { TAG ); const mergedConfig = map(config); - const geoGroups = Object.keys(config['geoOverride']); // Stop at the first group that the geoService says we're in and then merge configs. for (let i = 0; i < geoGroups.length; i++) { if (geoService.isInCountryGroup(geoGroups[i]) === GEO_IN_GROUP.IN) { deepMerge(mergedConfig, config['geoOverride'][geoGroups[i]], 1); + this.matchedGeoGroup_ = geoGroups[i]; break; } } diff --git a/extensions/amp-consent/0.1/test/test-amp-consent.js b/extensions/amp-consent/0.1/test/test-amp-consent.js index abb662095267f..d94ae19567fe0 100644 --- a/extensions/amp-consent/0.1/test/test-amp-consent.js +++ b/extensions/amp-consent/0.1/test/test-amp-consent.js @@ -186,6 +186,19 @@ describes.realWin( 'consentStateValue': 'unknown', 'consentString': undefined, 'isDirty': false, + 'matchedGeoGroup': null, + }); + }); + + it('send post request to server with no matched group', async () => { + await ampConsent.buildCallback(); + await macroTask(); + expect(requestBody).to.deep.equal({ + 'consentInstanceId': 'ABC', + 'consentStateValue': 'unknown', + 'consentString': undefined, + 'isDirty': false, + 'matchedGeoGroup': null, }); }); @@ -230,6 +243,57 @@ describes.realWin( expect(await ampConsent.getConsentRequiredPromise_()).to.be.true; }); + it('send post request to server with matched group', async () => { + const remoteConfig = { + 'consentInstanceId': 'abc', + 'geoOverride': { + 'nafta': { + 'checkConsentHref': 'https://geo-override-check2/', + 'consentRequired': true, + }, + }, + }; + ISOCountryGroups = ['nafta']; + ampConsent = getAmpConsent(doc, remoteConfig); + await ampConsent.buildCallback(); + await macroTask(); + expect(requestBody).to.deep.equal({ + 'consentInstanceId': 'abc', + 'consentStateValue': 'unknown', + 'consentString': undefined, + 'isDirty': false, + 'matchedGeoGroup': 'nafta', + }); + }); + + it('only geoOverrides the first matched group', async () => { + const remoteConfig = { + 'consentInstanceId': 'abc', + 'geoOverride': { + 'na': { + 'checkConsentHref': 'https://geo-override-check2/', + 'consentRequired': true, + }, + 'eea': { + 'consentRequired': false, + }, + }, + }; + ISOCountryGroups = ['na', 'eea']; + ampConsent = getAmpConsent(doc, remoteConfig); + await ampConsent.buildCallback(); + await macroTask(); + expect(await ampConsent.getConsentRequiredPromise_()).to.be.true; + expect(ampConsent.matchedGeoGroup_).to.equal('na'); + expect(requestBody).to.deep.equal({ + 'consentInstanceId': 'abc', + 'consentStateValue': 'unknown', + 'consentString': undefined, + 'isDirty': false, + 'matchedGeoGroup': 'na', + }); + }); + it('fallsback to true with invalide remote reponse', async () => { const remoteConfig = { 'consentInstanceId': 'abc', From 4698e65f01d139476a0e55027efeb96d8b25c3d9 Mon Sep 17 00:00:00 2001 From: Jack Steinberg Date: Mon, 25 Nov 2019 16:51:25 -0500 Subject: [PATCH 006/436] =?UTF-8?q?=E2=9C=A8Add=20amp-story-quiz=20compone?= =?UTF-8?q?nt=20to=20Stories=20format=20(#25495)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- build-system/compile/bundles.config.js | 7 +- examples/amp-story/quiz.html | 74 ++++++ extensions/amp-story/1.0/amp-story-quiz.css | 162 ++++++++++++ extensions/amp-story/1.0/amp-story-quiz.js | 243 ++++++++++++++++++ extensions/amp-story/1.0/amp-story.js | 2 + extensions/amp-story/1.0/page-advancement.js | 5 +- .../amp-story/1.0/test/test-amp-story-quiz.js | 174 +++++++++++++ 7 files changed, 663 insertions(+), 4 deletions(-) create mode 100644 examples/amp-story/quiz.html create mode 100644 extensions/amp-story/1.0/amp-story-quiz.css create mode 100644 extensions/amp-story/1.0/amp-story-quiz.js create mode 100644 extensions/amp-story/1.0/test/test-amp-story-quiz.js diff --git a/build-system/compile/bundles.config.js b/build-system/compile/bundles.config.js index 5bf6105787067..73429bdf5bc5a 100644 --- a/build-system/compile/bundles.config.js +++ b/build-system/compile/bundles.config.js @@ -879,16 +879,17 @@ exports.extensionBundles = [ hasCss: true, cssBinaries: [ 'amp-story-bookend', - 'amp-story-tooltip', 'amp-story-consent', 'amp-story-draggable-drawer-header', 'amp-story-hint', - 'amp-story-unsupported-browser-layer', - 'amp-story-viewport-warning-layer', 'amp-story-info-dialog', + 'amp-story-quiz', 'amp-story-share', 'amp-story-share-menu', 'amp-story-system-layer', + 'amp-story-tooltip', + 'amp-story-unsupported-browser-layer', + 'amp-story-viewport-warning-layer', ], }, type: TYPES.MISC, diff --git a/examples/amp-story/quiz.html b/examples/amp-story/quiz.html new file mode 100644 index 0000000000000..71844aaa25111 --- /dev/null +++ b/examples/amp-story/quiz.html @@ -0,0 +1,74 @@ + + + + + amp-story-quiz example + + + + + + + + + + + + + + +

Cat Facts!

+ + + + + + + + +

When did the domestic cat first appear on the scene?

+ + + + +
+
+
+ + + + + + +

True or False: Cats can be right-pawed or left-pawed.

+ + + +
+
+
+ + + diff --git a/extensions/amp-story/1.0/amp-story-quiz.css b/extensions/amp-story/1.0/amp-story-quiz.css new file mode 100644 index 0000000000000..cf27365f59c3b --- /dev/null +++ b/extensions/amp-story/1.0/amp-story-quiz.css @@ -0,0 +1,162 @@ +/** + * Copyright 2017 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +@import url('https://fonts.googleapis.com/css?family=Poppins&display=swap'); + +.i-amphtml-story-quiz-container { + align-self: start; + --correct-color: #5BBA74 !important; + --correct-color-shaded: #2D8E42 !important; + --incorrect-color: #F34E4E !important; + --incorrect-color-shaded: #cc4141 !important; + --accent-color: #005AF0; + font-family: 'Poppins-bold', sans-serif !important; + background: var(--accent-color) !important; + border-radius: 32px !important; +} + +.i-amphtml-story-quiz-prompt-container { + display: flex !important; + justify-items: center !important; + padding: 20px !important; + color: white !important; +} + +.i-amphtml-story-quiz-prompt { + margin: 0px !important; + overflow : hidden !important; + text-overflow: ellipsis !important; + display: -webkit-box !important; + -webkit-line-clamp: 4 !important; + -webkit-box-orient: vertical !important; + visibility: visible !important; +} + +h1.i-amphtml-story-quiz-prompt { + font-size: 28px !important; + line-height: 36px !important; + max-height: 144px !important; +} + +h2.i-amphtml-story-quiz-prompt { + font-size: 22px !important; + line-height: 28px !important; + max-height: 112px !important; +} + +h3.i-amphtml-story-quiz-prompt { + font-size: 18px !important; + line-height: 24px !important; + max-height: 96px !important; +} + +.i-amphtml-story-quiz-option-container { + display: flex !important; + flex-direction: column; + background-color: white !important; + border-radius: 32px !important; + padding: 8px 8px 0px 8px !important; +} + +.i-amphtml-story-quiz-option { + display: flex !important; + justify-items: start !important; + align-items: center !important; + border-radius: 32px !important; + padding: 8px 16px 8px 8px !important; + background-color: inherit !important; + margin-bottom: 8px !important; + color: #5F6368 !important; + border: solid 1px #DADCE0 !important; + font-size: 16px !important; + line-height: 20px !important; +} + +/* Truncate option text and add an ellipsis after 2 lines. */ +.i-amphtml-story-quiz-option-text { + max-height: 40px !important; + overflow : hidden !important; + text-overflow: ellipsis !important; + display: -webkit-box !important; + -webkit-line-clamp: 2 !important; + -webkit-box-orient: vertical !important; + visibility: visible !important; +} + +.i-amphtml-story-quiz-post-selection .i-amphtml-story-quiz-option-selected { + color: white !important; + border: solid 1px transparent !important; +} + +.i-amphtml-story-quiz-answer-choice { + display: flex !important; + justify-content: center !important; + align-items: center !important; + height: 24px !important; + width: 24px !important; + min-width: 24px !important; + border-radius: 50% !important; + border: solid 2px !important; + padding: 5px !important; + margin-right: 16px !important; + color: var(--accent-color) !important; + font-size: 16px !important; + line-height: 16px !important; + font-weight: bold !important; +} + +[dir=rtl ] .i-amphtml-story-quiz-answer-choice { + margin-left: 16px !important; + margin-right: 0px !important; +} + +.i-amphtml-story-quiz-post-selection :not([correct]) > .i-amphtml-story-quiz-answer-choice { + color: var(--incorrect-color) !important; +} + +.i-amphtml-story-quiz-post-selection [correct] > .i-amphtml-story-quiz-answer-choice { + color: var(--correct-color) !important; +} + +.i-amphtml-story-quiz-post-selection .i-amphtml-story-quiz-option-selected:not([correct]) { + background-color: var(--incorrect-color) !important; +} + +.i-amphtml-story-quiz-post-selection .i-amphtml-story-quiz-option-selected[correct] { + background-color: var(--correct-color) !important; +} + +.i-amphtml-story-quiz-post-selection .i-amphtml-story-quiz-option-selected > .i-amphtml-story-quiz-answer-choice { + border: solid 2px transparent !important; +} + +/** TODO(jackbsteinberg): Use a more a11y-friendly approach than font-size: 0px; */ + +.i-amphtml-story-quiz-post-selection [correct] > .i-amphtml-story-quiz-answer-choice { + background-color: white !important; + background-image: url('data:image/svg+xml;charset=utf-8,') !important; + background-repeat: no-repeat !important; + background-position: center !important; + font-size: 0 !important; +} + +.i-amphtml-story-quiz-post-selection :not([correct]) > .i-amphtml-story-quiz-answer-choice { + background-color: white !important; + background-image: url('data:image/svg+xml;charset=utf-8,') !important; + background-repeat: no-repeat !important; + background-position: center !important; + font-size: 0 !important; +} diff --git a/extensions/amp-story/1.0/amp-story-quiz.js b/extensions/amp-story/1.0/amp-story-quiz.js new file mode 100644 index 0000000000000..45ea04110ea71 --- /dev/null +++ b/extensions/amp-story/1.0/amp-story-quiz.js @@ -0,0 +1,243 @@ +/** + * Copyright 2019 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {CSS} from '../../../build/amp-story-quiz-1.0.css'; +import {StateProperty, getStoreService} from './amp-story-store-service'; +import {closest} from '../../../src/dom'; +import {createShadowRootWithStyle} from './utils'; +import {dev} from '../../../src/log'; +import {htmlFor} from '../../../src/static-template'; +import {toArray} from '../../../src/types'; + +/** @const {!Array} */ +const answerChoiceOptions = ['A', 'B', 'C', 'D']; + +/** @const {string} */ +const TAG = 'amp-story-quiz'; + +/** + * Generates the template for the quiz + * + * @param {!Element} element + * @return {!Element} + */ +const buildQuizTemplate = element => { + const html = htmlFor(element); + return html` +
+
+
+
+ `; +}; + +/** + * Generates the template for each option + * + * @param {!Element} option + * @return {!Element} + */ +const buildOptionTemplate = option => { + const html = htmlFor(option); + return html` + + + + `; +}; + +export class AmpStoryQuiz extends AMP.BaseElement { + /** + * @param {!AmpElement} element + */ + constructor(element) { + super(element); + + /** @private {boolean} */ + this.hasReceivedResponse_ = false; + + /** @private {?Element} */ + this.quizEl_ = null; + + /** @private @const {!./amp-story-store-service.AmpStoryStoreService} */ + this.storeService_ = getStoreService(this.win); + } + + /** @override */ + buildCallback() { + this.quizEl_ = buildQuizTemplate(this.element); + this.attachContent_(); + this.initializeListeners_(); + createShadowRootWithStyle(this.element, this.quizEl_, CSS); + } + + /** + * Reacts to RTL state updates and triggers the UI for RTL. + * + * @param {boolean} rtlState + * @private + */ + onRtlStateUpdate_(rtlState) { + this.mutateElement(() => { + rtlState + ? this.quizEl_.setAttribute('dir', 'rtl') + : this.quizEl_.removeAttribute('dir'); + }); + } + + /** @override */ + isLayoutSupported(layout) { + // TODO(jackbsteinberg): This selection is temporary and may need to be revisited later + return layout === 'flex-item'; + } + + /** + * @return {?Element} + */ + getQuizElement() { + return this.quizEl_; + } + + /** + * Finds the prompt and options content + * and adds it to the quiz element. + * + * @private + */ + attachContent_() { + // TODO(jackbsteinberg): Optional prompt behavior must be implemented here + const promptInput = this.element.children[0]; + // First child must be heading h1-h3 + if (!['h1', 'h2', 'h3'].includes(promptInput.tagName.toLowerCase())) { + dev().error( + TAG, + 'The first child must be a heading element

,

, or

' + ); + } + + const prompt = document.createElement(promptInput.tagName); + prompt.textContent = promptInput.textContent; + prompt.classList.add('i-amphtml-story-quiz-prompt'); + this.element.removeChild(promptInput); + + const options = toArray(this.element.querySelectorAll('option')); + if (options.length < 2 || options.length > 4) { + dev().error(TAG, 'Improper number of options'); + } + + this.quizEl_ + .querySelector('.i-amphtml-story-quiz-prompt-container') + .appendChild(prompt); + + options.forEach((option, index) => this.configureOption_(option, index)); + + if (this.element.children.length !== 0) { + dev().error(TAG, 'Too many children'); + } + } + + /** + * Creates an option container with option content, + * adds styling and answer choices, + * and adds it to the quiz element. + * + * @param {Element} option + * @param {number} index + * @private + */ + configureOption_(option, index) { + const convertedOption = buildOptionTemplate(dev().assertElement(option)); + + // Fill in the answer choice + convertedOption.querySelector( + '.i-amphtml-story-quiz-answer-choice' + ).textContent = answerChoiceOptions[index]; + + // Transfer the option information into a span then remove the option + const optionText = document.createElement('span'); + optionText.classList.add('i-amphtml-story-quiz-option-text'); + optionText.textContent = option.textContent; + convertedOption.appendChild(optionText); + + if (option.hasAttribute('correct')) { + convertedOption.setAttribute('correct', 'correct'); + } + this.element.removeChild(option); + + // Add the option to the quiz element + this.quizEl_ + .querySelector('.i-amphtml-story-quiz-option-container') + .appendChild(convertedOption); + } + + /** + * Attaches functions to each option to handle state transition. + * + * @private + */ + initializeListeners_() { + // Add a listener for changes in the RTL state + this.storeService_.subscribe( + StateProperty.RTL_STATE, + rtlState => { + this.onRtlStateUpdate_(rtlState); + }, + true /** callToInitialize */ + ); + + // Add a click listener to the element to trigger the class change via tapping the prompt + this.quizEl_.addEventListener('click', e => this.handleTap_(e)); + } + + /** + * Handles a tap event on the quiz element + * + * @param {Event} e + * @private + */ + handleTap_(e) { + if (this.hasReceivedResponse_) { + return; + } + + const optionEl = closest( + dev().assertElement(e.target), + element => { + return element.classList.contains('i-amphtml-story-quiz-option'); + }, + this.quizEl_ + ); + + if (optionEl) { + this.handleOptionSelection_(optionEl); + } + } + + /** + * Triggers changes to quiz state on response interaction + * + * @param {!Element} optionEl + * @private + */ + handleOptionSelection_(optionEl) { + this.mutateElement(() => { + optionEl.classList.add('i-amphtml-story-quiz-option-selected'); + this.quizEl_.classList.add('i-amphtml-story-quiz-post-selection'); + + this.hasReceivedResponse_ = true; + }); + } +} diff --git a/extensions/amp-story/1.0/amp-story.js b/extensions/amp-story/1.0/amp-story.js index 37b4d9c6fbad2..d1edfb1d2d9f8 100644 --- a/extensions/amp-story/1.0/amp-story.js +++ b/extensions/amp-story/1.0/amp-story.js @@ -52,6 +52,7 @@ import {AmpStoryGridLayer} from './amp-story-grid-layer'; import {AmpStoryHint} from './amp-story-hint'; import {AmpStoryPage, NavigationDirection, PageState} from './amp-story-page'; import {AmpStoryPageAttachment} from './amp-story-page-attachment'; +import {AmpStoryQuiz} from './amp-story-quiz'; import {AmpStoryRenderService} from './amp-story-render-service'; import {AnalyticsVariable, getVariableService} from './variable-service'; import {CSS} from '../../../build/amp-story-1.0.css'; @@ -2726,5 +2727,6 @@ AMP.extension('amp-story', '1.0', AMP => { AMP.registerElement('amp-story-grid-layer', AmpStoryGridLayer); AMP.registerElement('amp-story-page', AmpStoryPage); AMP.registerElement('amp-story-page-attachment', AmpStoryPageAttachment); + AMP.registerElement('amp-story-quiz', AmpStoryQuiz); AMP.registerServiceForDoc('amp-story-render', AmpStoryRenderService); }); diff --git a/extensions/amp-story/1.0/page-advancement.js b/extensions/amp-story/1.0/page-advancement.js index 2c05f0d1356d8..2dea5d3cb307b 100644 --- a/extensions/amp-story/1.0/page-advancement.js +++ b/extensions/amp-story/1.0/page-advancement.js @@ -430,7 +430,10 @@ class ManualAdvancement extends AdvancementConfig { dev().assertElement(event.target), el => { tagName = el.tagName.toLowerCase(); - if (tagName === 'amp-story-page-attachment') { + if ( + tagName === 'amp-story-page-attachment' || + tagName === 'amp-story-quiz' + ) { shouldHandleEvent = false; return true; } diff --git a/extensions/amp-story/1.0/test/test-amp-story-quiz.js b/extensions/amp-story/1.0/test/test-amp-story-quiz.js new file mode 100644 index 0000000000000..edd6e124b738f --- /dev/null +++ b/extensions/amp-story/1.0/test/test-amp-story-quiz.js @@ -0,0 +1,174 @@ +/** + * Copyright 2019 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {AmpStoryQuiz} from '../amp-story-quiz'; +import {AmpStoryStoreService} from '../amp-story-store-service'; +import {registerServiceBuilder} from '../../../../src/service'; + +/** + * Populates the quiz with some number of prompts and some number of options + * + * @param {Window} win + * @param {AmpStoryQuiz} quiz + * @param {number} numPrompts + * @param {number} numOptions + */ +const populateQuiz = (win, quizElement, numPrompts = 1, numOptions = 4) => { + for (let i = 0; i < numPrompts; i++) { + const prompt = win.document.createElement('h1'); + prompt.textContent = 'prompt'; + quizElement.appendChild(prompt); + } + + const option = win.document.createElement('option'); + option.textContent = 'option'; + for (let i = 0; i < numOptions; i++) { + quizElement.appendChild(option.cloneNode()); + } + + quizElement.setAttribute('id', 'quizId'); +}; + +/** + * Populates the quiz with a prompt and three options + * + * @param {Window} win + * @param {AmpStoryQuiz} quiz + */ +const populateStandardQuizContent = (win, quizElement) => { + populateQuiz(win, quizElement); +}; + +describes.realWin( + 'amp-story-quiz', + { + amp: true, + }, + env => { + let win; + let ampStoryQuiz; + let storyEl; + + beforeEach(() => { + win = env.win; + const ampStoryQuizEl = win.document.createElement('amp-story-quiz'); + ampStoryQuizEl.getResources = () => win.__AMP_SERVICES.resources.obj; + + const storeService = new AmpStoryStoreService(win); + registerServiceBuilder(win, 'story-store', () => storeService); + + storyEl = win.document.createElement('amp-story'); + const storyPage = win.document.createElement('amp-story-page'); + const gridLayer = win.document.createElement('amp-story-grid-layer'); + gridLayer.appendChild(ampStoryQuizEl); + storyPage.appendChild(gridLayer); + storyEl.appendChild(storyPage); + + win.document.body.appendChild(storyEl); + ampStoryQuiz = new AmpStoryQuiz(ampStoryQuizEl); + + env.sandbox.stub(ampStoryQuiz, 'mutateElement').callsFake(fn => fn()); + }); + + it('should take the html and reformat it', async () => { + populateStandardQuizContent(win, ampStoryQuiz.element); + ampStoryQuiz.buildCallback(); + await ampStoryQuiz.layoutCallback(); + expect(ampStoryQuiz.getQuizElement().children.length).to.equal(2); + }); + + it('should structure the content in the quiz element', async () => { + populateStandardQuizContent(win, ampStoryQuiz.element); + ampStoryQuiz.buildCallback(); + await ampStoryQuiz.layoutCallback(); + + const quizContent = ampStoryQuiz.getQuizElement().children; + expect(quizContent[0]).to.have.class( + 'i-amphtml-story-quiz-prompt-container' + ); + expect(quizContent[1]).to.have.class( + 'i-amphtml-story-quiz-option-container' + ); + + // check prompt container structure + expect(quizContent[0].children.length).to.equal(1); + expect( + quizContent[0].querySelectorAll('.i-amphtml-story-quiz-prompt') + ).to.have.length(1); + + // check option container structure + expect(quizContent[1].childNodes.length).to.equal(4); + expect( + quizContent[1].querySelectorAll('.i-amphtml-story-quiz-option') + ).to.have.length(4); + }); + + it('should throw an error with fewer than one prompt', () => { + populateQuiz(win, ampStoryQuiz.element, 0); + expect(ampStoryQuiz.buildCallback).to.throw(); + }); + + it('should throw an error with more than one prompt', () => { + populateQuiz(win, ampStoryQuiz.element, 2); + expect(ampStoryQuiz.buildCallback).to.throw(); + }); + + it('should throw an error with fewer than two options', () => { + populateQuiz(win, ampStoryQuiz.element, 1, 1); + expect(ampStoryQuiz.buildCallback).to.throw(); + }); + + it('should throw an error with more than four options', () => { + populateQuiz(win, ampStoryQuiz.element, 1, 5); + expect(ampStoryQuiz.buildCallback).to.throw(); + }); + + it('should enter the post-interaction state on option click', async () => { + populateStandardQuizContent(win, ampStoryQuiz.element); + ampStoryQuiz.buildCallback(); + await ampStoryQuiz.layoutCallback(); + const quizElement = ampStoryQuiz.getQuizElement(); + const quizOption = quizElement.querySelector( + '.i-amphtml-story-quiz-option' + ); + + quizOption.click(); + + expect(quizElement).to.have.class('i-amphtml-story-quiz-post-selection'); + expect(quizOption).to.have.class('i-amphtml-story-quiz-option-selected'); + }); + + it('should only record the first option response', async () => { + populateStandardQuizContent(win, ampStoryQuiz.element); + ampStoryQuiz.buildCallback(); + await ampStoryQuiz.layoutCallback(); + const quizElement = ampStoryQuiz.getQuizElement(); + const quizOptions = quizElement.querySelectorAll( + '.i-amphtml-story-quiz-option' + ); + + quizOptions[0].click(); + quizOptions[1].click(); + + expect(quizOptions[0]).to.have.class( + 'i-amphtml-story-quiz-option-selected' + ); + expect(quizOptions[1]).to.not.have.class( + 'i-amphtml-story-quiz-option-selected' + ); + }); + } +); From 6730e6156bc42c8ae29bbc04ac26906bbb6959f2 Mon Sep 17 00:00:00 2001 From: William Chou Date: Mon, 25 Nov 2019 18:04:26 -0500 Subject: [PATCH 007/436] Fix reloading for 'latest' extension versions. (#25763) --- src/service/extension-location.js | 4 +--- src/service/extensions-impl.js | 8 ++++---- test/unit/test-extension-location.js | 12 ++++++++++-- test/unit/test-extensions.js | 22 +++++++++++++++++++++- 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/service/extension-location.js b/src/service/extension-location.js index 712ccd39f9dc6..6d9e1a37855b5 100644 --- a/src/service/extension-location.js +++ b/src/service/extension-location.js @@ -98,9 +98,7 @@ export function calculateEntryPointScriptUrl( * @return {!ExtensionInfoDef} */ export function parseExtensionUrl(scriptUrl) { - const regex = /^(.*)\/(.*)-([0-9.]+)\.js$/i; - const matches = scriptUrl.match(regex); - + const matches = scriptUrl.match(/^(.*)\/(.*)-([0-9.]+|latest)\.js$/i); return { extensionId: matches ? matches[2] : undefined, extensionVersion: matches ? matches[3] : undefined, diff --git a/src/service/extensions-impl.js b/src/service/extensions-impl.js index f6fec78003dbf..d5b8db138ee4a 100644 --- a/src/service/extensions-impl.js +++ b/src/service/extensions-impl.js @@ -271,18 +271,18 @@ export class Extensions { */ getExtensionScript_(extensionId, includeInserted = true) { // Always ignore + + + + +
This is a placeholder
+
This is a fallback
+
+ + + + From 4ebb61b8094f6c9f0da55f5992a3764386fb4805 Mon Sep 17 00:00:00 2001 From: Jake Fried Date: Tue, 26 Nov 2019 15:07:47 -0500 Subject: [PATCH 011/436] AMP Runtime: add support to strip xssi prevention prefixes before converting fetch responses to json (#25489) * AMP Runtime: add support to strip xssi * remove unneeded tests * add in missing jsdoc param * fix linter complaints * match the type definition in ts of actual response.json(), to make closure compiler happy * add user().warn for absent prefixes. * add tagname for user.warn * will nit * fix batched-json tests by adding xhr stub --- extensions/amp-list/0.1/amp-list.js | 1 + .../amp-list/0.1/test/validator-amp-list.html | 6 +++ .../amp-list/0.1/test/validator-amp-list.out | 6 +++ .../amp-list/validator-amp-list.protoascii | 1 + src/batched-json.js | 5 ++- src/service/xhr-impl.js | 28 ++++++++++++- test/unit/test-batched-json.js | 5 +++ test/unit/test-xhr.js | 41 +++++++++++++++++++ 8 files changed, 91 insertions(+), 2 deletions(-) diff --git a/extensions/amp-list/0.1/amp-list.js b/extensions/amp-list/0.1/amp-list.js index cd439266634c0..79894b982369d 100644 --- a/extensions/amp-list/0.1/amp-list.js +++ b/extensions/amp-list/0.1/amp-list.js @@ -1242,6 +1242,7 @@ export class AmpList extends AMP.BaseElement { urlReplacement: this.getPolicy_(), refresh, token, + stripPrefix: this.element.getAttribute('strip-prefix') || undefined, }); } diff --git a/extensions/amp-list/0.1/test/validator-amp-list.html b/extensions/amp-list/0.1/test/validator-amp-list.html index f4d399eda487d..e8e41a4ff742e 100644 --- a/extensions/amp-list/0.1/test/validator-amp-list.html +++ b/extensions/amp-list/0.1/test/validator-amp-list.html @@ -192,5 +192,11 @@
+ + +
+
diff --git a/extensions/amp-list/0.1/test/validator-amp-list.out b/extensions/amp-list/0.1/test/validator-amp-list.out index 0204afdfa7458..b3a51435425fe 100644 --- a/extensions/amp-list/0.1/test/validator-amp-list.out +++ b/extensions/amp-list/0.1/test/validator-amp-list.out @@ -227,5 +227,11 @@ amp-list/0.1/test/validator-amp-list.html:184:2 Attribute 'template' in tag 'amp amp-list/0.1/test/validator-amp-list.html:194:2 The mandatory attribute 'type' is missing in tag 'template'. (see https://amp.dev/documentation/components/amp-mustache) >> ^~~~~~~~~ amp-list/0.1/test/validator-amp-list.html:194:2 The tag 'template' requires including the 'amp-mustache' extension JavaScript. (see https://amp.dev/documentation/components/amp-mustache) +| +| +|
+|
| | \ No newline at end of file diff --git a/extensions/amp-list/validator-amp-list.protoascii b/extensions/amp-list/validator-amp-list.protoascii index 4f9d28f313967..f0e0c4fd2100a 100644 --- a/extensions/amp-list/validator-amp-list.protoascii +++ b/extensions/amp-list/validator-amp-list.protoascii @@ -88,6 +88,7 @@ tags: { # with mandatory src and/or [src] attr value: "fetch" } attrs: { name: "single-item" } + attrs: { name: "strip-prefix" } attrs: { name: "src" mandatory_anyof: "['src','[src]','data-amp-bind-src']" diff --git a/src/batched-json.js b/src/batched-json.js index de7c17883761d..0fd49c3f4bc2b 100644 --- a/src/batched-json.js +++ b/src/batched-json.js @@ -42,6 +42,8 @@ export const UrlReplacementPolicy = { * vars. If OPT_IN, replaces whitelisted URL vars. Otherwise, don't expand. * @param {boolean|undefined} options.refresh Forces refresh of browser cache. * @param {string|undefined} options.token Auth token that forces a POST request. + * @param {string|undefined} options.stripPrefix Prefix to optionally + * strip from the response before calling parseJson. * @return {!Promise>} Resolved with JSON * result or rejected if response is invalid. */ @@ -53,6 +55,7 @@ export function batchFetchJsonFor( urlReplacement = UrlReplacementPolicy.NONE, refresh = false, token = undefined, + stripPrefix = undefined, } = {} ) { assertHttpsUrl(element.getAttribute('src'), element); @@ -70,7 +73,7 @@ export function batchFetchJsonFor( } return xhr.fetchJson(data.xhrUrl, data.fetchOpt); }) - .then(res => res.json()) + .then(res => Services.xhrFor(ampdoc.win).xssiJson(res, stripPrefix)) .then(data => { if (data == null) { throw new Error('Response is undefined.'); diff --git a/src/service/xhr-impl.js b/src/service/xhr-impl.js index 087e9d4bb7269..0228a20179195 100644 --- a/src/service/xhr-impl.js +++ b/src/service/xhr-impl.js @@ -23,10 +23,12 @@ import { setupInput, setupJsonFetchInit, } from '../utils/xhr-utils'; +import {dev, user} from '../log'; import {getCorsUrl, parseUrlDeprecated} from '../url'; import {getService, registerServiceBuilder} from '../service'; import {isFormDataWrapper} from '../form-data-wrapper'; -import {user} from '../log'; +import {parseJson} from '../json'; +import {startsWith} from '../string'; /** * A service that polyfills Fetch API for use within AMP. @@ -147,6 +149,30 @@ export class Xhr { return this.fetch(input, setupInit(opt_init, 'text/plain')); } + /** + * A subsitute for the standard response.json(), which may optionally strip a prefix before calling JSON.parse(). + * + * @param {!Response} res fetch response to convert to json. + * @param {string|undefined} prefix to strip away. + * @return {Promise<*>} + */ + xssiJson(res, prefix) { + if (!prefix) { + return res.json(); + } + + return res.text().then(txt => { + if (!startsWith(txt, dev().assertString(prefix))) { + user().warn( + 'XHR', + `Failed to strip missing prefix "${prefix}" in fetch response.` + ); + return parseJson(txt); + } + return parseJson(txt.slice(prefix.length)); + }); + } + /** * @param {string} input URL * @param {?FetchInitDef=} opt_init Fetch options object. diff --git a/test/unit/test-batched-json.js b/test/unit/test-batched-json.js index 58dd9ecc6bab7..948cb42ef61b9 100644 --- a/test/unit/test-batched-json.js +++ b/test/unit/test-batched-json.js @@ -24,6 +24,7 @@ describe('batchFetchJsonFor', () => { // Service fakes. let urlReplacements; let batchedXhr; + let xhr; // Function stubs. let fetchJson; // Mutable return variables. @@ -54,6 +55,10 @@ describe('batchFetchJsonFor', () => { json: () => Promise.resolve(data), }) ); + + xhr = {xssiJson: () => Promise.resolve(data)}; + window.sandbox.stub(Services, 'xhrFor').returns(xhr); + batchedXhr = {fetchJson}; window.sandbox.stub(Services, 'batchedXhrFor').returns(batchedXhr); }); diff --git a/test/unit/test-xhr.js b/test/unit/test-xhr.js index 2b18c9b98c112..0894a87a3e14e 100644 --- a/test/unit/test-xhr.js +++ b/test/unit/test-xhr.js @@ -1142,4 +1142,45 @@ describe }); }); }); + + describe('#xssiJson', () => { + let xhr; + beforeEach(() => { + xhr = xhrServiceForTesting(polyfillWin); + setupMockXhr(); + }); + + it('should call response.json() if prefix is either missing or the empty string', () => { + xhrCreated.then(mock => mock.respond(200, [], '{a: 1}')); + const response = { + json: () => Promise.resolve(), + text: () => Promise.reject(new Error('should not be called')), + }; + + return Promise.all([ + xhr.xssiJson(response), + xhr.xssiJson(response, ''), + ]); + }); + + it('should not strip characters if the prefix is not present', () => { + xhrCreated.then(mock => mock.respond(200, [], '{"a": 1}')); + return xhr + .fetchJson('/abc') + .then(res => xhr.xssiJson(res, 'while(1)')) + .then(json => { + expect(json).to.be.deep.equal({a: 1}); + }); + }); + + it('should strip prefix from the response text if prefix is present', () => { + xhrCreated.then(mock => mock.respond(200, [], 'while(1){"a": 1}')); + return xhr + .fetchJson('/abc') + .then(res => xhr.xssiJson(res, 'while(1)')) + .then(json => { + expect(json).to.be.deep.equal({a: 1}); + }); + }); + }); }); From 0100982c8e311f4483fb97ce1061136e76ccc66f Mon Sep 17 00:00:00 2001 From: Caroline Liu <10456171+caroqliu@users.noreply.github.com> Date: Tue, 26 Nov 2019 16:45:53 -0500 Subject: [PATCH 012/436] =?UTF-8?q?=20=E2=9C=A8=20suppor?= =?UTF-8?q?t=20`inline`=20attribute=20(#25100)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * First pass at "inline" attr * Update example + bug bash fixes * General cleanup * Replace contenteditable with textarea * Regex matching * Wait for resolve render promise * Remove positive lookahead * Consume Tab event * General cleanup * Focus for SR * Early return in endpoint * Alan smaller fixes * Binding refactor * Add inline case to manual test page * gulp check-types on bindings * Add comment for filter type * Remove inline sample (Should add with e2e tests) * Remove circular import * Unit tests and associated changes * Comments * Fetch dynamic data dependent on email not inline * s/owl/AMP logo * Run `gulp prettify` * Alan comments * Update fetching on endpoint * Move trigger to inline binding * Independently test bindings * Regex modifications * Alan comments * s/sandbox/env.sandbox * Fix getSingleInputOrTextArea * Stub autocomplete for binding tests * Alan comments * Refactor test helper methods * Use src/dom.js createElementWithAttributes * Alan comments * Move `toggleExperiment` to `beforeEach` --- build-system/server/app.js | 14 + build-system/server/autocomplete-test-data.js | 33 + css/ampdoc.css | 8 +- examples/autocomplete.amp.html | 1576 ++++++++++------- .../amp-autocomplete/0.1/amp-autocomplete.css | 6 +- .../amp-autocomplete/0.1/amp-autocomplete.js | 366 ++-- .../0.1/autocomplete-binding-def.js | 91 + .../0.1/autocomplete-binding-inline.js | 204 +++ .../0.1/autocomplete-binding-single.js | 146 ++ .../test/test-amp-autocomplete-bindings.js | 219 +++ .../0.1/test/test-amp-autocomplete-init.js | 14 +- .../0.1/test/test-amp-autocomplete.js | 159 +- .../amp-autocomplete.amp.html | 60 +- 13 files changed, 2075 insertions(+), 821 deletions(-) create mode 100644 build-system/server/autocomplete-test-data.js create mode 100644 extensions/amp-autocomplete/0.1/autocomplete-binding-def.js create mode 100644 extensions/amp-autocomplete/0.1/autocomplete-binding-inline.js create mode 100644 extensions/amp-autocomplete/0.1/autocomplete-binding-single.js create mode 100644 extensions/amp-autocomplete/0.1/test/test-amp-autocomplete-bindings.js diff --git a/build-system/server/app.js b/build-system/server/app.js index b14337c362fa3..4ae9be8da0a1a 100644 --- a/build-system/server/app.js +++ b/build-system/server/app.js @@ -33,6 +33,7 @@ const path = require('path'); const request = require('request'); const upload = require('multer')(); const pc = process; +const autocompleteEmailData = require('./autocomplete-test-data'); const runVideoTestBench = require('./app-video-testbench'); const { recaptchaFrameRequestHandler, @@ -410,6 +411,19 @@ app.use('/form/autocomplete/error', (req, res) => { res(500); }); +app.use('/form/mention/query', (req, res) => { + const query = req.query.q; + if (!query) { + res.json({items: autocompleteEmailData}); + return; + } + const lowerCaseQuery = query.toLowerCase().trim(); + const filtered = autocompleteEmailData.filter(l => + l.toLowerCase().startsWith(lowerCaseQuery) + ); + res.json({items: filtered}); +}); + app.use('/form/verify-search-json/post', (req, res) => { cors.assertCors(req, res, ['POST']); const form = new formidable.IncomingForm(); diff --git a/build-system/server/autocomplete-test-data.js b/build-system/server/autocomplete-test-data.js new file mode 100644 index 0000000000000..bfbf741e85b9c --- /dev/null +++ b/build-system/server/autocomplete-test-data.js @@ -0,0 +1,33 @@ +/** + * Copyright 2019 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const mentionEmails = [ + 'harrypotter@hogwarts.edu', + 'albusdumbledore@hogwarts.edu', + 'voldemort@deatheater.org', + 'severussnape@hogwarts.edu', + 'siriusblack@hogwarts.edu', + 'hermionegranger@hogwarts.edu', + 'ronweasley@hogwarts.edu', + 'dracomalfoy@hogwarts.edu', + 'nevillelongbottom@hogwarts.edu', + 'rubeushagrid@hogwarts.edu', + 'dobby@hogwarts.edu', + 'bellatrixlestrange@deatheater.org', + 'minervamcgonagall@hogwarts.edu', +]; + +module.exports = mentionEmails; diff --git a/css/ampdoc.css b/css/ampdoc.css index b576e6865d604..1eea89a61a7d2 100644 --- a/css/ampdoc.css +++ b/css/ampdoc.css @@ -320,12 +320,14 @@ amp-autocomplete { display: inline-block !important; } -amp-autocomplete > input { - padding: .5rem; /* default-ui-space-large */ - border: 1px solid rgba(0,0,0,.33) /* default-ui-med-gray */ +amp-autocomplete > input, +amp-autocomplete > textarea { + padding: 0.5rem; /* default-ui-space-large */ + border: 1px solid rgba(0, 0, 0, 0.33); /* default-ui-med-gray */ } amp-autocomplete > input, +amp-autocomplete > textarea, .i-amphtml-autocomplete-results { font-size: 1rem; /* default-ui-font-size */ line-height: 1.5rem; /* default-ui-space-large */ diff --git a/examples/autocomplete.amp.html b/examples/autocomplete.amp.html index ee56620f90387..37f5c44f45cd3 100644 --- a/examples/autocomplete.amp.html +++ b/examples/autocomplete.amp.html @@ -1,59 +1,194 @@ - + - - - - AMP Autocomplete Demo - - - - - - - - + + + + + - - + } + input[name='favoriteState'] { + width: 2em; + } + input[type='submit'] { + display: block; + margin-top: 10px; + } + .out-of-stock { + text-align: right; + text-transform: uppercase; + font: 8pt 'Arial', sans-serif; + right: 0; + } + .autocomplete-partial { + font-weight: bold; + } + + #select-example .autocomplete-partial { + color: red; + text-decoration: underline; + } + .wide { + width: 50vw; + } + .autocomplete-selected { + color: blue; + text-decoration: underline; + } + .profile-pic { + width: 30px; + height: 30px; + margin-right: 10px; + margin-left: -5px; + } + .close-friends { + display: flex; + align-items: center; + line-height: 18px; + } + textarea { + padding: 0.5rem; /* default-ui-space-large */ + border: 1px solid rgba(0, 0, 0, 0.33); /* default-ui-med-gray */ + font-size: 1rem; /* default-ui-font-size */ + line-height: 1.5rem; /* default-ui-space-large */ + } + + +

Autocomplete

+

Inline

+
+ +
+ + +
+ +
+ + + +
+ +
+ +
+
+

Hello World

+

Select Event

-

min-characters = 0, partial red underline

+

min-characters = 0, partial red underline

- - + + - -
+ +
-
+

Hello World

@@ -61,749 +196,966 @@

Disable browser autofill

form has no autocomplete attr

-
+
- + - +

form has autocomplete="on"

-
+ -
+
- + - +

form and input have autocomplete="off"

-
+ -
+
- + - +

Attributes

no attributes

- - + + - -
+ +
-
+

min-characters = 0

- - + + - -
+ +
-
+

min-characters = 3

- - + + - -
+ +
-
+

max-entries = 3

- - + + - -
+ +
-
+

max-entries = 10, min-characters = 0

- - + + - -
+ +
-
+

submit-on-enter

- - - - -
- -
-
- -

highlight-user-entry

-
- - + + - -
+ +
-
+
+ + +

highlight-user-entry

+
+ + + + +
+ +

suggest-first

- - + + - -
+ +
-
+

suggest-first and submit-on-enter

- - + + - -
+ +
-
+

items

- - + + - -
+ +
-
+

Filters

substring

- - + + - -
+ +
-
+

prefix

- - + + - -
+ +
-
+

token-prefix

- - + + - -
+ +
-
+

fuzzy

- - + + - -
+ +
-
+

none to inline data

- - - + + + - -
+ +
-
+

none to endpoint

- - + [src]="'/form/autocomplete/query?q=' + (query || '')" + submit-on-enter + > + - -
+ +
-
+

Datasources in form context

Static data

-
-
-
- - - -
- -
- -
-
- -
+ +
+
+ + + +
+ +
+ +
+
+ +

Remote data (with small data)

-
-
- - - -
- -
- -
-
- -

Both data sources provided (should cause warning and display remote data)

-
-
- - -
- -
- -
+ +
+ + + +
+ +
+ +
+
+ +

+ Both data sources provided (should cause warning and display remote data) +

+
+
+ + +
+ +
+ +

Neither data sources provided (should cause warning)

-
-
- - -
- -
- -
+ +
+ + +
+ +
+ +

Dynamic data, changing json path

-
-
- -
-
- -
- -
+ +
+ +
+
+ +
+ +
- - + +

Dynamic data, changing json object

-
-
- -
-
- -
- -
+ +
+ +
+
+ +
+ +
- - + +

Dynamic data, changing json path and object

-
-
- -
-
- -
- -
-
- - +
+
+ +
+
+
+ +
+
+ +

Templates in search context

Plain Text, Submit on enter

-
-
- - -
-
- -
+ +
+ + +
+
+ +

Rich Text, Template Child, Default Value Property

-
-
- - -
-
-