diff --git a/.eslintrc b/.eslintrc index 5ed350dd56c66..6e574219e35b2 100644 --- a/.eslintrc +++ b/.eslintrc @@ -63,7 +63,7 @@ "amphtml-internal/no-is-amp-alt": 2, "amphtml-internal/no-mixed-operators": 2, "amphtml-internal/no-module-exports": 2, - "amphtml-internal/no-non-string-log-args": 1, + "amphtml-internal/no-non-string-log-args": 0, "amphtml-internal/no-spread": 2, "amphtml-internal/no-style-display": 2, "amphtml-internal/no-style-property-setting": 2, diff --git a/3p/README.md b/3p/README.md index d08f395e66f97..ebe391b642a9e 100644 --- a/3p/README.md +++ b/3p/README.md @@ -21,7 +21,7 @@ Examples: Youtube, Vimeo videos; Tweets, Instagrams; comment systems; polls; qui - All JS on container page must be open source and bundled with AMP. - JavaScript loaded into iframe should be reasonable with respect to functionality. - Use the `sandbox` attribute on iframe if possible. -- Provide unit and integration tests. +- Provide unit and [integration tests](#adding-proper-integration-tests). - Embeds that require browser plugins, such as Flash, Java, ActiveX, Silverlight, etc. are disallowed unless necessary. Special review required. We cannot currently see a reason why these should be allowed. ## Ads @@ -34,7 +34,7 @@ Examples: Youtube, Vimeo videos; Tweets, Instagrams; comment systems; polls; qui - Support viewability and other metrics/instrumentation as supplied by AMP (via postMessage API) - Try to keep overall iframe count at one per ad. Explain why more are needed. - Share JS between iframes on the same page. -- Provide unit and integration tests. +- Provide unit and [integration tests](#adding-proper-integration-tests). - Provide test accounts for inclusion in our open source repository for integration tests. The following aren't hard requirements, but are performance optimizations we should strive to incorporate. Please provide a timeline as to when you expect to follow these guidelines: @@ -59,4 +59,11 @@ Review the [ads/README](../ads/README.md) for further details on ad integration. - JavaScript can not be involved with the initiation of font loading. - Font loading gets controlled (but not initiated) by [``](https://github.com/ampproject/amphtml/issues/648). - AMP by default does not allow inclusion of external stylesheets, but it is happy to whitelist URL prefixes of font providers for font inclusion via link tags. These link tags and their fonts must be served via HTTPS. -- If a font provider does referrer based "security" it needs to whitelist the AMP proxy origins before being included in the link tag whitelist. AMP proxy sends the appropriate referrer header such as "https://cdn.ampproject.org" and "https://amp.cloudflare.com". +- If a font provider does referrer based "security" it needs to whitelist the AMP proxy origins before being included in the link tag whitelist. AMP proxy sends the appropriate referrer header such as `https://cdn.ampproject.org` and `https://amp.cloudflare.com`. + +# Adding proper integration tests +You should ensure there are integration tests for your extension. These should be added to the AMP +repo where it makes sense. In some cases this won't be possible because it relies on bringing up +third-party infrastructure. In these cases you should maintain testing for the extension on your +infrastructure against both production AMP and [canary](https://github.com/ampproject/amphtml/blob/master/contributing/release-schedule.md#amp-dev-channel). +Upon any monitored failures, an escalation can be raised in [regular AMP communication channel](https://github.com/ampproject/amphtml/blob/master/CONTRIBUTING.md#discussion-channels). \ No newline at end of file diff --git a/3p/integration.js b/3p/integration.js index 6788e6fa5ac95..15aa9b53c7d1d 100644 --- a/3p/integration.js +++ b/3p/integration.js @@ -112,6 +112,7 @@ import {amoad} from '../ads/amoad'; import {appnexus} from '../ads/appnexus'; import {appvador} from '../ads/appvador'; import {atomx} from '../ads/atomx'; +import {baidu} from '../ads/baidu'; import {bidtellect} from '../ads/bidtellect'; import {brainy} from '../ads/brainy'; import {bringhub} from '../ads/bringhub'; @@ -141,6 +142,7 @@ import {f1h} from '../ads/f1h'; import {felmat} from '../ads/felmat'; import {flite} from '../ads/flite'; import {fluct} from '../ads/fluct'; +import {freewheel} from '../ads/freewheel'; import {fusion} from '../ads/fusion'; import {genieessp} from '../ads/genieessp'; import {giraff} from '../ads/giraff'; @@ -329,6 +331,7 @@ register('amoad', amoad); register('appnexus', appnexus); register('appvador', appvador); register('atomx', atomx); +register('baidu', baidu); register('beopinion', beopinion); register('bidtellect', bidtellect); register('bodymovinanimation', bodymovinanimation); @@ -362,6 +365,7 @@ register('facebook', facebook); register('felmat', felmat); register('flite', flite); register('fluct', fluct); +register('freewheel', freewheel); register('fusion', fusion); register('genieessp', genieessp); register('giraff', giraff); diff --git a/ads/README.md b/ads/README.md index 78e64617c5585..f79b73b5337da 100644 --- a/ads/README.md +++ b/ads/README.md @@ -351,6 +351,7 @@ To speed up the review process, please run `gulp lint` and `gulp check-types`, t ### Other tips +- It's highly recommended to maintain [an integration test outside AMP repo](../3p/README.md#adding-proper-integration-tests). - Please consider implementing the `render-start` and `no-content-available` APIs (see [Available APIs](#available-apis)), which helps AMP to provide user a much better ad loading experience. - [CLA](../CONTRIBUTING.md#contributing-code): for anyone who has trouble to pass the automatic CLA check in a pull request, try to follow the guidelines provided by the CLA Bot. Common mistakes are: 1. Using a different email address in the git commit. diff --git a/ads/_config.js b/ads/_config.js index b225f5a2b9215..f7018b9760a3f 100644 --- a/ads/_config.js +++ b/ads/_config.js @@ -461,6 +461,11 @@ export const adConfig = { ], }, + 'freewheel': { + prefetch: 'https://cdn.stickyadstv.com/prime-time/fw-amp.min.js', + renderStartImplemented: true, + }, + 'fusion': { prefetch: 'https://assets.adtomafusion.net/fusion/latest/fusion-amp.min.js', }, @@ -1068,4 +1073,9 @@ export const adConfig = { ], }, + 'baidu': { + prefetch: 'https://dup.baidustatic.com/js/dm.js', + renderStartImplemented: true, + }, + }; diff --git a/ads/baidu.js b/ads/baidu.js new file mode 100644 index 0000000000000..4af9a0817aa3c --- /dev/null +++ b/ads/baidu.js @@ -0,0 +1,58 @@ +/** + * Copyright 2015 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 { + loadScript, + validateData, +} from '../3p/3p'; + +/** + * @param {!Window} global + * @param {!Object} data + */ +export function baidu(global, data) { + validateData(data, ['cproid']); + + const id = '_' + Math.random().toString(36).slice(2); + const container = global.document.createElement('div'); + container.id = id; + global.document.getElementById('c').appendChild(container); + + global.slotbydup = global.slotbydup || []; + global.slotbydup.push({ + id: data['cproid'], + container: id, + display: 'inlay-fix', + async: true, + }); + + global.addEventListener('message', () => { + global.context.renderStart(); + }); + + loadScript( + global, + 'https://dup.baidustatic.com/js/dm.js', + () => {}, + () => { + // noContentAvailable should be called, + // if parent iframe receives no message. + // setTimeout can work, but it's not that reliable. + // So, only the faliure of JS loading is dealed with for now. + global.context.noContentAvailable(); + } + ); +} diff --git a/ads/baidu.md b/ads/baidu.md new file mode 100644 index 0000000000000..3081d47a3d592 --- /dev/null +++ b/ads/baidu.md @@ -0,0 +1,34 @@ + + +# Baidu + +## Example + +```html + + +``` + +## Configuration + +For additional detials and support, see [baidu ad union website](http://union.baidu.com/product/prod-cpro.html). + +### Required Parameters + +* `data-cproid` - baidu union ad id diff --git a/ads/freewheel.js b/ads/freewheel.js new file mode 100644 index 0000000000000..f08d7dc92b4b8 --- /dev/null +++ b/ads/freewheel.js @@ -0,0 +1,38 @@ +/** + * Copyright 2018 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 {loadScript, validateData} from '../3p/3p'; + +/** + * @param {!Window} global + * @param {!Object} data + */ +export function freewheel(global, data) { + /*eslint "google-camelcase/google-camelcase": 0*/ + global._freewheel_amp = { + data, + }; + + validateData( + data, + ['zone'], + ['zone','gdpr','gdpr_consent','useCMP','zIndex','blurDisplay', + 'timeline','soundButton','defaultMute','onOver','closeAction', + 'errorAction','pauseRatio','label','vastUrlParams'] + ); + + loadScript(global, 'https://cdn.stickyadstv.com/prime-time/fw-amp.min.js'); +} diff --git a/ads/freewheel.md b/ads/freewheel.md new file mode 100644 index 0000000000000..a9c3d0a25308a --- /dev/null +++ b/ads/freewheel.md @@ -0,0 +1,61 @@ + + +# FreeWheel + +## Example + +### Expand-banner + +```html + + +``` + +### FloorAd + +```html + + + + +``` + +## Configuration + +For details on the configuration semantics, please contact the FreeWheel support team : clientsidesdk@freewheel.tv + +Supported parameters: +All parameters are optional, unless otherwise stated +- `data-zone` [required] +- `data-blurDisplay` +- `data-timeline` +- `data-soundButton` +- `data-defaultMute` +- `data-onOver` +- `data-closeAction` +- `data-errorAction` +- `data-pauseRatio` +- `data-label` +- `data-vastUrlParams` +- `data-gdpr` +- `data-gdpr_consent` +- `data-useCMP` \ No newline at end of file diff --git a/ads/inabox/inabox-messaging-host.js b/ads/inabox/inabox-messaging-host.js index 76635c2a4b20c..44b8e5151c7e4 100644 --- a/ads/inabox/inabox-messaging-host.js +++ b/ads/inabox/inabox-messaging-host.js @@ -359,9 +359,14 @@ export class InaboxMessagingHost { * @private */ function canInspectWindow_(win) { + // TODO: this is not reliable. The compiler assumes that property reads are + // side-effect free. The recommended fix is to use goog.reflect.sinkValue + // but since we're not using the closure library I'm not sure how to do this. + // See https://github.com/google/closure-compiler/issues/3156 try { - const unused = !!win.location.href && win['test']; // eslint-disable-line no-unused-vars - return true; + // win['test'] could be truthy but not true the compiler shouldn't be able + // to optimize this check away. + return !!win.location.href && (win['test'] || true); } catch (unusedErr) { // eslint-disable-line no-unused-vars return false; } diff --git a/build-system/amp4test.js b/build-system/amp4test.js index dc81a55c14ce8..103f7d3d1a5b0 100644 --- a/build-system/amp4test.js +++ b/build-system/amp4test.js @@ -96,13 +96,16 @@ const bank = {}; * Deposit a request. An ID has to be specified. Will override previous request * if the same ID already exists. */ -app.use('/request-bank/deposit/:id/', (req, res) => { +app.use('/request-bank/:bid/deposit/:id/', (req, res) => { + if (!bank[req.params.bid]) { + bank[req.params.bid] = {}; + } const key = req.params.id; log('SERVER-LOG [DEPOSIT]: ', key); - if (typeof bank[key] === 'function') { - bank[key](req); + if (typeof bank[req.params.bid][key] === 'function') { + bank[req.params.bid][key](req); } else { - bank[key] = req; + bank[req.params.bid][key] = req; } res.end(); }); @@ -112,10 +115,13 @@ app.use('/request-bank/deposit/:id/', (req, res) => { * return it immediately. Otherwise wait until it gets deposited * The same request cannot be withdrawn twice at the same time. */ -app.use('/request-bank/withdraw/:id/', (req, res) => { +app.use('/request-bank/:bid/withdraw/:id/', (req, res) => { + if (!bank[req.params.bid]) { + bank[req.params.bid] = {}; + } const key = req.params.id; log('SERVER-LOG [WITHDRAW]: ' + key); - const result = bank[key]; + const result = bank[req.params.bid][key]; if (typeof result === 'function') { return res.status(500).send('another client is withdrawing this ID'); } @@ -125,11 +131,83 @@ app.use('/request-bank/withdraw/:id/', (req, res) => { body: result.body, url: result.url, }); - delete bank[key]; + delete bank[req.params.bid][key]; }; if (result) { callback(result); } else { - bank[key] = callback; + bank[req.params.bid][key] = callback; + } +}); + +/** + * Clean up all pending withdraw & deposit requests. + */ +app.use('/request-bank/:bid/teardown/', (req, res) => { + bank[req.params.bid] = {}; + log('SERVER-LOG [TEARDOWN]'); + res.end(); +}); + +/** + * Serves a fake ad for test-amp-ad-fake.js + */ +app.get('/a4a/:bid', (req, res) => { + const sourceOrigin = req.query['__amp_source_origin']; + if (sourceOrigin) { + res.setHeader('AMP-Access-Control-Allow-Source-Origin', sourceOrigin); + } + const {bid} = req.params; + res.send(` + + + + + + + + + + + + +`); }); diff --git a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/index.js b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/index.js index 8592ed7c866af..c34d219539c18 100644 --- a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/index.js +++ b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/index.js @@ -23,6 +23,13 @@ function isRemovableMethod(t, node, names) { }); } +const typeMap = { + 'assertElement': '!Element', + 'assertString': 'string', + 'assertNumber': 'number', + 'assertBoolean': 'boolean', +}; + const removableDevAsserts = [ 'assert', 'fine', @@ -66,16 +73,33 @@ module.exports = function(babel) { // This might not be the case like in assertEnum which we currently // don't remove. const args = path.node.arguments[0]; + const type = typeMap[property.name]; + if (args) { if (parenthesized) { path.replaceWith(t.parenthesizedExpression(args)); path.skip(); - } else { + // If is not an assert type, we won't need to do type annotation. + // If it has no type that we can cast to, then we also won't need to + // do type annotation. + } else if (!property.name.startsWith('assert') || !type) { path.replaceWith(args); + } else { + // Special case null value argument since it's mostly used for + // interface methods with no implementation which will most likely + // get DCE'd by Closure Compiler since they are unused code methods. + if (args.type === 'NullLiteral') { + return; + } else { + path.replaceWith(t.parenthesizedExpression(args)); + // Add a cast annotation to fix type. + path.addComment('leading', `* @type {${type}} `); + } } } else { // This is to resolve right hand side usage of expression where - // no argument is passed in. + // no argument is passed in. This bare undefined value is eventually + // stripped by Closure Compiler. path.replaceWith(t.identifier('undefined')); } }, diff --git a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-boolean/input.js b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-boolean/input.js index 52847fabeeb33..c2249a764890f 100644 --- a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-boolean/input.js +++ b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-boolean/input.js @@ -13,6 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +const falsey = false; +dev().assertBoolean(falsey); dev().assertBoolean(true); let result = dev().assertBoolean(false, 'hello', 'world'); let result2 = dev().assertBoolean(); diff --git a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-boolean/output.js b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-boolean/output.js index 966b0294367ab..c4973511d8718 100644 --- a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-boolean/output.js +++ b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-boolean/output.js @@ -13,6 +13,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -true; -let result = false; +const falsey = false; + +/** @type {boolean} */ +(falsey); + +/** @type {boolean} */ +(true); +let result = +/** @type {boolean} */ +(false); let result2 = undefined; diff --git a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-element/output.js b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-element/output.js index e31898597168e..4b42976cad9c4 100644 --- a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-element/output.js +++ b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-element/output.js @@ -13,6 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -element; -let result = element; + +/** @type {!Element} */ +(element); +let result = +/** @type {!Element} */ +(element); let result2 = undefined; diff --git a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-number/input.js b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-number/input.js index 741bf9eadf545..bea63f0fb7b9e 100644 --- a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-number/input.js +++ b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-number/input.js @@ -13,6 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +let num = 5; +dev().assertNumber(num); dev().assertNumber(1 + 1); let result = dev().assertNumber(3, 'hello', 'world'); let result2 = dev().assertNumber(); diff --git a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-number/output.js b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-number/output.js index a3e49f5a6d095..e3406a937bdad 100644 --- a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-number/output.js +++ b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-number/output.js @@ -13,6 +13,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -1 + 1; -let result = 3; +let num = 5; + +/** @type {number} */ +(num); + +/** @type {number} */ +(1 + 1); +let result = +/** @type {number} */ +(3); let result2 = undefined; diff --git a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-string/input.js b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-string/input.js index aaf5f4d8b6f58..962ec3d2f7a61 100644 --- a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-string/input.js +++ b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-string/input.js @@ -13,6 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +let str = 'foo'; +dev().assertString(str); dev().assertString('hello'); let result = dev().assertString('world'); let result2 = dev().assertString(); +let result3 = dev().assertString(null); diff --git a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-string/output.js b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-string/output.js index eb28d2cc6cffd..90364214d448b 100644 --- a/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-string/output.js +++ b/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-string/output.js @@ -13,6 +13,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -'hello'; -let result = 'world'; +let str = 'foo'; + +/** @type {string} */ +(str); + +/** @type {string} */ +('hello'); +let result = +/** @type {string} */ +('world'); let result2 = undefined; +let result3 = dev().assertString(null); diff --git a/build-system/build.conf.js b/build-system/build.conf.js index 1410d3ffae132..41d34a56486d8 100644 --- a/build-system/build.conf.js +++ b/build-system/build.conf.js @@ -15,6 +15,8 @@ */ const defaultPlugins = [ + require.resolve( + './babel-plugins/babel-plugin-transform-amp-asserts'), require.resolve( './babel-plugins/babel-plugin-transform-parenthesize-expression'), ]; diff --git a/build-system/config.js b/build-system/config.js index 0eb082c4e7651..df38cfa3f1cab 100644 --- a/build-system/config.js +++ b/build-system/config.js @@ -103,6 +103,31 @@ const lintGlobs = [ // To ignore a file / directory, add it to .eslintignore. ]; +/** + * Array of 3p bootstrap urls + * Defined by the following object schema: + * basename: the name of the 3p frame without extension + * max: the path of the readable html + * min: the name of the minimized html + */ +const thirdPartyFrames = [ + { + basename: 'frame', + max: '3p/frame.max.html', + min: 'frame.html', + }, + { + basename: 'nameframe', + max: '3p/nameframe.max.html', + min: 'nameframe.html', + }, + { + basename: 'recaptcha', + max: '3p/recaptcha.max.html', + min: 'recaptcha.html', + }, +]; + /** @const */ module.exports = { testPaths, @@ -113,8 +138,9 @@ module.exports = { unitTestPaths, unitTestOnSaucePaths, integrationTestPaths, - devDashboardTestPaths, lintGlobs, + devDashboardTestPaths, + thirdPartyFrames, jsonGlobs: [ '**/*.json', '!{node_modules,build,dist,dist.3p,dist.tools,' + diff --git a/build-system/eslint-rules/no-module-exports.js b/build-system/eslint-rules/no-module-exports.js index 01cdd77ef9c54..8b5704c1f0880 100644 --- a/build-system/eslint-rules/no-module-exports.js +++ b/build-system/eslint-rules/no-module-exports.js @@ -30,17 +30,10 @@ module.exports = { }, create(context) { - - function isModuleExports(node) { - return node.object.type === 'Identifier' && (/^(module.exports)$/).test(node.object.name) - } - return { - MemberExpression(node) { - if (isModuleExports(node)) { - context.report({ node, messageId: "unexpected"}); - } - } + "MemberExpression[object.name=module][property.name=exports]": function(node) { + context.report({ node, messageId: "unexpected"}); + } }; } }; diff --git a/build-system/eslint-rules/no-non-string-log-args.js b/build-system/eslint-rules/no-non-string-log-args.js index 6dc33ae62b9c8..4dd1624b5b6a0 100644 --- a/build-system/eslint-rules/no-non-string-log-args.js +++ b/build-system/eslint-rules/no-non-string-log-args.js @@ -88,9 +88,9 @@ const selector = transformableMethods.map(method => { module.exports = { - meta: { - fixable: 'code', - }, + //meta: { + //fixable: 'code', + //}, create(context) { const source = context.getSourceCode(); diff --git a/build-system/git.js b/build-system/git.js index 9389b14db406f..43c89e7dacc44 100644 --- a/build-system/git.js +++ b/build-system/git.js @@ -25,11 +25,20 @@ const {getStdout} = require('./exec'); * Returns the branch point of the current branch off of master. * @return {string} */ -exports.gitBranchPoint = function() { +exports.gitBranchPointFromMaster = function() { + return getStdout('git merge-base master HEAD^').trim(); +}; + +/** + * When running on Travis, this returns the branch point of the current branch + * off of master, as merged by Travis. When not running on Travis, falls back to + * gitBranchPointFromMaster. + */ +exports.gitTravisCommitRangeStart = function() { if (process.env.TRAVIS_COMMIT_RANGE) { return process.env.TRAVIS_COMMIT_RANGE.substr(0, 40); } - return getStdout('git merge-base master HEAD^').trim(); + return exports.gitBranchPointFromMaster(); }; /** @@ -38,7 +47,7 @@ exports.gitBranchPoint = function() { * @return {!Array} */ exports.gitDiffNameOnlyMaster = function() { - const branchPoint = exports.gitBranchPoint(); + const branchPoint = exports.gitBranchPointFromMaster(); return getStdout(`git diff --name-only ${branchPoint}`).trim().split('\n'); }; @@ -48,7 +57,7 @@ exports.gitDiffNameOnlyMaster = function() { * @return {string} */ exports.gitDiffStatMaster = function() { - const branchPoint = exports.gitBranchPoint(); + const branchPoint = exports.gitBranchPointFromMaster(); return getStdout(`git -c color.ui=always diff --stat ${branchPoint}`); }; @@ -58,7 +67,7 @@ exports.gitDiffStatMaster = function() { * @return {!Array} */ exports.gitDiffAddedNameOnlyMaster = function() { - const branchPoint = exports.gitBranchPoint(); + const branchPoint = exports.gitBranchPointFromMaster(); return getStdout(`git diff --name-only --diff-filter=ARC ${branchPoint}`) .trim().split('\n'); }; diff --git a/build-system/pr-check.js b/build-system/pr-check.js index b740cc07e2789..8d505c57e38c7 100644 --- a/build-system/pr-check.js +++ b/build-system/pr-check.js @@ -375,7 +375,9 @@ const command = { } if (process.env.TRAVIS) { if (coverage) { - timedExecOrDie(cmd + ' --headless --coverage'); + // TODO(choumx, #19658): --headless disabled for integration tests on + // Travis until Chrome 72. + timedExecOrDie(cmd + ' --coverage'); } else { startSauceConnect(); timedExecOrDie(cmd + ' --saucelabs'); @@ -388,7 +390,9 @@ const command = { runSinglePassCompiledIntegrationTests: function() { timedExecOrDie('rm -R dist'); timedExecOrDie('gulp dist --fortesting --single_pass --pseudo_names'); - timedExecOrDie('gulp test --integration --nobuild --headless ' + // TODO(choumx, #19658): --headless disabled for integration tests on + // Travis until Chrome 72. + timedExecOrDie('gulp test --integration --nobuild ' + '--compiled --single_pass'); timedExecOrDie('rm -R dist'); }, @@ -643,6 +647,7 @@ function main() { } if (buildTargets.has('RUNTIME')) { command.buildRuntimeMinified(/* extensions */ false); + command.runBundleSizeCheck(/* action */ 'pr'); } else { // Skip the bundle-size check to satisfy the required GitHub check. command.runBundleSizeCheck(/* action */ 'skipped'); @@ -661,9 +666,6 @@ function main() { buildTargets.has('BUILD_SYSTEM')) { command.verifyVisualDiffTests(); } - if (buildTargets.has('RUNTIME')) { - command.runBundleSizeCheck(/* action */ 'pr'); - } if (buildTargets.has('VALIDATOR_WEBUI')) { command.buildValidatorWebUI(); } diff --git a/build-system/sauce_connect/start_sauce_connect.sh b/build-system/sauce_connect/start_sauce_connect.sh index 1eea3279b6bd6..59cab42bd0171 100755 --- a/build-system/sauce_connect/start_sauce_connect.sh +++ b/build-system/sauce_connect/start_sauce_connect.sh @@ -20,7 +20,7 @@ CYAN() { echo -e "\033[0;36m$1\033[0m"; } YELLOW() { echo -e "\033[1;33m$1\033[0m"; } -SC_VERSION="sc-4.5.1-linux" +SC_VERSION="sc-4.5.2-linux" DOWNLOAD_URL="https://saucelabs.com/downloads/$SC_VERSION.tar.gz" DOWNLOAD_DIR="sauce_connect" TAR_FILE="$DOWNLOAD_DIR/$SC_VERSION.tar.gz" diff --git a/build-system/tasks/bundle-size.js b/build-system/tasks/bundle-size.js index 3e77978f2fc89..7e0be91577cfc 100644 --- a/build-system/tasks/bundle-size.js +++ b/build-system/tasks/bundle-size.js @@ -26,7 +26,7 @@ const path = require('path'); const requestPost = BBPromise.promisify(require('request').post); const url = require('url'); const {getStdout} = require('../exec'); -const {gitBranchPoint, gitCommitHash} = require('../git'); +const {gitCommitHash, gitTravisCommitRangeStart} = require('../git'); const runtimeFile = './dist/v0.js'; @@ -108,7 +108,7 @@ function isPullRequest() { * @return {string} the `master` ancestor's bundle size. */ async function getAncestorBundleSize() { - const gitBranchPointSha = gitBranchPoint(); + const gitBranchPointSha = gitTravisCommitRangeStart(); const gitBranchPointShortSha = gitBranchPointSha.substring(0, 7); log('Branch point from master is', cyan(gitBranchPointShortSha)); return await octokit.repos.getContents( diff --git a/build-system/tasks/karma.conf.js b/build-system/tasks/karma.conf.js index f0adafca8d9ef..158e2061fd631 100644 --- a/build-system/tasks/karma.conf.js +++ b/build-system/tasks/karma.conf.js @@ -143,7 +143,9 @@ module.exports = { }, Chrome_no_extensions_headless: { base: 'ChromeHeadless', - flags: ['--no-sandbox'].concat(COMMON_CHROME_FLAGS), + // https://developers.google.com/web/updates/2017/04/headless-chrome#frontend + flags: ['--no-sandbox --remote-debugging-port=9222'] + .concat(COMMON_CHROME_FLAGS), }, // SauceLabs configurations. // New configurations can be created here: diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js index bbe6f1e21b7be..5741958b42e0c 100644 --- a/build-system/tasks/presubmit-checks.js +++ b/build-system/tasks/presubmit-checks.js @@ -868,6 +868,7 @@ const forbiddenTermsSrcInclusive = { 'ads/_a4a-config.js', 'build-system/app.js', 'build-system/app-index/template.js', + 'build-system/amp4test.js', 'dist.3p/current/integration.js', 'extensions/amp-iframe/0.1/amp-iframe.js', 'src/config.js', diff --git a/build-system/tasks/runtime-test.js b/build-system/tasks/runtime-test.js index 87f156e48b92b..5581db635eec8 100644 --- a/build-system/tasks/runtime-test.js +++ b/build-system/tasks/runtime-test.js @@ -78,7 +78,6 @@ function getConfig() { 'SL_Chrome', 'SL_Firefox', 'SL_Safari_11', - 'SL_iOS_11', 'SL_Edge_17', ] // TODO(amp-infra): Evaluate and add more platforms here. @@ -86,6 +85,7 @@ function getConfig() { //'SL_Chrome_Android_7', //'SL_Safari_12', //'SL_iOS_12' + //'SL_iOS_11', //'SL_IE_11' : [ // With --saucelabs_lite, a subset of the unit tests are run. diff --git a/build-system/tasks/visual-diff/index.js b/build-system/tasks/visual-diff/index.js index d7ad9d7b4f0cb..f0c1547bd6c09 100644 --- a/build-system/tasks/visual-diff/index.js +++ b/build-system/tasks/visual-diff/index.js @@ -26,7 +26,7 @@ const request = BBPromise.promisify(require('request')); const sleep = require('sleep-promise'); const tryConnect = require('try-net-connect'); const {execOrDie, execScriptAsync} = require('../../exec'); -const {gitBranchName, gitBranchPoint, gitCommitterEmail} = require('../../git'); +const {gitBranchName, gitCommitterEmail, gitTravisCommitRangeStart} = require('../../git'); const {log, verifyCssElements} = require('./helpers'); const {PercyAssetsLoader} = require('./percy-assets-loader'); @@ -97,7 +97,7 @@ function setPercyBranch() { */ function setPercyTargetCommit() { if (process.env.TRAVIS && !argv.master) { - process.env['PERCY_TARGET_COMMIT'] = gitBranchPoint(); + process.env['PERCY_TARGET_COMMIT'] = gitTravisCommitRangeStart(); } } diff --git a/css/amp.css b/css/amp.css index d173d8eae750e..3aaaa8b8a6a8b 100644 --- a/css/amp.css +++ b/css/amp.css @@ -662,14 +662,9 @@ amp-instagram { /** * Analytics & amp-story-auto-ads tags should never be visible. keep them hidden. */ -amp-analytics, amp-story-auto-ads { - /* Fixed to make position independent of page other elements. */ - position: fixed !important; - top: 0 !important; - width: 1px !important; - height: 1px !important; - overflow: hidden !important; - visibility: hidden; +amp-analytics:not(.i-amphtml-element), +amp-story-auto-ads:not(.i-amphtml-element){ + display: none !important; } /** @@ -801,13 +796,13 @@ amp-accordion > section[expanded] > :last-child { } /* load-more elements should be hidden by default */ -amp-list[load-more] > [load-more-loading], -amp-list[load-more] > [load-more-failed], -amp-list[load-more] > [load-more-button], -amp-list[load-more] > [load-more-end], -amp-list[load-more] > [load-more-button] > .i-amphtml-loader +amp-list[load-more] [load-more-loading], +amp-list[load-more] [load-more-failed], +amp-list[load-more] [load-more-button], +amp-list[load-more] [load-more-end], +amp-list[load-more] [load-more-button] > .i-amphtml-loader { - visibility: hidden; + display: none; } /** diff --git a/examples/ads.amp.html b/examples/ads.amp.html index b07dcf2bacc9e..7ed2418a94027 100644 --- a/examples/ads.amp.html +++ b/examples/ads.amp.html @@ -102,6 +102,7 @@ + @@ -132,6 +133,7 @@ + @@ -662,6 +664,12 @@

Atomx

data-id="1234"> +

Baidu

+ + + Fusion data-parameters="age=99&isMobile&gender=male"> +

FreeWheel

+ + +

Geniee SSP

AMP Analytics reportWhen
+ + + + + + + - + + - - - + + + \ No newline at end of file diff --git a/examples/viewer.html b/examples/viewer.html index 73d2015f3cb61..c9a981b2585d0 100644 --- a/examples/viewer.html +++ b/examples/viewer.html @@ -1,5 +1,5 @@ - + Viewer @@ -405,6 +405,8 @@ Viewer.prototype.processRequest_ = function(name, data, awaitResponse) { log(this.id + ': Viewer.prototype.processRequest_', name); switch(name) { + case 'bindReady': + return Promise.resolve(); case 'documentLoaded': return this.documentReady_(); case 'focusin': @@ -462,7 +464,11 @@ // Mocked response. new Response return Promise.resolve({ - "html": "
  • Some list item
  • Some list item
  • Some list item
", + "html": + "
    " + + "
  • " + + "
  • Some list item
  • " + + "
  • Some list item
", "body" : "", "init" : { "headers": { diff --git a/extensions/amp-a4a/0.1/a4a-variable-source.js b/extensions/amp-a4a/0.1/a4a-variable-source.js index b1ee0e0b6dc90..0dc436c877c01 100644 --- a/extensions/amp-a4a/0.1/a4a-variable-source.js +++ b/extensions/amp-a4a/0.1/a4a-variable-source.js @@ -79,9 +79,13 @@ export class A4AVariableSource extends VariableSource { */ constructor(ampdoc, embedWin) { super(ampdoc); + + // Use parent URL replacements service for fallback. + const {documentElement} = ampdoc.win.document; + const urlReplacements = Services.urlReplacementsForDoc(documentElement); + /** @private {VariableSource} global variable source for fallback. */ - this.globalVariableSource_ = Services.urlReplacementsForDoc(ampdoc) - .getVariableSource(); + this.globalVariableSource_ = urlReplacements.getVariableSource(); /** @private {!Window} */ this.win_ = embedWin; diff --git a/extensions/amp-a4a/0.1/real-time-config-manager.js b/extensions/amp-a4a/0.1/real-time-config-manager.js index 6054323e26827..384854b9965d1 100644 --- a/extensions/amp-a4a/0.1/real-time-config-manager.js +++ b/extensions/amp-a4a/0.1/real-time-config-manager.js @@ -136,8 +136,8 @@ export class RealTimeConfigManager { ERROR_TYPE: errorType, HREF: this.win_.location.href, }; - const url = Services.urlReplacementsForDoc(this.ampDoc_).expandUrlSync( - errorReportingUrl, macros, whitelist); + const service = Services.urlReplacementsForDoc(this.a4aElement_.element); + const url = service.expandUrlSync(errorReportingUrl, macros, whitelist); new this.win_.Image().src = url; } @@ -373,7 +373,7 @@ export class RealTimeConfigManager { const urlReplacementStartTime = Date.now(); this.promiseArray_.push(Services.timerFor(this.win_).timeoutPromise( timeoutMillis, - Services.urlReplacementsForDoc(this.ampDoc_).expandUrlAsync( + Services.urlReplacementsForDoc(this.a4aElement_.element).expandUrlAsync( url, macros, whitelist)).then(url => { checkStillCurrent(); timeoutMillis -= (urlReplacementStartTime - Date.now()); diff --git a/extensions/amp-a4a/0.1/test/test-a4a-var-source.js b/extensions/amp-a4a/0.1/test/test-a4a-var-source.js index bf91df4d63dd4..417214043a091 100644 --- a/extensions/amp-a4a/0.1/test/test-a4a-var-source.js +++ b/extensions/amp-a4a/0.1/test/test-a4a-var-source.js @@ -99,4 +99,9 @@ describe('A4AVariableSource', () => { // Access data. undefinedVariable('ACCESS_READER_ID'); undefinedVariable('AUTHDATA'); + + // amp-bind state. + // AMP_STATE() is not scoped to the FIE so this cannot be safely removed + // without refactoring the implementation in url-replacements-impl.js. + undefinedVariable('AMP_STATE'); }); diff --git a/extensions/amp-a4a/0.1/test/test-real-time-config-manager.js b/extensions/amp-a4a/0.1/test/test-real-time-config-manager.js index 1a86afa780a88..c4ac0230ef380 100644 --- a/extensions/amp-a4a/0.1/test/test-real-time-config-manager.js +++ b/extensions/amp-a4a/0.1/test/test-real-time-config-manager.js @@ -45,8 +45,10 @@ describes.realWin('real-time-config-manager', {amp: true}, env => { beforeEach(() => { sandbox = env.sandbox; - // ensures window location == AMP cache passes + + // Ensures window location == AMP cache passes. env.win.AMP_MODE.test = true; + const doc = env.win.document; element = createElementWithAttributes(env.win.document, 'amp-ad', { 'width': '200', @@ -58,6 +60,12 @@ describes.realWin('real-time-config-manager', {amp: true}, env => { fetchJsonStub = sandbox.stub(Xhr.prototype, 'fetchJson'); a4aElement = new AmpA4A(element); + // RealTimeConfigManager uses the UrlReplacements service scoped to the A4A + // (FIE), but for testing stub in the parent service for simplicity. + const urlReplacements = Services.urlReplacementsForDoc(env.ampdoc); + sandbox.stub(Services, 'urlReplacementsForDoc') + .withArgs(a4aElement.element).returns(urlReplacements); + rtc = new RealTimeConfigManager(a4aElement); maybeExecuteRealTimeConfig_ = rtc.maybeExecuteRealTimeConfig.bind(rtc); getCalloutParam_ = rtc.getCalloutParam_.bind(rtc); @@ -765,7 +773,7 @@ describes.realWin('real-time-config-manager', {amp: true}, env => { }); describe('sendErrorMessage', () => { - let imageStub, requestUrl, ampDoc; + let imageStub, requestUrl; let errorType, errorReportingUrl; let imageMock; @@ -776,7 +784,6 @@ describes.realWin('real-time-config-manager', {amp: true}, env => { sandbox.stub(Xhr.prototype, 'fetch'); imageMock = {}; imageStub = sandbox.stub(env.win, 'Image').returns(imageMock); - ampDoc = a4aElement.getAmpDoc(); errorType = RTC_ERROR_ENUM.TIMEOUT; errorReportingUrl = 'https://www.example.com?e=ERROR_TYPE&h=HREF'; @@ -785,12 +792,13 @@ describes.realWin('real-time-config-manager', {amp: true}, env => { ERROR_TYPE: errorType, HREF: env.win.location.href, }; - requestUrl = Services.urlReplacementsForDoc(ampDoc).expandUrlSync( - errorReportingUrl, macros, whitelist); + + requestUrl = Services.urlReplacementsForDoc(a4aElement.element) + .expandUrlSync(errorReportingUrl, macros, whitelist); }); it('should send error message pingback to correct url', () => { - sendErrorMessage(errorType, errorReportingUrl, env.win, ampDoc); + sendErrorMessage(errorType, errorReportingUrl); expect(imageStub).to.be.calledOnce; expect(imageMock.src).to.equal(requestUrl); }); diff --git a/extensions/amp-access/0.1/amp-access-source.js b/extensions/amp-access/0.1/amp-access-source.js index a9d50a37650b8..9d8e12e5bf6c6 100644 --- a/extensions/amp-access/0.1/amp-access-source.js +++ b/extensions/amp-access/0.1/amp-access-source.js @@ -24,7 +24,6 @@ import {Deferred} from '../../../src/utils/promise'; import {Services} from '../../../src/services'; import { assertHttpsUrl, - getSourceOrigin, parseQueryString, } from '../../../src/url'; import {dev, user} from '../../../src/log'; @@ -106,14 +105,8 @@ export class AccessSource { /** @const {!AccessTypeAdapterDef} */ this.adapter_ = this.createAdapter_(configJson); - /** @const @private {!string} */ - this.pubOrigin_ = getSourceOrigin(ampdoc.win.location); - /** @const @private {!../../../src/service/url-replacements-impl.UrlReplacements} */ - this.urlReplacements_ = Services.urlReplacementsForDoc(ampdoc); - - /** @private @const {!../../../src/service/viewer-impl.Viewer} */ - this.viewer_ = Services.viewerForDoc(ampdoc); + this.urlReplacements_ = Services.urlReplacementsForDoc(accessElement); /** @private @const {function(string):Promise} */ this.openLoginDialog_ = openLoginDialog.bind(null, ampdoc); diff --git a/extensions/amp-access/0.1/test/test-amp-access-source.js b/extensions/amp-access/0.1/test/test-amp-access-source.js index cdbcb6e238813..efc094932b88f 100644 --- a/extensions/amp-access/0.1/test/test-amp-access-source.js +++ b/extensions/amp-access/0.1/test/test-amp-access-source.js @@ -36,6 +36,9 @@ describes.fakeWin('AccessSource', { let ampdoc; let element; + // Undefined initialization params for AccessSource (unused in tests so far). + let readerIdFn, scheduleViewFn, onReauthorizeFn; + beforeEach(() => { win = env.win; ampdoc = env.ampdoc; @@ -57,7 +60,8 @@ describes.fakeWin('AccessSource', { }); function expectSourceType(ampdoc, config, type, adapter) { - const source = new AccessSource(ampdoc, config, null); + const source = new AccessSource(ampdoc, config, readerIdFn, scheduleViewFn, + onReauthorizeFn, element); expect(source.type_).to.equal(type); expect(source.adapter_).to.be.instanceOf(adapter); } @@ -72,7 +76,8 @@ describes.fakeWin('AccessSource', { }, }; - const service = new AccessSource(ampdoc, config); + const service = new AccessSource(ampdoc, config, readerIdFn, scheduleViewFn, + onReauthorizeFn, element); expect(service.loginConfig_).to.deep.equal({ 'login1': 'https://acme.com/l1', 'login2': 'https://acme.com/l2', @@ -130,7 +135,8 @@ describes.fakeWin('AccessSource', { type: 'vendor', vendor: 'vendor1', }; - const source = new AccessSource(ampdoc, config); + const source = new AccessSource(ampdoc, config, readerIdFn, scheduleViewFn, + onReauthorizeFn, element); sandbox.stub(source.adapter_, 'getConfig'); source.getAdapterConfig(); expect(source.adapter_.getConfig.called).to.be.true; @@ -182,7 +188,8 @@ describes.fakeWin('AccessSource', { 'login': 'https://acme.com/l', 'authorizationFallbackResponse': {'error': true}, }; - const service = new AccessSource(ampdoc, config); + const service = new AccessSource(ampdoc, config, readerIdFn, scheduleViewFn, + onReauthorizeFn, element); expect(service.authorizationFallbackResponse_).to.deep.equal( {'error': true}); }); @@ -192,7 +199,8 @@ describes.fakeWin('AccessSource', { type: 'vendor', vendor: 'vendor1', }; - const source = new AccessSource(ampdoc, config); + const source = new AccessSource(ampdoc, config, readerIdFn, scheduleViewFn, + onReauthorizeFn, element); const sourceMock = sandbox.mock(source); sourceMock.expects('login_') .withExactArgs('https://url', '') @@ -212,6 +220,9 @@ describes.fakeWin('AccessSource adapter context', { let source; let context; + // Undefined initialization params for AccessSource (unused in tests so far). + let scheduleViewFn, onReauthorizeFn; + beforeEach(() => { win = env.win; ampdoc = env.ampdoc; @@ -233,8 +244,9 @@ describes.fakeWin('AccessSource adapter context', { configElement.textContent = JSON.stringify(config); document.body.appendChild(configElement); - source = new AccessSource(ampdoc, config, - () => Promise.resolve('reader1')); + const readerIdFn = () => Promise.resolve('reader1'); + source = new AccessSource(ampdoc, config, readerIdFn, scheduleViewFn, + onReauthorizeFn, configElement); context = source.adapter_.context_; }); @@ -299,6 +311,9 @@ describes.fakeWin('AccessSource authorization', { let adapterMock; let source; + // Undefined initialization params for AccessSource (unused in tests so far). + let scheduleViewFn, onReauthorizeFn; + beforeEach(() => { win = env.win; ampdoc = env.ampdoc; @@ -313,7 +328,10 @@ describes.fakeWin('AccessSource authorization', { 'pingback': 'https://acme.com/p?rid=READER_ID', 'login': 'https://acme.com/l?rid=READER_ID', }; - source = new AccessSource(ampdoc, config, () => Promise.resolve('reader1')); + const readerIdFn = () => Promise.resolve('reader1'); + source = new AccessSource(ampdoc, config, readerIdFn, scheduleViewFn, + onReauthorizeFn, win.document.documentElement); + const adapter = { getConfig: () => { }, diff --git a/extensions/amp-access/0.1/test/test-amp-access.js b/extensions/amp-access/0.1/test/test-amp-access.js index 721d042e4c0ca..dbe37628f89d7 100644 --- a/extensions/amp-access/0.1/test/test-amp-access.js +++ b/extensions/amp-access/0.1/test/test-amp-access.js @@ -18,13 +18,13 @@ import {AccessClientAdapter} from '../amp-access-client'; import {AccessService} from '../amp-access'; import {AmpEvents} from '../../../../src/amp-events'; import {Observable} from '../../../../src/observable'; +import {Services} from '../../../../src/services'; import {cidServiceForDocForTesting} from '../../../../src/service/cid-impl'; import {installPerformanceService} from '../../../../src/service/performance-impl'; import {toggleExperiment} from '../../../../src/experiments'; - describes.fakeWin('AccessService', { amp: true, location: 'https://pub.com/doc1', @@ -1695,8 +1695,8 @@ describes.fakeWin('AccessService multiple sources', { const authorizationStub = sandbox.stub(sourceBeer, 'runAuthorization').callsFake( () => Promise.resolve()); - const broadcastStub = sandbox.stub(sourceBeer.viewer_, - 'broadcast'); + const viewer = Services.viewerForDoc(ampdoc); + const broadcastStub = sandbox.stub(viewer, 'broadcast'); const sourceBeerMock = sandbox.mock(sourceBeer); sourceBeerMock.expects('openLoginDialog_') .withExactArgs('https://acme.com/l?rid=reader1') @@ -1727,8 +1727,8 @@ describes.fakeWin('AccessService multiple sources', { const authorizationStub = sandbox.stub(sourceDonuts, 'runAuthorization').callsFake( () => Promise.resolve()); - const broadcastStub = sandbox.stub(sourceDonuts.viewer_, - 'broadcast'); + const viewer = Services.viewerForDoc(ampdoc); + const broadcastStub = sandbox.stub(viewer, 'broadcast'); const sourceDonutsMock = sandbox.mock(sourceDonuts); sourceDonutsMock.expects('openLoginDialog_') .withExactArgs('https://acme.com/l?rid=reader1') diff --git a/extensions/amp-action-macro/amp-action-macro.md b/extensions/amp-action-macro/amp-action-macro.md index 63e617da25743..4f2863546cd7f 100644 --- a/extensions/amp-action-macro/amp-action-macro.md +++ b/extensions/amp-action-macro/amp-action-macro.md @@ -29,26 +29,39 @@ limitations under the License. amp-action-macro.amp.html - [TOC] - ## Overview? + +[TOC] + +## Overview + The `amp-action-macro` component allows for the creation of reusable actions. - ## Example - ```html + +## Example + +```html ``` - ```html + +```html
Close all
``` - ## Attributes - ##### id - Used to uniquely identify the action. This is referenced in an action invokation. - ##### action - The action to invoke. Any valid amp action is allowed here. See Actions and events in AMP. - ```html - e.g. + +## Attributes + +##### id + +Used to uniquely identify the action. This is referenced in an action invokation. + +##### action + +The action to invoke. Any valid amp action is allowed here. See [actions and events in AMP](https://www.ampproject.org/docs/interaction_dynamic/amp-actions-and-events). + +e.g. + +```html @@ -56,4 +69,4 @@ The `amp-action-macro` component allows for the creation of reusable actions. id="action1" action="ampList.refresh()"> ... - ``` \ No newline at end of file + ``` diff --git a/extensions/amp-ad-exit/0.1/amp-ad-exit.js b/extensions/amp-ad-exit/0.1/amp-ad-exit.js index b408f874ed042..2d58727f4b58e 100644 --- a/extensions/amp-ad-exit/0.1/amp-ad-exit.js +++ b/extensions/amp-ad-exit/0.1/amp-ad-exit.js @@ -121,7 +121,7 @@ export class AmpAdExit extends AMP.BaseElement { 'CLICK_X': () => event.clientX, 'CLICK_Y': () => event.clientY, }; - const replacements = Services.urlReplacementsForDoc(this.getAmpDoc()); + const replacements = Services.urlReplacementsForDoc(this.element); const whitelist = { 'RANDOM': true, 'CLICK_X': true, diff --git a/extensions/amp-ad-network-adsense-impl/0.1/test/test-amp-ad-network-adsense-impl.js b/extensions/amp-ad-network-adsense-impl/0.1/test/test-amp-ad-network-adsense-impl.js index 7a8335e2a3c07..2ff933b53018c 100644 --- a/extensions/amp-ad-network-adsense-impl/0.1/test/test-amp-ad-network-adsense-impl.js +++ b/extensions/amp-ad-network-adsense-impl/0.1/test/test-amp-ad-network-adsense-impl.js @@ -339,6 +339,12 @@ describes.realWin('amp-ad-network-adsense-impl', { { getUpgradeDelayMs: () => 1, }); + + // Make sure the ad iframe (FIE) has a local URL replacements service. + const urlReplacements = Services.urlReplacementsForDoc(element); + sandbox.stub(Services, 'urlReplacementsForDoc') + .withArgs(a).returns(urlReplacements); + impl.buildCallback(); impl.size_ = {width: 123, height: 456}; impl.onCreativeRender({customElementExtensions: []}); 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 a64d4c1ad0057..d147edfbdaf2d 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 @@ -411,6 +411,12 @@ describes.realWin('amp-ad-network-doubleclick-impl', realWinConfig, env => { { getUpgradeDelayMs: () => 1, }); + + // Make sure the ad iframe (FIE) has a local URL replacements service. + const urlReplacements = Services.urlReplacementsForDoc(element); + sandbox.stub(Services, 'urlReplacementsForDoc') + .withArgs(a).returns(urlReplacements); + impl.buildCallback(); impl.size_ = {width: 123, height: 456}; impl.onCreativeRender({customElementExtensions: []}); diff --git a/extensions/amp-ad/0.1/amp-ad-ui.js b/extensions/amp-ad/0.1/amp-ad-ui.js index ab3a6688f150b..39989de6f14d6 100644 --- a/extensions/amp-ad/0.1/amp-ad-ui.js +++ b/extensions/amp-ad/0.1/amp-ad-ui.js @@ -14,6 +14,7 @@ * limitations under the License. */ +import {ancestorElementsByTag} from '../../../src/dom'; import {getAdContainer} from '../../../src/ad-helper'; export class AmpAdUIHandler { @@ -65,12 +66,37 @@ export class AmpAdUIHandler { * Order: try collapse -> apply provided fallback -> apply default fallback */ applyNoContentUI() { - if (getAdContainer(this.element_) == 'AMP-STICKY-AD') { + if (getAdContainer(this.element_) === 'AMP-STICKY-AD') { // Special case: force collapse sticky-ad if no content. this.baseInstance_./*OK*/collapse(); return; } + if (getAdContainer(this.element_) === 'AMP-FX-FLYING-CARPET') { + + /** + * Special case: Force collapse the ad if it is the, + * only and direct child of a flying carpet. + * Also, this will not handle + * the amp-layout case for now, as it could be + * inefficient. And we have not seen an amp-layout + * used with flying carpet and ads yet. + */ + + const flyingCarpetElements = + ancestorElementsByTag(this.element_, 'amp-fx-flying-carpet'); + const flyingCarpetElement = flyingCarpetElements[0]; + + flyingCarpetElement.getImpl().then(implementation => { + const children = implementation.getChildren(); + + if (children.length === 1 && children[0] === this.element_) { + this.baseInstance_./*OK*/collapse(); + } + }); + return; + } + let attemptCollapsePromise; if (this.containerElement_) { // Collapse the container element if there's one diff --git a/extensions/amp-ad/0.1/test/test-amp-ad-ui.js b/extensions/amp-ad/0.1/test/test-amp-ad-ui.js index ae76290a914fe..6f8cabd9765c6 100644 --- a/extensions/amp-ad/0.1/test/test-amp-ad-ui.js +++ b/extensions/amp-ad/0.1/test/test-amp-ad-ui.js @@ -15,9 +15,11 @@ */ import * as adHelper from '../../../../src/ad-helper'; +import * as dom from '../../../../src/dom'; import {AmpAdUIHandler} from '../amp-ad-ui'; import {BaseElement} from '../../../../src/base-element'; import {createElementWithAttributes} from '../../../../src/dom'; +import {macroTask} from '../../../../testing/yield'; import {setStyles} from '../../../../src/style'; describes.realWin('amp-ad-ui handler', { @@ -43,7 +45,7 @@ describes.realWin('amp-ad-ui handler', { }); describe('applyNoContentUI', () => { - it('should force collapse ad in special container', () => { + it('should force collapse ad in sticky ad container', () => { adContainer = 'AMP-STICKY-AD'; const attemptCollapseSpy = sandbox.spy(adImpl, 'attemptCollapse'); const collapseSpy = sandbox.stub(adImpl, 'collapse').callsFake(() => {}); @@ -52,6 +54,77 @@ describes.realWin('amp-ad-ui handler', { expect(attemptCollapseSpy).to.not.be.called; }); + it('should force collapse ad inside flying carpet, ' + + 'if it is the only and direct child of flying carpet', function* () { + adContainer = 'AMP-FX-FLYING-CARPET'; + const attemptCollapseSpy = sandbox.spy(adImpl, 'attemptCollapse'); + const collapseSpy = sandbox.stub(adImpl, 'collapse').callsFake(() => {}); + + sandbox.stub(dom, 'ancestorElementsByTag').callsFake(() => { + return [{ + getImpl: () => Promise.resolve({ + getChildren: () => [adElement], + }), + }]; + }); + + uiHandler.applyNoContentUI(); + yield macroTask(); + + expect(collapseSpy).to.be.calledOnce; + expect(attemptCollapseSpy).to.not.be.called; + }); + + it('should NOT force collapse ad inside flying carpet, ' + + 'if there is another element', function* () { + adContainer = 'AMP-FX-FLYING-CARPET'; + const attemptCollapseSpy = sandbox.spy(adImpl, 'attemptCollapse'); + const collapseSpy = sandbox.stub(adImpl, 'collapse').callsFake(() => {}); + + const otherElement = env.win.document.createElement('div'); + + sandbox.stub(dom, 'ancestorElementsByTag').callsFake(() => { + return [{ + getImpl: () => Promise.resolve({ + getChildren: () => [adElement, otherElement], + }), + }]; + }); + + uiHandler.applyNoContentUI(); + yield macroTask(); + + expect(collapseSpy).to.not.be.called; + expect(attemptCollapseSpy).to.not.be.called; + }); + + it('should NOT force collapse ad inside flying carpet, ' + + 'if it is not a direct child of flying carpet.', function* () { + adContainer = 'AMP-FX-FLYING-CARPET'; + const attemptCollapseSpy = sandbox.spy(adImpl, 'attemptCollapse'); + const collapseSpy = sandbox.stub(adImpl, 'collapse').callsFake(() => {}); + + const otherElement = env.win.document.createElement('div'); + adElement.remove(); + otherElement.appendChild(adElement); + + sandbox.stub(dom, 'ancestorElementsByTag').callsFake(() => { + return [{ + getImpl: () => Promise.resolve({ + getChildren: () => [otherElement], + }), + }]; + }); + + uiHandler.applyNoContentUI(); + yield macroTask(); + + expect(collapseSpy).to.not.be.called; + expect(attemptCollapseSpy).to.not.be.called; + }); + + + it('should collapse ad amp-layout container if there is one', () => { adElement = createElementWithAttributes(env.win.document, 'amp-ad', { 'data-ad-container-id': 'test', diff --git a/extensions/amp-ad/amp-ad.md b/extensions/amp-ad/amp-ad.md index f9f11e55d48a9..fa1cd6fab3f06 100644 --- a/extensions/amp-ad/amp-ad.md +++ b/extensions/amp-ad/amp-ad.md @@ -231,6 +231,7 @@ See [amp-ad rules](https://github.com/ampproject/amphtml/blob/master/extensions/ - [AppNexus](../../ads/appnexus.md) - [AppVador](../../ads/appvador.md) - [Atomx](../../ads/atomx.md) +- [Baidu](../../ads/baidu.md) - [BeOpinion](../amp-beopinion/amp-beopinion.md) - [Bidtellect](../../ads/bidtellect.md) - [brainy](../../ads/brainy.md) @@ -258,6 +259,7 @@ See [amp-ad rules](https://github.com/ampproject/amphtml/blob/master/extensions/ - [FlexOneHARRIER](../../ads/f1h.md) - [Flite](../../ads/flite.md) - [fluct](../../ads/fluct.md) +- [FreeWheel](../../ads/freewheel.md) - [Fusion](../../ads/fusion.md) - [GenieeSSP](../../ads/genieessp.md) - [Giraff](../../ads/giraff.md) diff --git a/extensions/amp-analytics/0.1/amp-analytics.css b/extensions/amp-analytics/0.1/amp-analytics.css new file mode 100644 index 0000000000000..5c351cf241160 --- /dev/null +++ b/extensions/amp-analytics/0.1/amp-analytics.css @@ -0,0 +1,36 @@ +/** + * Copyright 2018 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. + */ + +/** + * Analytics tags should never be visible. keep them hidden. + */ +amp-analytics { + /* Fixed to make position independent of page other elements. */ + position: fixed !important; + top: 0 !important; + left: 0 !important; + width: 1px !important; + height: 1px !important; + overflow: hidden !important; + visibility: hidden; +} + +html.i-amphtml-fie > amp-analytics { + /* Remove position fixed if it's in iframe. + * So runtime can measure layoutBox correctly + */ + position: initial !important; +} diff --git a/extensions/amp-analytics/0.1/amp-analytics.js b/extensions/amp-analytics/0.1/amp-analytics.js index b7e3108e03719..1e389e254d86f 100644 --- a/extensions/amp-analytics/0.1/amp-analytics.js +++ b/extensions/amp-analytics/0.1/amp-analytics.js @@ -42,6 +42,7 @@ import {getMode} from '../../../src/mode'; import {installLinkerReaderService} from './linker-reader'; import {isArray, isEnumValue} from '../../../src/types'; import {isIframed} from '../../../src/dom'; +import {isInFie} from '../../../src/friendly-iframe-embed'; import {toggle} from '../../../src/style'; const TAG = 'amp-analytics'; @@ -114,7 +115,7 @@ export class AmpAnalytics extends AMP.BaseElement { /** @override */ isAlwaysFixed() { - return true; + return !isInFie(this.element); } /** @override */ @@ -486,7 +487,7 @@ export class AmpAnalytics extends AMP.BaseElement { initializeLinker_() { const type = this.element.getAttribute('type'); this.linkerManager_ = new LinkerManager(this.getAmpDoc(), - this.config_, type); + this.config_, type, this.element); this.linkerManager_.init(); } @@ -570,7 +571,7 @@ export class AmpAnalytics extends AMP.BaseElement { } const expansionOptions = this.expansionOptions_(event, trigger); expandPostMessage(this.getAmpDoc(), msg, this.config_['extraUrlParams'], - trigger, expansionOptions) + trigger, expansionOptions, this.element) .then(message => { if (isIframed(this.win)) { // Only post message with explict `parentPostMessage` to inabox host diff --git a/extensions/amp-analytics/0.1/linker-manager.js b/extensions/amp-analytics/0.1/linker-manager.js index b2d3fc85137f7..60c4c7607c77b 100644 --- a/extensions/amp-analytics/0.1/linker-manager.js +++ b/extensions/amp-analytics/0.1/linker-manager.js @@ -18,7 +18,7 @@ import {ExpansionOptions, variableServiceFor} from './variables'; import {Priority} from '../../../src/service/navigation'; import {Services} from '../../../src/services'; import {WindowInterface} from '../../../src/window-interface'; -import {addParamToUrl} from '../../../src/url'; +import {addMissingParamsToUrl, addParamToUrl} from '../../../src/url'; import {createElementWithAttributes} from '../../../src/dom'; import {createLinker} from './linker'; import {dict} from '../../../src/utils/object'; @@ -38,30 +38,34 @@ export class LinkerManager { * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc * @param {!JsonObject} config * @param {?string} type + * @param {!Element} element */ - constructor(ampdoc, config, type) { - /** @private {!../../../src/service/ampdoc-impl.AmpDoc} */ + constructor(ampdoc, config, type, element) { + /** @const @private {!../../../src/service/ampdoc-impl.AmpDoc} */ this.ampdoc_ = ampdoc; /** @private {?JsonObject|undefined} */ this.config_ = config['linkers']; - /** @private {!JsonObject} */ + /** @const @private {!JsonObject} */ this.vars_ = config['vars'] || {}; - /** @private {?string} */ + /** @const @private {?string} */ this.type_ = type; + /** @const @private {!Element} */ + this.element_ = element; + /** @private {!Array} */ this.allLinkerPromises_ = []; - /** @private {!JsonObject} */ + /** @const @private {!JsonObject} */ this.resolvedIds_ = dict(); - /** @private {!../../../src/service/url-impl.Url} */ + /** @const @private {!../../../src/service/url-impl.Url} */ this.urlService_ = Services.urlForDoc(this.ampdoc_); - /** @private {Promise<../../amp-form/0.1/form-submit-service.FormSubmitService>} */ + /** @const @private {Promise<../../amp-form/0.1/form-submit-service.FormSubmitService>} */ this.formSubmitService_ = Services.formSubmitPromiseForDoc(ampdoc); /** @private {?UnlistenDef} */ @@ -156,8 +160,8 @@ export class LinkerManager { Object.assign({}, defaultConfig, config[name]); if (mergedConfig['enabled'] !== true) { - user().info(TAG, `linker config for ${name} is not enabled and ` + - 'will be ignored.'); + user().info(TAG, 'linker config for %s is not enabled and ' + + 'will be ignored.', name); return; } @@ -186,8 +190,10 @@ export class LinkerManager { expandTemplateWithUrlParams_(template, expansionOptions) { return variableServiceFor(this.ampdoc_.win) .expandTemplate(template, expansionOptions) - .then(expanded => Services.urlReplacementsForDoc( - this.ampdoc_).expandUrlAsync(expanded)); + .then(expanded => { + const urlReplacements = Services.urlReplacementsForDoc(this.element_); + return urlReplacements.expandUrlAsync(expanded); + }); } @@ -259,7 +265,9 @@ export class LinkerManager { const linkerValue = createLinker(/* version */ '1', this.resolvedIds_[name]); if (linkerValue) { - el.href = addParamToUrl(href, name, linkerValue); + const params = dict(); + params[name] = linkerValue; + el.href = addMissingParamsToUrl(href, params); } } } @@ -284,6 +292,13 @@ export class LinkerManager { // If no domains given, default to friendly domain matching. if (!domains) { + // Don't append linker for exact domain match, relative urls, or + // fragments. + const winHostname = WindowInterface.getHostname(this.ampdoc_.win); + if (winHostname === hostname) { + return false; + } + const {sourceUrl, canonicalUrl} = Services.documentInfoForDoc(this.ampdoc_); const sourceOrigin = this.urlService_.parse(sourceUrl).hostname; diff --git a/extensions/amp-analytics/0.1/requests.js b/extensions/amp-analytics/0.1/requests.js index dfa0a0b3678e8..c83a32bd07586 100644 --- a/extensions/amp-analytics/0.1/requests.js +++ b/extensions/amp-analytics/0.1/requests.js @@ -135,7 +135,8 @@ export class RequestHandler { const params = Object.assign({}, configParams, trigger['extraUrlParams']); const timestamp = this.win.Date.now(); const batchSegmentPromise = expandExtraUrlParams( - this.ampdoc_, params, expansionOption, bindings, this.whiteList_) + this.variableService_, this.urlReplacementService_, params, + expansionOption, bindings, this.whiteList_) .then(params => { return dict({ 'trigger': trigger['on'], @@ -299,12 +300,14 @@ export class RequestHandler { * @param {?JsonObject} configParams * @param {!JsonObject} trigger * @param {!./variables.ExpansionOptions} expansionOption + * @param {!Element} element * @return {Promise} */ export function expandPostMessage( - ampdoc, msg, configParams, trigger, expansionOption) { + ampdoc, msg, configParams, trigger, expansionOption, element) +{ const variableService = variableServiceFor(ampdoc.win); - const urlReplacementService = Services.urlReplacementsForDoc(ampdoc); + const urlReplacementService = Services.urlReplacementsForDoc(element); const bindings = variableService.getMacros(); expansionOption.freezeVar('extraUrlParams'); @@ -321,7 +324,8 @@ export function expandPostMessage( return basePromise.then(expandedMsg => { const params = Object.assign({}, configParams, trigger['extraUrlParams']); //return base url with the appended extra url params; - return expandExtraUrlParams(ampdoc, params, expansionOption, bindings) + return expandExtraUrlParams(variableService, urlReplacementService, params, + expansionOption, bindings) .then(extraUrlParams => { return defaultSerializer(expandedMsg, [ dict({'extraUrlParams': extraUrlParams}), @@ -332,7 +336,8 @@ export function expandPostMessage( /** * Function that handler extraUrlParams from config and trigger. - * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc + * @param {!./variables.VariableService} variableService + * @param {!../../../src/service/url-replacements-impl.UrlReplacements} urlReplacements * @param {!Object} params * @param {!./variables.ExpansionOptions} expansionOption * @param {!Object} bindings @@ -340,11 +345,9 @@ export function expandPostMessage( * @return {!Promise} * @private */ -function expandExtraUrlParams( - ampdoc, params, expansionOption, bindings, opt_whitelist) { - const variableService = variableServiceFor(ampdoc.win); - const urlReplacements = Services.urlReplacementsForDoc(ampdoc); - +function expandExtraUrlParams(variableService, urlReplacements, params, + expansionOption, bindings, opt_whitelist) +{ const requestPromises = []; // Don't encode param values here, // as we'll do it later in the getExtraUrlParamsString call. diff --git a/extensions/amp-analytics/0.1/test/test-linker-manager.js b/extensions/amp-analytics/0.1/test/test-linker-manager.js index 977844aa75d12..96453d6d799c7 100644 --- a/extensions/amp-analytics/0.1/test/test-linker-manager.js +++ b/extensions/amp-analytics/0.1/test/test-linker-manager.js @@ -33,6 +33,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { let doc; let windowInterface; let handlers; + let element; let beforeSubmitStub; beforeEach(() => { @@ -53,6 +54,13 @@ describes.realWin('Linker Manager', {amp: true}, env => { canonicalUrl: 'https://www.canonical.com/some/path?q=123', }); + // LinkerManager uses the UrlReplacements service scoped to the element, + // but for testing stub in the top-level ampdoc service for simplicity. + element = {}; + const urlReplacements = Services.urlReplacementsForDoc(ampdoc); + sandbox.stub(Services, 'urlReplacementsForDoc') + .withArgs(element).returns(urlReplacements); + handlers = []; sandbox.stub(Services, 'navigationForDoc').returns({ registerAnchorMutator: (callback, priority) => { @@ -64,6 +72,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { windowInterface.getLocation.returns({ origin: 'https://amp-source-com.cdn.ampproject.org', }); + windowInterface.getHostname.returns('amp-source-com.cdn.ampproject.org'); installVariableService(win); }); @@ -77,18 +86,18 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }, }, - }, null).init(); + }, /* type */ null, element).init(); expect(handlers.length).to.equal(1); }); it('does not register anchor mutator if no linkers config', () => { - new LinkerManager(ampdoc, {}, null).init(); + new LinkerManager(ampdoc, {}, /* type */ null, element).init(); expect(handlers.length).to.equal(0); }); it('does not register anchor mutator if empty linkers config', () => { - new LinkerManager(ampdoc, {linkers: {}}, null).init(); + new LinkerManager(ampdoc, {linkers: {}}, /* type */ null, element).init(); expect(handlers.length).to.equal(0); }); @@ -106,7 +115,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }, }, - }, null).init(); + }, /* type */ null, element).init(); expect(handlers.length).to.equal(0); }); @@ -123,7 +132,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }, }, - }, null).init(); + }, /* type */ null, element).init(); expect(handlers.length).to.equal(0); }); @@ -141,7 +150,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }, }, - }, null).init(); + }, /* type */ null, element).init(); expect(handlers.length).to.equal(1); }); @@ -173,7 +182,8 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }; - return new LinkerManager(ampdoc, config, null).init().then(() => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { expect(handlers.length).to.equal(1); expect(clickAnchor('https://www.source.com/dest?a=1')).to.equal( 'https://www.source.com/dest' + @@ -196,7 +206,8 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }; - return new LinkerManager(ampdoc, config, null).init().then(() => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { expect(handlers.length).to.equal(1); expect(clickAnchor('https://www.source.com/dest?a=1')).to.equal( 'https://www.source.com/dest?a=1' @@ -218,7 +229,8 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }; - return new LinkerManager(ampdoc, config, null).init().then(() => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { clock.tick(1000 * 60 * 5); // 5 minutes. const linkerUrl = clickAnchor('https://www.source.com/dest?a=1'); @@ -249,14 +261,15 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }; - return new LinkerManager(ampdoc, config, null).init().then(() => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { // testLinker1 should apply to both canonical and source // testLinker2 should not const canonicalDomainUrl = clickAnchor('https://www.canonical.com/path'); expect(canonicalDomainUrl).to.contain('testLinker1='); expect(canonicalDomainUrl).to.not.contain('testLinker2='); - const sourceDomainUrl = clickAnchor('https://amp.source.com/path'); + const sourceDomainUrl = clickAnchor('https://www.source.com/path'); expect(sourceDomainUrl).to.contain('testLinker1='); expect(sourceDomainUrl).to.not.contain('testLinker2='); @@ -291,7 +304,8 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }; - return new LinkerManager(ampdoc, config, null).init().then(() => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { const fooDomainUrl = clickAnchor('https://foo.com/path'); const barDomainUrl = clickAnchor('https://bar.com/path'); @@ -314,7 +328,8 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }; - return new LinkerManager(ampdoc, config, null).init().then(() => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { const url1 = clickAnchor('https://www.source.com/path'); const url2 = clickAnchor('https://amp.www.source.com/path'); const url3 = clickAnchor('https://canonical.com/path'); @@ -351,7 +366,8 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }; - return new LinkerManager(ampdoc, config, null).init().then(() => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { const a = clickAnchor('https://www.source.com/path'); expect(a).to.contain('testLinker1='); expect(a).to.not.contain('testLinker2='); @@ -379,15 +395,121 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }; - return new LinkerManager(ampdoc, config).init().then(() => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { const a = clickAnchor('https://www.source.com'); expect(a).to.not.contain('testLinker1='); expect(a).to.contain('testLinker2='); }); }); - describe('when CID API enabled', () => { + it('should only add the linker param once', () => { + const config = { + linkers: { + enabled: true, + destinationDomains: ['foo.com'], + testLinker1: { + ids: { + id: '111', + }, + }, + }, + }; + + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { + const fooDomainUrl = clickAnchor('https://foo.com/path'); + expect(fooDomainUrl).to.contain('testLinker1='); + + const fooDomainUrlSecondClick = clickAnchor('https://foo.com/path'); + const splitResult = fooDomainUrlSecondClick.split('testLinker1'); + expect(splitResult.length).to.be.equal(2); + }); + }); + + describe('same domain matching', () => { + let config; + + beforeEach(() => { + windowInterface.getLocation.returns({ + origin: 'https://amp.source.com', + }); + windowInterface.getHostname.returns('amp.source.com'); + config = { + linkers: { + testLinker: { + enabled: true, + proxyOnly: false, + ids: { + foo: 'bar', + }, + }, + }, + }; + }); + it('should not add linker if same domain', () => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { + const url = clickAnchor('https://amp.source.com/'); + expect(url).to.not.contain('testLinker'); + }); + }); + + it('should add linker if subdomain is different but friendly', () => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { + const url = clickAnchor('https://m.source.com/'); + expect(url).to.contain('testLinker'); + }); + }); + + it('should add linker if same domain is in destination domains', () => { + const config = { + linkers: { + testLinker: { + enabled: true, + proxyOnly: false, + ids: { + foo: 'bar', + }, + destinationDomains: ['amp.source.com'], + }, + }, + }; + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { + const url = clickAnchor('https://amp.source.com/'); + expect(url).to.contain('testLinker'); + }); + }); + + it('should not add linker if href is fragment', () => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { + const a = { + href: '#hello', + hostname: 'amp.source.com', + }; + handlers.forEach(handler => handler(a, {type: 'click'})); + expect(a.href).to.not.contain('testLinker'); + }); + }); + + it('should not add linker if href is relative', () => { + const lm = new LinkerManager(ampdoc, config, /* type */ null, element); + return lm.init().then(() => { + const a = { + href: '/foo', + hostname: 'amp.source.com', + }; + handlers.forEach(handler => handler(a, {type: 'click'})); + expect(a.href).to.not.contain('testLinker'); + }); + }); + }); + + describe('when CID API enabled', () => { beforeEach(() => { addCidApiMeta(); }); @@ -404,12 +526,11 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }, }; - - return new LinkerManager(ampdoc, config, 'googleanalytics') - .init().then(() => { - const a = clickAnchor('https://www.source.com/path'); - expect(a).to.contain('testLinker1='); - }); + const lm = new LinkerManager(ampdoc, config, 'googleanalytics', element); + return lm.init().then(() => { + const a = clickAnchor('https://www.source.com/path'); + expect(a).to.contain('testLinker1='); + }); }); it('should only add one linker for auto opt-in', () => { @@ -424,8 +545,10 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }, }; - const p1 = new LinkerManager(ampdoc, config, 'googleanalytics').init(); - const p2 = new LinkerManager(ampdoc, config, 'googleanalytics').init(); + const p1 = + new LinkerManager(ampdoc, config, 'googleanalytics', element).init(); + const p2 = + new LinkerManager(ampdoc, config, 'googleanalytics', element).init(); return Promise.all([p1, p2]).then(() => { const a = clickAnchor('https://www.source.com/path'); expect(a).to.not.match(/(testLinker1=.*){2}/); @@ -445,7 +568,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }; - new LinkerManager(ampdoc, config, 'somevendor'); + new LinkerManager(ampdoc, config, 'somevendor', element); expect(handlers.length).to.equal(0); }); @@ -461,7 +584,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }; - new LinkerManager(ampdoc, config, 'googleanalytics'); + new LinkerManager(ampdoc, config, 'googleanalytics', element); expect(handlers.length).to.equal(0); }); @@ -493,7 +616,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }; - new LinkerManager(ampdoc, config, 'googleanalytics'); + new LinkerManager(ampdoc, config, 'googleanalytics', element); expect(handlers.length).to.equal(0); }); }); @@ -534,7 +657,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }, }, - }, null); + }, /* type */ null, element); return linkerManager.init().then(() => { expect(beforeSubmitStub.calledOnce).to.be.true; @@ -554,7 +677,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, destinationDomains: ['www.ampproject.com'], }, - }, null); + }, /* type */ null, element); return linkerManager.init().then(() => { const form = createForm(); @@ -584,7 +707,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, destinationDomains: ['www.ampproject.com'], }, - }, null); + }, /* type */ null, element); return linkerManager.init().then(() => { const form = createForm(); @@ -613,7 +736,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, destinationDomains: ['www.ampproject.com'], }, - }, null); + }, /* type */ null, element); return linkerManager.init().then(() => { const form = createForm(); @@ -643,7 +766,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, destinationDomains: ['www.ampproject.com'], }, - }, null); + }, /* type */ null, element); return linkerManager.init().then(() => { const form = createForm(); @@ -670,7 +793,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }, }, - }, null); + }, /* type */ null, element); const manager2 = new LinkerManager(ampdoc, { linkers: { @@ -682,7 +805,7 @@ describes.realWin('Linker Manager', {amp: true}, env => { }, }, }, - }, null); + }, /* type */ null, element); const p1 = manager1.init(); const p2 = manager2.init(); diff --git a/extensions/amp-analytics/0.1/test/test-requests.js b/extensions/amp-analytics/0.1/test/test-requests.js index 252ca35c39189..67a6e62251b40 100644 --- a/extensions/amp-analytics/0.1/test/test-requests.js +++ b/extensions/amp-analytics/0.1/test/test-requests.js @@ -385,6 +385,8 @@ describes.realWin('Requests', {amp: 1}, env => { describe('expandPostMessage', () => { let expansionOptions; let params; + let element; + beforeEach(() => { expansionOptions = new ExpansionOptions({ 'teste1': 'TESTE1', @@ -393,6 +395,9 @@ describes.realWin('Requests', {amp: 1}, env => { 'e1': '${teste1}', 'e2': 'teste2', }; + // expandPostMessage() uses the URL replacements service scoped to the + // passed element. Use the top-level service for testing. + element = env.win.document.documentElement; }); it('should expand', () => { @@ -401,7 +406,8 @@ describes.realWin('Requests', {amp: 1}, env => { 'test foo 123 ... ${teste1}', undefined, {}, - expansionOptions).then(msg => { + expansionOptions, + element).then(msg => { expect(msg).to.equal('test foo 123 ... TESTE1'); }); }); @@ -412,13 +418,15 @@ describes.realWin('Requests', {amp: 1}, env => { 'test ${extraUrlParams} foo', params, /* configParams */ {}, /* trigger */ - expansionOptions); + expansionOptions, + element); const appendPromise = expandPostMessage( ampdoc, 'test foo', params, /* configParams */ {}, /* trigger */ - expansionOptions); + expansionOptions, + element); return replacePromise.then(replace => { expect(replace).to.equal('test e1=TESTE1&e2=teste2 foo'); expect(appendPromise).to.eventually.equal('test foo'); diff --git a/extensions/amp-analytics/0.1/test/test-visibility-manager.js b/extensions/amp-analytics/0.1/test/test-visibility-manager.js index 83efc3f55d57a..3b3eecfb53693 100644 --- a/extensions/amp-analytics/0.1/test/test-visibility-manager.js +++ b/extensions/amp-analytics/0.1/test/test-visibility-manager.js @@ -1084,6 +1084,237 @@ describes.realWin('VisibilityManager integrated', {amp: true}, env => { }); }); + it('should wait for readyPromise with readyReportPromise', async() => { + viewer.setVisibilityState_(VisibilityState.VISIBLE); + visibility = new VisibilityManagerForDoc(ampdoc); + + visibility.listenElement(ampElement, {}, readyPromise, () => + readyReportPromise, eventResolver); + const model = visibility.models_[0]; + + await Promise.resolve(); + expect(isModelResolved(model)).to.be.false; + + clock.tick(20); + fireIntersect(25); // visible + clock.tick(30); + + readyReportResolver(); + await Promise.resolve(); + expect(isModelResolved(model)).to.be.false; + + clock.tick(35); + + readyResolver(); + await Promise.resolve(); + await Promise.resolve(); // Wait again for handles to execute. + expect(isModelResolved(model)).to.be.true; + + clock.tick(40); // Not counted since model is resolved. + + const state = await eventPromise; + expect(state).to.contains({ + backgrounded: 0, + backgroundedAtStart: 0, + elementHeight: 100, + elementWidth: 100, + elementX: 0, + elementY: 75, + firstSeenTime: 85, + lastSeenTime: 85, + lastVisibleTime: 85, + loadTimeVisibility: 25, + maxVisiblePercentage: 25, + minVisiblePercentage: 25, + totalVisibleTime: 0, + maxContinuousVisibleTime: 0, + }); + }); + + it('should wait for readyReportPromise with reportWhen', async() => { + viewer.setVisibilityState_(VisibilityState.VISIBLE); + visibility = new VisibilityManagerForDoc(ampdoc); + + visibility.listenElement(ampElement, {reportWhen: 'documentExit'}, + readyPromise, () => readyReportPromise, eventResolver); + const model = visibility.models_[0]; + + await Promise.resolve(); + expect(isModelResolved(model)).to.be.false; + + clock.tick(20); + fireIntersect(25); // visible + clock.tick(30); + + readyResolver(); + await Promise.resolve(); + expect(isModelResolved(model)).to.be.false; + + clock.tick(40); + + readyReportResolver(); + await Promise.resolve(); + expect(isModelResolved(model)).to.be.true; + + const state = await eventPromise; + expect(state).to.contains({ + backgrounded: 0, + backgroundedAtStart: 0, + elementHeight: 100, + elementWidth: 100, + elementX: 0, + elementY: 75, + firstSeenTime: 50, + lastSeenTime: 90, + lastVisibleTime: 90, + loadTimeVisibility: 25, + maxVisiblePercentage: 25, + minVisiblePercentage: 25, + totalVisibleTime: 40, + maxContinuousVisibleTime: 40, + }); + }); + + it('should wait for readyReportPromise with reportWhen and never meets ' + + 'visiblePercentageMin', async() => { + viewer.setVisibilityState_(VisibilityState.VISIBLE); + visibility = new VisibilityManagerForDoc(ampdoc); + + visibility.listenElement(ampElement, { + reportWhen: 'documentExit', + visiblePercentageMin: 50, + }, + readyPromise, () => readyReportPromise, eventResolver); + const model = visibility.models_[0]; + + await Promise.resolve(); + expect(isModelResolved(model)).to.be.false; + + clock.tick(20); + fireIntersect(25); // Doesn't meet visiblePercentageMin. + clock.tick(30); + + readyResolver(); + await Promise.resolve(); + expect(isModelResolved(model)).to.be.false; + + clock.tick(40); + + readyReportResolver(); + await Promise.resolve(); + expect(isModelResolved(model)).to.be.true; + + const state = await eventPromise; + expect(state).to.contains({ + backgrounded: 0, + backgroundedAtStart: 0, + elementHeight: 100, + elementWidth: 100, + elementX: 0, + elementY: 75, + firstSeenTime: 50, + lastSeenTime: 90, + lastVisibleTime: 0, // Didn't meet visibility. + loadTimeVisibility: 25, + // FIXME: max/minVisiblePercentage should equal loadTimeVisibility. + // See https://github.com/ampproject/amphtml/issues/19567 + maxVisiblePercentage: 0, + minVisiblePercentage: 0, + totalVisibleTime: 0, + maxContinuousVisibleTime: 0, + }); + }); + + it('should accumulate timings and wait for readyReportPromise with ' + + 'reportWhen and high minTotalVisibleTime', async() => { + viewer.setVisibilityState_(VisibilityState.VISIBLE); + visibility = new VisibilityManagerForDoc(ampdoc); + + visibility.listenElement(ampElement, { + reportWhen: 'documentExit', + minTotalVisibleTime: 100000, // Never met + }, readyPromise, () => readyReportPromise, eventResolver); + const model = visibility.models_[0]; + + await Promise.resolve(); + expect(isModelResolved(model)).to.be.false; + + readyResolver(); + await Promise.resolve(); + expect(isModelResolved(model)).to.be.false; + + clock.tick(20); + fireIntersect(25); // visible + clock.tick(20); + fireIntersect(0); // hidden + clock.tick(20); + fireIntersect(35); // visible again + clock.tick(30); + fireIntersect(0); // hidden + clock.tick(20); + + readyReportResolver(); + await Promise.resolve(); + expect(isModelResolved(model)).to.be.true; + + const state = await eventPromise; + expect(state).to.contains({ + backgrounded: 0, + backgroundedAtStart: 0, + elementHeight: 100, + elementWidth: 100, + elementX: 0, + elementY: 100, + firstSeenTime: 20, + lastSeenTime: 60, + lastVisibleTime: 90, + loadTimeVisibility: 25, + maxVisiblePercentage: 35, + minVisiblePercentage: 25, + totalVisibleTime: 50, + maxContinuousVisibleTime: 30, + }); + }); + + it('should wait for readyReportPromise when missing readyPromise', + async() => { + viewer.setVisibilityState_(VisibilityState.VISIBLE); + visibility = new VisibilityManagerForDoc(ampdoc); + + visibility.listenElement(ampElement, {}, null, () => + readyReportPromise, eventResolver); + const model = visibility.models_[0]; + + clock.tick(20); + fireIntersect(25); // visible + clock.tick(30); + + await Promise.resolve(); + expect(isModelResolved(model)).to.be.false; + + readyReportResolver(); + await Promise.resolve(); + expect(isModelResolved(model)).to.be.true; + + const state = await eventPromise; + expect(state).to.contains({ + backgrounded: 0, + backgroundedAtStart: 0, + elementHeight: 100, + elementWidth: 100, + elementX: 0, + elementY: 75, + firstSeenTime: 20, + lastSeenTime: 50, + lastVisibleTime: 50, + loadTimeVisibility: 25, + maxVisiblePercentage: 25, + minVisiblePercentage: 25, + totalVisibleTime: 30, + maxContinuousVisibleTime: 30, + }); + }); + it('should execute "visible" trigger with percent range', () => { viewer.setVisibilityState_(VisibilityState.VISIBLE); visibility = new VisibilityManagerForDoc(ampdoc); diff --git a/extensions/amp-analytics/0.1/test/test-visibility-model.js b/extensions/amp-analytics/0.1/test/test-visibility-model.js index ab3106ff7e9ad..82a302d2525f7 100644 --- a/extensions/amp-analytics/0.1/test/test-visibility-model.js +++ b/extensions/amp-analytics/0.1/test/test-visibility-model.js @@ -24,6 +24,13 @@ describes.sandboxed('VisibilityModel', {}, () => { let startTime; let clock; + const tick = async(timeout = 0) => { + // Wait for the micro-task queue to clear since we need to wait for + // internal promises to finish before running assertions; + await Promise.resolve(); + clock.tick(timeout); + }; + beforeEach(() => { clock = sandbox.useFakeTimers(); startTime = 10000; @@ -164,8 +171,7 @@ describes.sandboxed('VisibilityModel', {}, () => { }); }); - describe('structure', () => { - let visibility; + describe('structure', () => { let visibility; let calcVisibility; beforeEach(() => { @@ -459,14 +465,14 @@ describes.sandboxed('VisibilityModel', {}, () => { reportPromise = new Promise(resolve => { promiseResolver = resolve; }); - vh.setReportReady(() => {return reportPromise;}); + vh.setReportReady(() => reportPromise); }); it('conditions met, send when report ready', () => { - visibilityValueForTesting = 0.5; - vh.update_(1); + visibilityValueForTesting = 1; + vh.update(); clock.tick(20); - vh.update_(1); + vh.update(); expect(eventSpy).to.not.be.called; promiseResolver(); return reportPromise.then(() => { @@ -476,9 +482,9 @@ describes.sandboxed('VisibilityModel', {}, () => { it('conditions met, but no longer met when ready to report', () => { visibilityValueForTesting = 1; - vh.update_(1); + vh.update(); clock.tick(20); - vh.update_(1); + vh.update(); eventSpy.resetHistory(); expect(eventSpy).to.not.be.called; clock.tick(1001); @@ -487,6 +493,46 @@ describes.sandboxed('VisibilityModel', {}, () => { expect(eventSpy).to.not.be.called; }); }); + + describe('with reportWhen', () => { + beforeEach(() => { + visibilityValueForTesting = 0; + }); + + const shouldTriggerEventTestSpecs = [ + {reportWhen: 'documentExit'}, + {reportWhen: 'documentHidden'}, + {reportWhen: 'documentExit', totalTimeMin: 100000, + visiblePercentageMin: 50}, + ]; + + for (const i in shouldTriggerEventTestSpecs) { + it('should trigger event with reportWhen,' + + `test case #${i}`, async() => { + const vh = new VisibilityModel( + shouldTriggerEventTestSpecs[i], () => 0); + + vh.onTriggerEvent(eventSpy); + // TODO(warrengm): Inverting the two following lines will break this + // test. Consider updating the API to make it safer. + vh.setReportReady(() => reportPromise); + vh.setReady(true); + + vh.update(); + await tick(); + expect(eventSpy).to.not.be.called; + + promiseResolver(); + await tick(); + expect(eventSpy).to.be.calledOnce; + + // Subsequent calls should not trigger the event again. + vh.update(); + await tick(); + expect(eventSpy).to.be.calledOnce; + }); + } + }); }); it('conditions met, wait to reset', () => { diff --git a/extensions/amp-analytics/0.1/test/vendor-requests.json b/extensions/amp-analytics/0.1/test/vendor-requests.json index b76b9f620e42b..dff96b77f3879 100644 --- a/extensions/amp-analytics/0.1/test/vendor-requests.json +++ b/extensions/amp-analytics/0.1/test/vendor-requests.json @@ -303,6 +303,12 @@ "pageview": "https://srv.pixel.parsely.com/plogger/?rand=_timestamp_&idsite=!apikey&url=_ampdoc_url_&urlref=_document_referrer_&screen=_screen_width_x_screen_height_%7C_available_screen_width_x_available_screen_height_%7C_screen_color_depth_&title=_title_&date=_timestamp_&id=_client_id_&action=pageview", "heartbeat": "https://srv.pixel.parsely.com/plogger/?rand=_timestamp_&idsite=!apikey&url=_ampdoc_url_&urlref=_document_referrer_&screen=_screen_width_x_screen_height_%7C_available_screen_width_x_available_screen_height_%7C_screen_color_depth_&title=_title_&date=_timestamp_&id=_client_id_&action=heartbeat&tt=_total_engaged_time_&inc=_incremental_engaged_time_" }, + "permutive": { + "track": "https://!namespace.amp.permutive.com/track?k=!key&i=_client_id_&it=amp&_ep_client.type=amp&_ep_client.title=_title_&_ep_client.domain=_canonical_host_&_ep_client.url=_canonical_url_&_ep_client.referrer=_document_referrer_&_ep_client.user_agent=_user_agent_", + "pageview": "https://!namespace.amp.permutive.com/track?k=!key&i=_client_id_&it=amp&e=Pageview&_ep_isp_info=%24ip_isp_info&_ep_geo_info=%24ip_geo_info&_ep_client.type=amp&_ep_client.title=_title_&_ep_client.domain=_canonical_host_&_ep_client.url=_canonical_url_&_ep_client.referrer=_document_referrer_&_ep_client.user_agent=_user_agent_", + "engagement": "https://!namespace.amp.permutive.com/track?k=!key&i=_client_id_&it=amp&e=PageviewEngagement&_ep_engaged_time=5&_ep_client.type=amp&_ep_client.title=_title_&_ep_client.domain=_canonical_host_&_ep_client.url=_canonical_url_&_ep_client.referrer=_document_referrer_&_ep_client.user_agent=_user_agent_", + "completion": "https://!namespace.amp.permutive.com/track?k=!key&i=_client_id_&it=amp&e=PageviewEngagement&_ep_completion=0.25&_ep_client.type=amp&_ep_client.title=_title_&_ep_client.domain=_canonical_host_&_ep_client.url=_canonical_url_&_ep_client.referrer=_document_referrer_&_ep_client.user_agent=_user_agent_" + }, "piStats": { "host": "https://events.pi-stats.com", "basePrefix": "https://events.pi-stats.com/eventsamp/?e=PageLoad&pid=!property&url=_ampdoc_url_&cnt=!cntId&lang=!language&ref=_document_referrer_&id=_client_id_&ua=_user_agent_&ctype=web&blang=_browser_language_&v=2.0&dist=Javascript", diff --git a/extensions/amp-analytics/0.1/vendors.js b/extensions/amp-analytics/0.1/vendors.js index 369d5b381f8fc..7491f2c57802f 100644 --- a/extensions/amp-analytics/0.1/vendors.js +++ b/extensions/amp-analytics/0.1/vendors.js @@ -60,6 +60,7 @@ import { import {OEWADIRECT_CONFIG} from './vendors/oewadirect'; import {OEWA_CONFIG} from './vendors/oewa'; import {PARSELY_CONFIG} from './vendors/parsely'; +import {PERMUTIVE_CONFIG} from './vendors/permutive'; import {PIANO_CONFIG} from './vendors/piano'; import {PISTATS_CONFIG} from './vendors/piStats'; import {PRESSBOARD_CONFIG} from './vendors/pressboard'; @@ -214,6 +215,7 @@ export const ANALYTICS_CONFIG = /** @type {!JsonObject} */ ({ 'oracleInfinityAnalytics': ORACLEINFINITYANALYTICS_CONFIG, 'parsely': PARSELY_CONFIG, 'piStats': PISTATS_CONFIG, + 'permutive': PERMUTIVE_CONFIG, 'piano': PIANO_CONFIG, 'pressboard': PRESSBOARD_CONFIG, 'quantcast': QUANTCAST_CONFIG, diff --git a/extensions/amp-analytics/0.1/vendors/permutive.js b/extensions/amp-analytics/0.1/vendors/permutive.js new file mode 100644 index 0000000000000..482a0a03243b0 --- /dev/null +++ b/extensions/amp-analytics/0.1/vendors/permutive.js @@ -0,0 +1,75 @@ +/** + * Copyright 2018 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. + */ + +export const PERMUTIVE_CONFIG = /** @type {!JsonObject} */ ({ + 'vars': { + 'identity': '${clientId(_ga)}', + }, + 'requests': { + 'track': 'https://${namespace}.amp.permutive.com/track' + + '?k=${key}' + + '&i=${identity}' + + '&it=amp', + 'pageview': '${track}' + + '&e=Pageview' + + '&_ep_isp_info=%24ip_isp_info' + + '&_ep_geo_info=%24ip_geo_info', + 'engagement': '${track}' + + '&e=PageviewEngagement' + + '&_ep_engaged_time=5', + 'completion': '${track}' + + '&e=PageviewEngagement' + + '&_ep_completion=0.25', + }, + 'triggers': { + 'trackPageview': { + 'on': 'visible', + 'request': 'pageview', + }, + 'trackEngagement': { + 'on': 'visible', + 'timerSpec': { + 'interval': 5, + 'maxTimerLength': 600, + 'immediate': false, + }, + 'request': 'engagement', + }, + 'trackCompletion': { + 'on': 'scroll', + 'scrollSpec': { + 'verticalBoundaries': [25, 50, 75, 100], + }, + 'request': 'completion', + }, + }, + 'transport': { + 'beacon': false, + 'xhrpost': false, + 'image': true, + }, + 'extraUrlParams': { + 'properties.client.type': 'amp', + 'properties.client.title': '${title}', + 'properties.client.domain': '${canonicalHost}', + 'properties.client.url': '${canonicalUrl}', + 'properties.client.referrer': '${documentReferrer}', + 'properties.client.user_agent': '${userAgent}', + }, + 'extraUrlParamsReplaceMap': { + 'properties.': '_ep_', + }, +}); diff --git a/extensions/amp-analytics/0.1/visibility-manager.js b/extensions/amp-analytics/0.1/visibility-manager.js index 58502ad24098d..f2f3fa0b509de 100644 --- a/extensions/amp-analytics/0.1/visibility-manager.js +++ b/extensions/amp-analytics/0.1/visibility-manager.js @@ -322,6 +322,10 @@ export class VisibilityManager { */ listen_(model, spec, readyPromise, createReportPromiseFunc, callback, opt_element) { + if (createReportPromiseFunc) { + model.setReportReady(createReportPromiseFunc); + } + // Block visibility. if (readyPromise) { model.setReady(false); @@ -330,10 +334,6 @@ export class VisibilityManager { }); } - if (createReportPromiseFunc) { - model.setReportReady(createReportPromiseFunc); - } - // Process the event. model.onTriggerEvent(() => { const startTime = this.getStartTime(); diff --git a/extensions/amp-analytics/0.1/visibility-model.js b/extensions/amp-analytics/0.1/visibility-model.js index 56b36584cfc21..6f3bcf54b9eb2 100644 --- a/extensions/amp-analytics/0.1/visibility-model.js +++ b/extensions/amp-analytics/0.1/visibility-model.js @@ -51,6 +51,13 @@ export class VisibilityModel { this.spec_['visiblePercentageMax'] = 0; } + /** + * Accumulate visibility counters but do not fire the trigger until the + * ready promise resolves. + * @private @const {boolean} + */ + this.ignoreVisibilityForReport_ = spec['reportWhen'] !== undefined; + /** @private {boolean} */ this.repeat_ = spec['repeat'] === true; @@ -75,6 +82,9 @@ export class VisibilityModel { /** @const @private {time} */ this.createdTime_ = Date.now(); + // TODO(warrengm): Consider refactoring so that the ready defaults are + // false. + /** @private {boolean} */ this.ready_ = true; @@ -297,7 +307,11 @@ export class VisibilityModel { if (!this.eventResolver_) { return; } - const conditionsMet = this.updateCounters_(visibility); + + // When ignoreVisibilityForReport_ is true, we update counters but fire the + // event when the report ready promise is resolved. + const conditionsMet = + this.updateCounters_(visibility) || this.ignoreVisibilityForReport_; if (conditionsMet) { if (this.scheduledUpdateTimeoutId_) { clearTimeout(this.scheduledUpdateTimeoutId_); diff --git a/extensions/amp-analytics/amp-analytics.md b/extensions/amp-analytics/amp-analytics.md index c96418c4595b7..1007a1159cddf 100644 --- a/extensions/amp-analytics/amp-analytics.md +++ b/extensions/amp-analytics/amp-analytics.md @@ -442,7 +442,7 @@ NOTE: There is a [known issue](https://github.com/ampproject/amphtml/issues/1089 The `visibilitySpec` is a set of conditions and properties that can be applied to `visible` or `hidden` triggers to change when they fire. If multiple properties are specified, they must all be true in order for a request to fire. Configuration properties supported in `visibilitySpec` are: - `waitFor`: This property indicates that the visibility trigger should wait for a certain signal before tracking visibility. The supported values are `none`, `ini-load` and `render-start`. If `waitFor` is undefined, it is defaulted to [`ini-load`](#initial-load-trigger) when selector is specified, or to `none` otherwise. - - `reportWhen`: This property indicates that the visibility trigger should wait for a certain signal before sending the trigger. The only supported value is `documentExit`. `reportWhen` and `repeat` may not both be used in the same visibilitySpec. + - `reportWhen`: This property indicates that the visibility trigger should wait for a certain signal before sending the trigger. The only supported value is `documentExit`. `reportWhen` and `repeat` may not both be used in the same visibilitySpec. Note that when `reportWhen` is specified, the report will be sent at the time of the signal even if visibility requirements are not met at that time or have not been met previously. Any relevant variables (`totalVisibleTime`, etc.) will be populated according to the visibility requirements in this `visibilitySpec`. - `continuousTimeMin` and `continuousTimeMax`: These properties indicate that a request should be fired when (any part of) an element has been within the viewport for a continuous amount of time that is between the minimum and maximum specified times. The times are expressed in milliseconds. The `continuousTimeMin` is defaulted to 0 when not specified. - `totalTimeMin` and `totalTimeMax`: These properties indicate that a request should be fired when (any part of) an element has been within the viewport for a total amount of time that is between the minimum and maximum specified times. The times are expressed in milliseconds. The `totalTimeMin` is defaulted to 0 when not specified. - `visiblePercentageMin` and `visiblePercentageMax`: These properties indicate that a request should be fired when the proportion of an element that is visible within the viewport is between the minimum and maximum specified percentages. Percentage values between 0 and 100 are valid. Note that the upper bound (`visiblePercentageMax`) is inclusive. The lower bound (`visiblePercentageMin`) is exclusive, unless both bounds are set to 0 or both are set to 100. If both bounds are set to 0, then the trigger fires when the element is not visible. If both bounds are set to 100, the trigger fires when the element is fully visible. When these properties are defined along with other timing related properties, only the time when these properties are met are counted. The default values for `visiblePercentageMin` and `visiblePercentageMax` are 0 and 100, respectively. @@ -695,6 +695,12 @@ Detials on setting up your linker configuration are outlined in [Linker ID Forwa If you need to ingest this paramter, information on how this parameter is created is illistrated in [Linker ID Receiving](./linker-id-receiving.md). +#### Cookies + +The `cookies` feature supports writing cookies to the origin domain by extracting [`QUERY_PARAM`](https://github.com/ampproject/amphtml/blob/master/spec/amp-var-substitutions.md#query-parameter) and [`LINKER_PARAM`](./linker-id-receiving.md#linker-param) information from the document url. It can be used along with `linkers` features to perform ID syncing from the AMP proxied domain to AMP pages on a publisher's domain. + +Details on setting up the `cookies` configuration can be found at [Receiving Linker Params on AMP Pages](./linker-id-receiving.md#receiving-linker-params-on-amp-pages) + ## Validation See [amp-analytics rules](https://github.com/ampproject/amphtml/blob/master/extensions/amp-analytics/validator-amp-analytics.protoascii) in the AMP validator specification. diff --git a/extensions/amp-analytics/integrating-analytics.md b/extensions/amp-analytics/integrating-analytics.md index a87a7b486d149..128e41ba37752 100644 --- a/extensions/amp-analytics/integrating-analytics.md +++ b/extensions/amp-analytics/integrating-analytics.md @@ -37,6 +37,7 @@ reference. 1. Submit a Pull Request with this patch, referencing the Intent-To-Implement issue. 1. Add your analytics service to the [list of supported Analytics Vendors](https://github.com/ampproject/docs/blob/master/content/docs/analytics/analytics-vendors.md) by submitting a Pull Request to the [ampproject/docs](https://github.com/ampproject/docs) repo. Include the type, description, and link to your usage documentation. 1. Update your service's usage documentation and inform your customers. +1. It's highly recommended to maintain [an integration test outside AMP repo](../../3p/README.md#adding-proper-integration-tests). @@ -55,17 +56,6 @@ The endpoint approach is the same as the standard approach detailed in the previ To take this approach, review the documentation for publishers' integration with AMP Analytics. -## Add Batch Plugin -Batch plugin provides an easy way to construct batched requests on client side. Follow the instructions below to add a batch plugin. -1. Create an [Intent-To-Implement issue](../../CONTRIBUTING.md#contributing-features) stating that you'll be adding the batch plugin to your analytics service AMP HTML's runtime. -1. Develop a patch that implements the following: - 1. Add new plugin function in [batching-plugins.js](0.1/batching-plugins.js) - 1. Add test coverage to your plugin function - 1. Test your example in the [examples/analytics-vendors.amp.html](../../examples/analytics-vendors.amp.html) -1. Submit a Pull Request with this patch, referencing the Intent-To-Implement issue. - -Please note that every new plugin will be reviewed by AMP team on a case by case basis. The idea is to avoid a plugin as much as possible if there is a more generic approach. - ## Further Resources * Deep Dive: [Why not just use an iframe?](why-not-iframe.md) * Deep Dive: [Managing non-authenticated user state with AMP](https://github.com/ampproject/amphtml/blob/master/spec/amp-managing-user-state.md) diff --git a/extensions/amp-analytics/linker-id-forwarding.md b/extensions/amp-analytics/linker-id-forwarding.md index 0c935f6592339..c64af0e042ac8 100644 --- a/extensions/amp-analytics/linker-id-forwarding.md +++ b/extensions/amp-analytics/linker-id-forwarding.md @@ -21,7 +21,7 @@ This document outlines the configuration options that will determine in which co - `paramName` - This user defined name determines the name of the query parameter appended to the links. - `ids` - An object containing key-value pairs that is partially encoded and passed along in the param. - `proxyOnly` - (optional) Flag indicating whether the links should only be appended on pages served on a proxy origin. Defaults to `true`. -- `destinationDomains` - (optional) Links will be decorated if their domains are included in this array. Defaults to [`canonical`](https://github.com/ampproject/amphtml/blob/3b0feadab3b9b12ddb80edc9a30f959087134905/spec/amp-html-format.md#canon) and `source` domains. +- `destinationDomains` - (optional) Links will be decorated if their domains are included in this array. Defaults to [`canonical`](https://github.com/ampproject/amphtml/blob/3b0feadab3b9b12ddb80edc9a30f959087134905/spec/amp-html-format.md#canon) and `source` domains. A link matching the exact same hostname will not be decorated unless specified in this array.c - `enabled` - Publishers must explicity set this to `true` to opt-in to using this feature. This linker uses this configuration to generate a string in this structure: `=*****...` For more details see [Linker Param Format](./linker-id-receiving.md#Format) @@ -57,7 +57,7 @@ An example of a config that grants more granular control may look like the examp } } ``` -In this example configuration, the parameter would be appendend to any outgoing links matching the `source` or `canonical` domains. This is because the `destinationDomains` entry has been omitted and this is the default behavior. The example has `proxyOnly` set to `false`, this overrides the default behavior and indicates that the linker should manage outgoing links in all contexts this amp page might be served in. Finally, we have set `enabled` to be `true`. This is necessary to tell the runtime that we would like to enable this linker configuration. +In this example configuration, the parameter would be appendend to any outgoing links matching the `source` or `canonical` domains that are not an exact hostname match. This is because the `destinationDomains` entry has been omitted and this is the default behavior. The example has `proxyOnly` set to `false`, this overrides the default behavior and indicates that the linker should manage outgoing links in all contexts this amp page might be served in. Finally, we have set `enabled` to be `true`. This is necessary to tell the runtime that we would like to enable this linker configuration. #### Destination Domain Matching diff --git a/extensions/amp-analytics/linker-id-receiving.md b/extensions/amp-analytics/linker-id-receiving.md index bc31f3bc37c97..3e3f0e02ee0e0 100644 --- a/extensions/amp-analytics/linker-id-receiving.md +++ b/extensions/amp-analytics/linker-id-receiving.md @@ -1,8 +1,8 @@ -### Linker ID Receiving -#### Overview +## Linker ID Receiving +### Overview This document illustrates in detail how the linker param is constructed, so that analytics vendors or publishers implementing their own solutions are able to ingest this parameter on the destination page. -#### Format +### Format The linker param will be formatted in the following structure: `=*****...` @@ -53,3 +53,41 @@ You can find all the code for our implementaion in the [linker.js](./0.1/linker. #### Test Cases If you are implementing your own logic to verify these checksums, please ensure that you are using our test vectors located in [test-linker.js](./0.1/test/test-linker.js) + + +### Receiving Linker Params on AMP Pages + +The ``'s `cookies` feature can be used to extract information from the document url and write to the origin domain. + +#### Configuration + +A `cookies` example configuration looks like +``` +'cookies': { + 'enabled': true, + 'cookieName1': { + 'value': 'QUERY_PARAM(example)', + }, + 'cookieName2': { + 'value': 'LINKER_PARAM(exampleName, exampleValue)', + }, +} +``` + +The `enabled` value can be used to override default vendor settings. + +##### Cookie Names +Each key within the `cookies` config object defines the cookie name. It's value needs to be an object containing a single key-value pair. That key should be named `value`, and its value should be the macro that determines the information stored in the cookie. + +Note: The following key values are reserved, and cannot be used as cookie names. They are ['`referrerDomains`', '`enabled`', '`cookiePath`', '`cookieMaxAge`', '`cookieSecure`', '`cookieDomain`']. + +##### Cookie Values +Each cookie to write is defined by an object, where the value is defined by the required `value` field. + +Two macros are supported for the `value` field. [`QUERY_PARAM`](https://github.com/ampproject/amphtml/blob/master/spec/amp-var-substitutions.md#query-parameter) and [`LINKER_PARAM`](#linker-param). + +If there's error resoving the value, or the value is resolved to empty string. Nothing will be written to the cookie. + +##### LINKER PARAM +`LINKER_PARAM` takes two arguments, the `name` and the `value`. It will verify the checksum and read the `idName1*idValue1` key value pair from the linker param. The `value` ('idValue1' here) will be returned. + diff --git a/extensions/amp-bind/0.1/bind-events.js b/extensions/amp-bind/0.1/bind-events.js index 0f77fda406801..b0568d2a9f191 100644 --- a/extensions/amp-bind/0.1/bind-events.js +++ b/extensions/amp-bind/0.1/bind-events.js @@ -15,10 +15,9 @@ */ /** -* Enum used to specify Events for the Amp Bind extension -* -* @enum {string} -*/ + * TODO(choumx, #19657): Remove/replace with DOM polling in integration tests. + * @enum {string} + */ export const BindEvents = { INITIALIZE: 'amp:bind:initialize', RESCAN_TEMPLATE: 'amp:bind:rescan-template', diff --git a/extensions/amp-brightcove/0.1/amp-brightcove.js b/extensions/amp-brightcove/0.1/amp-brightcove.js index ebc96bbe80510..1075823bf7834 100644 --- a/extensions/amp-brightcove/0.1/amp-brightcove.js +++ b/extensions/amp-brightcove/0.1/amp-brightcove.js @@ -102,10 +102,9 @@ class AmpBrightcove extends AMP.BaseElement { /** @override */ buildCallback() { - const ampdoc = this.getAmpDoc(); - const deferred = new Deferred(); + this.urlReplacements_ = Services.urlReplacementsForDoc(this.element); - this.urlReplacements_ = Services.urlReplacementsForDoc(ampdoc); + const deferred = new Deferred(); this.playerReadyPromise_ = deferred.promise; this.playerReadyResolver_ = deferred.resolve; diff --git a/extensions/amp-call-tracking/0.1/amp-call-tracking.js b/extensions/amp-call-tracking/0.1/amp-call-tracking.js index d7b4bda08ed4e..2d128e93621fa 100644 --- a/extensions/amp-call-tracking/0.1/amp-call-tracking.js +++ b/extensions/amp-call-tracking/0.1/amp-call-tracking.js @@ -81,7 +81,7 @@ export class AmpCallTracking extends AMP.BaseElement { /** @override */ layoutCallback() { - return Services.urlReplacementsForDoc(this.getAmpDoc()) + return Services.urlReplacementsForDoc(this.element) .expandUrlAsync(user().assertString(this.configUrl_)) .then(url => fetch_(this.win, url)) .then(data => { diff --git a/extensions/amp-consent/0.1/consent-ui.js b/extensions/amp-consent/0.1/consent-ui.js index 900d90f45127d..72ac6d52145a8 100644 --- a/extensions/amp-consent/0.1/consent-ui.js +++ b/extensions/amp-consent/0.1/consent-ui.js @@ -32,7 +32,7 @@ import {setStyles, toggle} from '../../../src/style'; const TAG = 'amp-consent-ui'; // Classes for consent UI -const consentUiClasses = { +export const consentUiClasses = { iframeFullscreen: 'i-amphtml-consent-ui-iframe-fullscreen', iframeActive: 'i-amphtml-consent-ui-iframe-active', in: 'i-amphtml-consent-ui-in', diff --git a/extensions/amp-consent/0.1/test/test-consent-ui.js b/extensions/amp-consent/0.1/test/test-consent-ui.js index 52ffa75baeb6a..a8cd15bd380b8 100644 --- a/extensions/amp-consent/0.1/test/test-consent-ui.js +++ b/extensions/amp-consent/0.1/test/test-consent-ui.js @@ -16,16 +16,19 @@ import { ConsentUI, + consentUiClasses, } from '../consent-ui'; import {dict} from '../../../../src/utils/object'; import {elementByTag} from '../../../../src/dom'; import {toggleExperiment} from '../../../../src/experiments'; +import {whenCalled} from '../../../../testing/test-helper.js'; describes.realWin('consent-ui', { amp: { ampdoc: 'single', }, }, env => { + let sandbox; let win; let doc; let ampdoc; @@ -34,6 +37,7 @@ describes.realWin('consent-ui', { let parent; beforeEach(() => { + sandbox = env.sandbox; doc = env.win.document; ampdoc = env.ampdoc; win = env.win; @@ -66,6 +70,20 @@ describes.realWin('consent-ui', { toggleExperiment(win, 'amp-consent-v2', true); }); + afterEach(() => sandbox.restore()); + + const getReadyIframeCmpConsentUi = () => { + const config = dict({ + 'promptUISrc': 'https//promptUISrc', + }); + const consentUI = + new ConsentUI(mockInstance, config); + const showIframeSpy = sandbox.spy(consentUI, 'showIframe_'); + consentUI.show(); + consentUI.iframeReady_.resolve(); + return whenCalled(showIframeSpy).then(() => Promise.resolve(consentUI)); + }; + describe('init', () => { it('should repsect postPromptUI if there is one', function* () { consentUI = @@ -123,4 +141,103 @@ describes.realWin('consent-ui', { expect(elementByTag(parent, 'iframe')).to.be.null; }); }); + + describe('CMP Iframe', () => { + + it('should load the iframe, ' + + 'then show it with correct state CSS classes', () => { + const config = dict({ + 'promptUISrc': 'https//promptUISrc', + }); + consentUI = + new ConsentUI(mockInstance, config); + expect(parent.classList.contains('amp-active')).to.be.false; + expect(parent.classList.contains('amp-hidden')).to.be.false; + + const showIframeSpy = sandbox.spy(consentUI, 'showIframe_'); + + consentUI.show(); + expect(parent.classList.contains('amp-active')).to.be.true; + expect(parent.classList.contains(consentUiClasses.loading)).to.be.true; + expect(parent).to.not.have.display('none'); + + // Resolve the iframe ready + consentUI.iframeReady_.resolve(); + + return whenCalled(showIframeSpy).then(() => { + expect( + parent.classList.contains(consentUiClasses.iframeActive) + ).to.be.true; + }); + }); + }); + + describe('fullscreen', () => { + + it('should respond to the fullscreen event', () => { + + return getReadyIframeCmpConsentUi().then(consentUI => { + const enterFullscreenStub = sandbox.stub(consentUI, 'enterFullscreen_'); + + consentUI.ui_ = { + contentWindow: 'mock-src', + }; + consentUI.handleIframeMessages_({ + source: 'mock-src', + data: { + type: 'consent-ui-enter-fullscreen', + }, + }); + + expect(enterFullscreenStub).to.be.calledOnce; + }); + }); + + it('should not handle the fullscreen event, ' + + 'if the iframe wasn\'t visible', () => { + + return getReadyIframeCmpConsentUi().then(consentUI => { + const enterFullscreenStub = sandbox.stub(consentUI, 'enterFullscreen_'); + + consentUI.ui_ = { + contentWindow: 'mock-src', + }; + consentUI.isIframeVisible_ = false; + consentUI.handleIframeMessages_({ + source: 'mock-src', + data: { + type: 'consent-ui-enter-fullscreen', + }, + }); + + expect(enterFullscreenStub).to.not.be.called; + }); + }); + + + describe('enterFullscreen', () => { + it('should add fullscreen classes and set fullscreen state', () => { + return getReadyIframeCmpConsentUi().then(consentUI => { + consentUI.enterFullscreen_(); + + expect( + parent.classList.contains(consentUiClasses.iframeFullscreen) + ).to.be.true; + expect(consentUI.isFullscreen_).to.be.true; + }); + }); + + it('should not enter fullscreen if already fullscreen', () => { + return getReadyIframeCmpConsentUi().then(consentUI => { + consentUI.isFullscreen_ = true; + consentUI.enterFullscreen_(); + + expect( + parent.classList.contains(consentUiClasses.iframeFullscreen) + ).to.be.false; + }); + }); + }); + + }); }); diff --git a/extensions/amp-form/0.1/test/integration/test-integration-form-verifiers.js b/extensions/amp-form/0.1/test/integration/test-integration-form-verifiers.js index 62ef5f9304f8e..723f205bbce8d 100644 --- a/extensions/amp-form/0.1/test/integration/test-integration-form-verifiers.js +++ b/extensions/amp-form/0.1/test/integration/test-integration-form-verifiers.js @@ -22,7 +22,8 @@ const RENDER_TIMEOUT = 15000; const describeChrome = describe.configure().ifNewChrome().skipSinglePass(); -describeChrome.run('amp-form verifiers', function() { +// TODO(cvializ, #19647): Broken on SL Chrome 71. +describeChrome.skip('amp-form verifiers', function() { this.timeout(RENDER_TIMEOUT); describes.integration('verify-error template', { diff --git a/extensions/amp-form/0.1/test/integration/test-integration-form.js b/extensions/amp-form/0.1/test/integration/test-integration-form.js index 31f00fa203f6e..00be43ad21d36 100644 --- a/extensions/amp-form/0.1/test/integration/test-integration-form.js +++ b/extensions/amp-form/0.1/test/integration/test-integration-form.js @@ -163,7 +163,8 @@ describes.realWin('AmpForm Integration', { }); }); - describeChrome.run('Submit xhr-POST', function() { + // TODO(cvializ, #19647): Broken on SL Chrome 71. + describeChrome.skip('Submit xhr-POST', function() { this.timeout(RENDER_TIMEOUT); it('should submit and render success', () => { @@ -227,7 +228,8 @@ describes.realWin('AmpForm Integration', { }); }); - describeChrome.run('Submit xhr-GET', function() { + // TODO(cvializ, #19647): Broken on SL Chrome 71. + describeChrome.skip('Submit xhr-GET', function() { this.timeout(RENDER_TIMEOUT); it('should submit and render success', () => { @@ -293,7 +295,8 @@ describes.realWin('AmpForm Integration', { }); }); - describeChrome.run('Submit result message', () => { + // TODO(cvializ, #19647): Broken on SL Chrome 71. + describeChrome.skip('Submit result message', () => { it('should render messages with or without a template', () => { // Stubbing timeout to catch async-thrown errors and expect // them. These catch errors thrown inside the catch-clause of the diff --git a/extensions/amp-fx-flying-carpet/0.1/amp-fx-flying-carpet.js b/extensions/amp-fx-flying-carpet/0.1/amp-fx-flying-carpet.js index 06ef8a4774756..52b54aecadaee 100644 --- a/extensions/amp-fx-flying-carpet/0.1/amp-fx-flying-carpet.js +++ b/extensions/amp-fx-flying-carpet/0.1/amp-fx-flying-carpet.js @@ -69,6 +69,7 @@ export class AmpFlyingCarpet extends AMP.BaseElement { /** @override */ buildCallback() { + const doc = this.element.ownerDocument; const container = doc.createElement('div'); @@ -184,6 +185,14 @@ export class AmpFlyingCarpet extends AMP.BaseElement { } } + /** + * Returns our discovered children + * @return {!Array} + */ + getChildren() { + return this.children_; + } + /** * Determines the child nodes that are "visible". We purposefully ignore Text * nodes that only contain whitespace since they do not contribute anything diff --git a/extensions/amp-geo/amp-geo.md b/extensions/amp-geo/amp-geo.md index 523c9e198baea..7400f38c6c7a0 100644 --- a/extensions/amp-geo/amp-geo.md +++ b/extensions/amp-geo/amp-geo.md @@ -236,7 +236,9 @@ The string `{{AMP_ISO_COUNTRY_HOTPATCH}}` must be replaced at serving time with ### Debugging -Adding `#amp-geo=XX` to the document url forces the country to appear as the country `XX`. This allows you to test without having to VPN to a country. For security reasons, to prevent sharing of geo-spoofing urls, this feature is only available to users who have enabled the [Dev Channel](https://www.ampproject.org/docs/reference/experimental) or who are testing locally (i.e., `amp-geo.js` is served in development mode via [`gulp serve`](https://github.com/ampproject/amphtml/blob/master/contributing/DEVELOPING.md)) +Adding `#amp-geo=XX` to the document url forces the country to appear as the country `XX`. This allows you to test without having to VPN to a country. For security reasons, to prevent sharing of geo-spoofing urls, this feature is only available to users who have enabled the [Dev Channel](https://www.ampproject.org/docs/reference/experimental) or who are testing locally (i.e., `amp-geo.js` is served in development mode via [`gulp serve`](https://github.com/ampproject/amphtml/blob/master/contributing/DEVELOPING.md)). + +**Note:** Debugging in DevChannel may not work in Safari due to [ITP](https://webkit.org/blog/8311/intelligent-tracking-prevention-2-0/). ## Validation diff --git a/extensions/amp-list/0.1/amp-list.css b/extensions/amp-list/0.1/amp-list.css index 2f827e4bf73d5..2b595bc767de4 100644 --- a/extensions/amp-list/0.1/amp-list.css +++ b/extensions/amp-list/0.1/amp-list.css @@ -14,9 +14,10 @@ * limitations under the License. */ -amp-list[load-more] > [load-more-loading].amp-visible, -amp-list[load-more] > [load-more-failed].amp-visible, -amp-list[load-more] > [load-more-button].amp-visible, -amp-list[load-more] > [load-more-end].amp-visible { - visibility: visible; +amp-list[load-more] [load-more-loading].amp-visible, +amp-list[load-more] [load-more-failed].amp-visible, +amp-list[load-more] [load-more-button].amp-visible, +amp-list[load-more] [load-more-end].amp-visible { + display: block; + width: 100%; } \ No newline at end of file diff --git a/extensions/amp-list/0.1/amp-list.js b/extensions/amp-list/0.1/amp-list.js index 01836ab64c70e..77ace8eb59a6c 100644 --- a/extensions/amp-list/0.1/amp-list.js +++ b/extensions/amp-list/0.1/amp-list.js @@ -399,7 +399,6 @@ export class AmpList extends AMP.BaseElement { // Construct the fetch init data that would be called by the viewer // passed in as the 'originalRequest'. return requestForBatchFetch( - this.getAmpDoc(), this.element, this.getPolicy_(), refresh).then(r => { @@ -448,7 +447,7 @@ export class AmpList extends AMP.BaseElement { } this.renderItems_ = {data, append, resolver, rejecter, - 'payload': opt_payload}; + payload: opt_payload}; if (this.renderedItems_ && append) { this.renderItems_.data = this.renderedItems_.concat(data); @@ -487,7 +486,8 @@ export class AmpList extends AMP.BaseElement { if (this.ssrTemplateHelper_.isSupported()) { const html = /** @type {string} */ (current.data); this.templates_.findAndSetHtmlForTemplate(this.element, html) - .then(element => this.render_([element], current.append)) + .then(result => this.updateBindings_([result])) + .then(element => this.render_(element, current.append)) .then(onFulfilledCallback, onRejectedCallback); } else { const array = /** @type {!Array} */ (current.data); @@ -496,7 +496,7 @@ export class AmpList extends AMP.BaseElement { .then(results => this.updateBindings_(results)) .then(elements => this.render_(elements, current.append)) .then(() => this.maybeRenderLoadMoreTemplates_(payload)) - .then(() => this.loadMoreEnabled_ && this.setLoadMore_()) + .then(() => this.maybeSetLoadMore_()) .then(onFulfilledCallback, onRejectedCallback); } } @@ -507,6 +507,9 @@ export class AmpList extends AMP.BaseElement { * @private */ maybeRenderLoadMoreTemplates_(data) { + if (!this.loadMoreEnabled_) { + return Promise.resolve(); + } const promises = []; promises.push(this.maybeRenderLoadMoreElement_(this.loadMoreButton_, data)); promises.push(this.maybeRenderLoadMoreElement_( @@ -524,7 +527,7 @@ export class AmpList extends AMP.BaseElement { if (elem && this.templates_.hasTemplate(elem)) { return this.templates_.findAndRenderTemplate(elem, data) .then(newContents => { - this.mutateElement(() => { + return this.mutateElement(() => { removeChildren(dev().assertElement(elem)); elem.appendChild(newContents); }); @@ -601,6 +604,9 @@ export class AmpList extends AMP.BaseElement { removeChildren(container); } this.addElementsToContainer_(elements, container); + if (this.loadMoreEnabled_) { + this.moveButtonsToBottom_(container); + } } const event = createCustomEvent(this.win, @@ -717,25 +723,25 @@ export class AmpList extends AMP.BaseElement { } /** + * @return {!Promise} * @private */ - setLoadMore_() { - // Done loading, nothing more to load. - if (!this.loadMoreSrc_) { - return this.mutateElement(() => { - this.loadMoreButton_.classList.toggle('amp-visible', false); - this.loadMoreEndElement_.classList.toggle('amp-visible', true); - }); + maybeSetLoadMore_() { + const shouldSetLoadMore = this.loadMoreEnabled_ && this.loadMoreSrc_; + if (!shouldSetLoadMore) { + return Promise.resolve(); } const triggerOnScroll = this.element.getAttribute('load-more') === 'auto'; if (triggerOnScroll) { this.maybeSetupLoadMoreAuto_(); } if (this.loadMoreButton_) { - this.mutateElement(() => { + return this.mutateElement(() => { this.loadMoreButton_.classList.toggle('amp-visible', true); this.unlistenLoadMore_ = listen(this.loadMoreButton_, 'click', () => this.loadMoreCallback_()); + // Guarantees that the height accounts for the newly visible button + this.attemptToFit_(dev().assertElement(this.container_)); }); } if (!this.loadMoreButton_ && !triggerOnScroll) { @@ -743,6 +749,29 @@ export class AmpList extends AMP.BaseElement { 'load-more is specified but no means of paging (overflow or ' + 'load-more=auto) is available', this); } + return Promise.resolve(); + } + + + /** + * Moves all the load-more visual elements to the bottom of the list + * after newly appended items. + * @param {!Element} container + * @private + */ + moveButtonsToBottom_(container) { + if (this.loadMoreButton_) { + container.appendChild(this.loadMoreButton_); + } + if (this.loadMoreEndElement_) { + container.appendChild(this.loadMoreEndElement_); + } + if (this.loadMoreFailedElement_) { + container.appendChild(this.loadMoreFailedElement_); + } + if (this.loadMoreLoadingElement_) { + container.appendChild(this.loadMoreLoadingElement_); + } } /** @@ -759,7 +788,11 @@ export class AmpList extends AMP.BaseElement { this.toggleLoadMoreLoading_(true); return this.fetchList_(/* opt_append */ true) .then(() => { - this.toggleLoadMoreLoading_(false); + if (this.loadMoreSrc_) { + this.toggleLoadMoreLoading_(false); + } else { + this.setLoadMoreEnded_(); + } if (this.unlistenLoadMore_) { this.unlistenLoadMore_(); this.unlistenLoadMore_ = null; @@ -793,6 +826,20 @@ export class AmpList extends AMP.BaseElement { return this.loadMoreLoadingOverlay_; } + /** + * @return {!Promise} + * @private + */ + setLoadMoreEnded_() { + return this.mutateElement(() => { + this.loadMoreFailedElement_.classList.toggle('amp-visible', false); + this.loadMoreButton_.classList.toggle('amp-visible', false); + this.loadMoreLoadingElement_.classList.toggle('amp-visible', false); + if (this.loadMoreEndElement_) { + this.loadMoreEndElement_.classList.toggle('amp-visible', true); + } + }); + } /** * Toggles the visibility of the load-more-loading element, the * amp-load-more-loading CSS class, and the active state of the loader. @@ -801,10 +848,13 @@ export class AmpList extends AMP.BaseElement { */ toggleLoadMoreLoading_(state) { this.mutateElement(() => { - // If it's loading, then it's no longer failed + // If it's loading, then it's no longer failed or ended if (this.loadMoreFailedElement_) { this.loadMoreFailedElement_.classList.toggle('amp-visible', false); } + if (this.loadMoreEndElement_) { + this.loadMoreEndElement_.classList.toggle('amp-visible', false); + } if (this.loadMoreLoadingElement_) { this.loadMoreButton_.classList.toggle('amp-visible', !state); this.loadMoreLoadingElement_.classList.toggle('amp-visible', state); diff --git a/extensions/amp-list/0.1/test/integration/test-amp-list.js b/extensions/amp-list/0.1/test/integration/test-amp-list.js index 775f8f4f5ab13..b68afa4fad649 100644 --- a/extensions/amp-list/0.1/test/integration/test-amp-list.js +++ b/extensions/amp-list/0.1/test/integration/test-amp-list.js @@ -103,7 +103,8 @@ describe('amp-list', function() { '' + ''; - describes.integration('with bindable is-layout-container', { + // TODO(cathyxz, #19647): Fix test on Chrome 71. + describes.integration.skip('with bindable is-layout-container', { body: body3, extensions, experiments: ['amp-list-resizable-children']}, env => { diff --git a/extensions/amp-list/0.1/test/test-amp-list.js b/extensions/amp-list/0.1/test/test-amp-list.js index f95cbfff1c1ad..6ee44fa6082e3 100644 --- a/extensions/amp-list/0.1/test/test-amp-list.js +++ b/extensions/amp-list/0.1/test/test-amp-list.js @@ -444,6 +444,7 @@ describes.realWin('amp-list component', { it('should delegate template rendering to viewer', function*() { sandbox.stub(ssrTemplateHelper, 'fetchAndRenderTemplate') .returns(Promise.resolve({html: '

foo

'})); + sandbox.spy(list, 'updateBindings_'); // Expects mutate/measure and hiding of loading/placeholder indicators. expectRender(); @@ -472,7 +473,7 @@ describes.realWin('amp-list component', { expect(ssrTemplateHelper.fetchAndRenderTemplate).to.be.calledOnce; expect(ssrTemplateHelper.fetchAndRenderTemplate) .to.be.calledWithExactly(element, request, null, attrs); - + expect(list.updateBindings_).to.be.calledOnce; expect(list.container_.contains(rendered)).to.be.true; }); }); diff --git a/extensions/amp-pinterest/0.1/pinit-button.js b/extensions/amp-pinterest/0.1/pinit-button.js index 6dd86d27a8938..6106098641d19 100644 --- a/extensions/amp-pinterest/0.1/pinit-button.js +++ b/extensions/amp-pinterest/0.1/pinit-button.js @@ -56,8 +56,11 @@ export class PinItButton { this.round = rootElement.getAttribute('data-round'); this.tall = rootElement.getAttribute('data-tall'); this.description = rootElement.getAttribute('data-description'); + /** @type {?string} */ this.media = null; + /** @type {?string} */ this.url = null; + /** @type {?string} */ this.href = null; } diff --git a/extensions/amp-powr-player/0.1/amp-powr-player.js b/extensions/amp-powr-player/0.1/amp-powr-player.js index 94b657610494c..0eb00e4a87ab0 100644 --- a/extensions/amp-powr-player/0.1/amp-powr-player.js +++ b/extensions/amp-powr-player/0.1/amp-powr-player.js @@ -110,10 +110,9 @@ class AmpPowrPlayer extends AMP.BaseElement { /** @override */ buildCallback() { - const ampdoc = this.getAmpDoc(); - const deferred = new Deferred(); + this.urlReplacements_ = Services.urlReplacementsForDoc(this.element); - this.urlReplacements_ = Services.urlReplacementsForDoc(ampdoc); + const deferred = new Deferred(); this.playerReadyPromise_ = deferred.promise; this.playerReadyResolver_ = deferred.resolve; } diff --git a/extensions/amp-social-share/0.1/amp-social-share.js b/extensions/amp-social-share/0.1/amp-social-share.js index 559cc172162bd..817e49b855751 100644 --- a/extensions/amp-social-share/0.1/amp-social-share.js +++ b/extensions/amp-social-share/0.1/amp-social-share.js @@ -91,7 +91,7 @@ class AmpSocialShare extends AMP.BaseElement { getDataParamsFromAttributes(element)); const hrefWithVars = addParamsToUrl(this.shareEndpoint_, this.params_); - const urlReplacements = Services.urlReplacementsForDoc(this.getAmpDoc()); + const urlReplacements = Services.urlReplacementsForDoc(this.element); const bindingVars = typeConfig['bindings']; const bindings = {}; if (bindingVars) { diff --git a/extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.css b/extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.css index da8f5a8b09040..38928942552f0 100644 --- a/extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.css +++ b/extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.css @@ -20,6 +20,20 @@ transform: scale(1.0) translateX(-100%) translateY(200%) !important; } +/** + * amp-story-auto-ads tags should never be visible. keep them hidden. + */ +amp-story-auto-ads { + /* Fixed to make position independent of page other elements. */ + position: fixed !important; + top: 0 !important; + left: 0 !important; + width: 1px !important; + height: 1px !important; + overflow: hidden !important; + visibility: hidden; +} + .i-amphtml-story-ad-link { background-color: #FFFFFF !important; border-radius: 20px !important; diff --git a/extensions/amp-story/0.1/amp-story-request-service.js b/extensions/amp-story/0.1/amp-story-request-service.js index 6c6428134d8b5..0d1b73ad0b8cb 100644 --- a/extensions/amp-story/0.1/amp-story-request-service.js +++ b/extensions/amp-story/0.1/amp-story-request-service.js @@ -15,7 +15,6 @@ */ import {Services} from '../../../src/services'; -import {getAmpdoc} from '../../../src/service'; import {once} from '../../../src/utils/function'; import {user} from '../../../src/log'; @@ -67,7 +66,7 @@ export class AmpStoryRequestService { const opts = {}; opts.requireAmpResponseSourceOrigin = false; - return Services.urlReplacementsForDoc(getAmpdoc(this.storyElement_)) + return Services.urlReplacementsForDoc(this.storyElement_) .expandUrlAsync(user().assertString(rawUrl)) .then(url => this.xhr_.fetchJson(url, opts)) .then(response => { diff --git a/extensions/amp-story/1.0/_locales/en.js b/extensions/amp-story/1.0/_locales/en.js index fe94c58023bce..b6b1361bca9f6 100644 --- a/extensions/amp-story/1.0/_locales/en.js +++ b/extensions/amp-story/1.0/_locales/en.js @@ -76,6 +76,10 @@ export default /** @const {!LocalizedStringBundleDef} */ ({ description: 'Label for a link to documentation on how AMP links are ' + 'handled.', }, + [LocalizedStringId.AMP_STORY_PAGE_PLAY_VIDEO]: { + string: 'Play video', + description: 'Label for a button to play the video visible on the page.', + }, [LocalizedStringId.AMP_STORY_HINT_UI_NEXT_LABEL]: { string: 'Tap Next', description: 'Label indicating that users can navigate to the next ' + diff --git a/extensions/amp-story/1.0/amp-story-bookend.css b/extensions/amp-story/1.0/amp-story-bookend.css index 48cfd666c808b..f3d73ff690cef 100644 --- a/extensions/amp-story/1.0/amp-story-bookend.css +++ b/extensions/amp-story/1.0/amp-story-bookend.css @@ -77,12 +77,13 @@ overflow: hidden !important; } -.i-amphtml-story-bookend-consent { - margin: 0 32px !important; +.i-amphtml-story-bookend-top-level { + margin-left: 24px !important; + margin-right: 24px !important; } .i-amphtml-story-bookend-consent:last-child { - margin-bottom: 32px !important; + margin-bottom: 24px !important; } .i-amphtml-story-bookend-consent .i-amphtml-story-bookend-heading { @@ -248,7 +249,6 @@ } .i-amphtml-story-bookend-replay { - margin: 0 24px !important; overflow: hidden !important; display: flex !important; max-height: 120px !important; @@ -279,7 +279,6 @@ } .i-amphtml-story-bookend-component-set { - margin: 0 24px !important; margin-bottom: 24px !important; } diff --git a/extensions/amp-story/1.0/amp-story-page.js b/extensions/amp-story/1.0/amp-story-page.js index ea43ae15be4e7..35ef16e0c8552 100644 --- a/extensions/amp-story/1.0/amp-story-page.js +++ b/extensions/amp-story/1.0/amp-story-page.js @@ -39,6 +39,7 @@ import {Deferred} from '../../../src/utils/promise'; import {EventType, dispatch} from './events'; import {Layout} from '../../../src/layout'; import {LoadingSpinner} from './loading-spinner'; +import {LocalizedStringId} from './localization'; import {MediaPool} from './media-pool'; import {Services} from '../../../src/services'; import {VideoServiceSync} from '../../../src/service/video-service-sync-impl'; @@ -56,8 +57,10 @@ import { } from '../../../src/friendly-iframe-embed'; import {getLogEntries} from './logging'; import {getMode} from '../../../src/mode'; +import {htmlFor} from '../../../src/static-template'; import {listen} from '../../../src/event-helper'; import {toArray} from '../../../src/types'; +import {toggle} from '../../../src/style'; import {upgradeBackgroundAudio} from './audio'; /** @@ -86,6 +89,18 @@ const ADVERTISEMENT_ATTR_NAME = 'ad'; /** @private @const {number} */ const REWIND_TIMEOUT_MS = 350; +/** + * @param {!Element} element + * @return {!Element} + */ +const buildPlayMessageElement = element => + htmlFor(element)` + `; + /** * amp-story-page states. * @enum {number} @@ -118,6 +133,9 @@ export class AmpStoryPage extends AMP.BaseElement { /** @private {?LoadingSpinner} */ this.loadingSpinner_ = null; + /** @private {?Element} */ + this.playMessageEl_ = null; + /** @private @const {!Promise} */ this.mediaLayoutPromise_ = this.waitForMediaLayout_(); @@ -279,6 +297,7 @@ export class AmpStoryPage extends AMP.BaseElement { this.advancement_.stop(); this.stopListeningToVideoEvents_(); + this.togglePlayMessage_(false); if (this.storeService_.get(StateProperty.UI_STATE) === UIType.DESKTOP) { // The rewinding is delayed on desktop so that it happens at a lower // opacity instead of immediately jumping to the first frame. See #17985. @@ -476,7 +495,13 @@ export class AmpStoryPage extends AMP.BaseElement { mediaEl.play(); } else { return mediaPool.play( - /** @type {!./media-pool.DomElementDef} */ (mediaEl)); + /** @type {!./media-pool.DomElementDef} */ (mediaEl)).catch(() => { + // Auto playing the media failed, which could be caused by a data + // saver, or a battery saving mode. Display a message so we can + // get a user gesture to bless the media elements, and play them. + this.debounceToggleLoadingSpinner_(false); + this.togglePlayMessage_(true); + }); } }); } @@ -841,6 +866,52 @@ export class AmpStoryPage extends AMP.BaseElement { }); } + /** + * Builds and appends a message and icon to play the story on tap. + * This message is built when the playback failed (data saver, low battery + * modes, ...). + * @private + */ + buildAndAppendPlayMessage_() { + const localizationService = Services.localizationService(this.win); + + this.playMessageEl_ = buildPlayMessageElement(this.element); + const labelEl = + this.playMessageEl_.querySelector('.i-amphtml-story-page-play-label'); + labelEl.textContent = localizationService.getLocalizedString( + LocalizedStringId.AMP_STORY_PAGE_PLAY_VIDEO); + + this.playMessageEl_.addEventListener('click', () => { + this.togglePlayMessage_(false); + this.mediaPoolPromise_ + .then(mediaPool => mediaPool.blessAll()) + .then(() => this.playAllMedia_()); + }); + + this.mutateElement(() => this.element.appendChild(this.playMessageEl_)); + } + + /** + * Toggles the visibility of the "Play video" fallback message. + * @param {boolean} isActive + * @private + */ + togglePlayMessage_(isActive) { + if (!isActive) { + this.playMessageEl_ && + this.mutateElement(() => + toggle(dev().assertElement(this.playMessageEl_), false)); + return; + } + + if (!this.playMessageEl_) { + this.buildAndAppendPlayMessage_(); + } + + this.mutateElement(() => + toggle(dev().assertElement(this.playMessageEl_), true)); + } + /** * check to see if this page is a wrapper for an ad * @return {boolean} diff --git a/extensions/amp-story/1.0/amp-story-request-service.js b/extensions/amp-story/1.0/amp-story-request-service.js index 0ba04c04af290..ba292fa17c8db 100644 --- a/extensions/amp-story/1.0/amp-story-request-service.js +++ b/extensions/amp-story/1.0/amp-story-request-service.js @@ -16,10 +16,10 @@ import {Services} from '../../../src/services'; import {childElementByTag} from '../../../src/dom'; -import {getAmpdoc, registerServiceBuilder} from '../../../src/service'; import {getChildJsonConfig} from '../../../src/json'; import {isProtocolValid} from '../../../src/url'; import {once} from '../../../src/utils/function'; +import {registerServiceBuilder} from '../../../src/service'; import {user} from '../../../src/log'; /** @private @const {string} */ @@ -89,7 +89,7 @@ export class AmpStoryRequestService { return Promise.resolve(null); } - return Services.urlReplacementsForDoc(getAmpdoc(this.storyElement_)) + return Services.urlReplacementsForDoc(this.storyElement_) .expandUrlAsync(user().assertString(rawUrl)) .then(url => this.xhr_.fetchJson(url, opts)) .then(response => { diff --git a/extensions/amp-story/1.0/amp-story-store-service.js b/extensions/amp-story/1.0/amp-story-store-service.js index cb4d6a70b2e0e..4343205058f08 100644 --- a/extensions/amp-story/1.0/amp-story-store-service.js +++ b/extensions/amp-story/1.0/amp-story-store-service.js @@ -87,6 +87,7 @@ export const UIType = { * consentId: ?string, * currentPageId: string, * currentPageIndex: number, + * pagesCount: number, * }} */ export let State; @@ -130,6 +131,7 @@ export const StateProperty = { CONSENT_ID: 'consentId', CURRENT_PAGE_ID: 'currentPageId', CURRENT_PAGE_INDEX: 'currentPageIndex', + PAGES_COUNT: 'pagesCount', }; @@ -138,6 +140,7 @@ export const Action = { ADD_TO_ACTIONS_WHITELIST: 'addToActionsWhitelist', CHANGE_PAGE: 'setCurrentPageId', SET_CONSENT_ID: 'setConsentId', + SET_PAGES_COUNT: 'setPagesCount', TOGGLE_ACCESS: 'toggleAccess', TOGGLE_AD: 'toggleAd', TOGGLE_BOOKEND: 'toggleBookend', @@ -286,8 +289,11 @@ const actions = (state, action, data) => { [StateProperty.CURRENT_PAGE_ID]: data.id, [StateProperty.CURRENT_PAGE_INDEX]: data.index, })); + case Action.SET_PAGES_COUNT: + return /** @type {!State} */ (Object.assign( + {}, state, {[StateProperty.PAGES_COUNT]: data})); default: - dev().error(TAG, `Unknown action ${action}.`); + dev().error(TAG, 'Unknown action %s.', action); return state; } }; @@ -319,7 +325,7 @@ export class AmpStoryStoreService { */ get(key) { if (!hasOwn(this.state_, key)) { - dev().error(TAG, `Unknown state ${key}.`); + dev().error(TAG, 'Unknown state %s.', key); return; } return this.state_[key]; @@ -334,7 +340,7 @@ export class AmpStoryStoreService { */ subscribe(key, listener, callToInitialize = false) { if (!hasOwn(this.state_, key)) { - dev().error(TAG, `Can't subscribe to unknown state ${key}.`); + dev().error(TAG, 'Can\'t subscribe to unknown state %s.', key); return; } if (!this.listeners_[key]) { @@ -408,6 +414,7 @@ export class AmpStoryStoreService { [StateProperty.CONSENT_ID]: null, [StateProperty.CURRENT_PAGE_ID]: '', [StateProperty.CURRENT_PAGE_INDEX]: 0, + [StateProperty.PAGES_COUNT]: 0, }); } diff --git a/extensions/amp-story/1.0/amp-story.css b/extensions/amp-story/1.0/amp-story.css index a519315f661e4..4591158187796 100644 --- a/extensions/amp-story/1.0/amp-story.css +++ b/extensions/amp-story/1.0/amp-story.css @@ -643,3 +643,50 @@ amp-story .amp-video-eq, 50% { transform: rotate(5deg) } to { transform: rotate(-130deg) } } + +.i-amphtml-story-page-play-button { + display: flex !important; + align-items: center !important; + position: absolute !important; + bottom: 0 !important; + right: 0 !important; + height: 40px !important; + border: 0 !important; + margin: 8px 0 0 8px !important; /* Making the click target 48px.*/ + padding: 0 !important; + animation: play-button-fly-in 0.4s cubic-bezier(0.4, 0.0, 0.2, 1) !important; + background-color: transparent !important; + z-index: 10000 !important; +} + +@keyframes play-button-fly-in { + 0% { + opacity: 0; + transform: translateX(12px); + } + to { + opacity: 1; + transform: translateX(0); + } +} + +.i-amphtml-story-page-play-button[hidden] { + display: none !important; +} + +.i-amphtml-story-page-play-label { + color: #fff !important; + font-family: 'Roboto', sans-serif !important; + font-size: 14px !important; + text-shadow: 0px 0px 6px rgba(0, 0, 0, 0.36) !important; +} + +.i-amphtml-story-page-play-icon { + display: inline-block !important; + height: 24px !important; + width: 24px !important; + margin: 0 8px !important; + border-radius: 24px !important; + filter: drop-shadow(0px 0px 6px rgba(0, 0, 0, 0.36)) !important; + background-image: url('data:image/svg+xml;charset=utf-8,') !important; +} diff --git a/extensions/amp-story/1.0/amp-story.js b/extensions/amp-story/1.0/amp-story.js index f93fd493362b5..f3d3055006879 100644 --- a/extensions/amp-story/1.0/amp-story.js +++ b/extensions/amp-story/1.0/amp-story.js @@ -234,7 +234,7 @@ export class AmpStory extends AMP.BaseElement { /** Instantiates the viewport warning layer. */ new ViewportWarningLayer(this.win, this.element); - /** @private @const {!Array} */ + /** @private {!Array} */ this.pages_ = []; /** @private @const {!Array} */ @@ -845,7 +845,7 @@ export class AmpStory extends AMP.BaseElement { if (toRemoveChildren.length === 0) { return; } - dev().error(TAG, `amp-consent only allows tags: ${allowedTags}`); + dev().error(TAG, 'amp-consent only allows tags: %s', allowedTags); toRemoveChildren.forEach(el => consentEl.removeChild(el)); } @@ -914,13 +914,12 @@ export class AmpStory extends AMP.BaseElement { initializePages_() { const pageImplPromises = Array.prototype.map.call( this.element.querySelectorAll('amp-story-page'), - (pageEl, index) => { - return pageEl.getImpl().then(pageImpl => { - this.pages_[index] = pageImpl; - }); - }); + pageEl => pageEl.getImpl()); - return Promise.all(pageImplPromises); + return Promise.all(pageImplPromises).then(pages => { + this.storeService_.dispatch(Action.SET_PAGES_COUNT, pages.length); + this.pages_ = pages; + }); } /** @@ -1775,7 +1774,7 @@ export class AmpStory extends AMP.BaseElement { const pageIndex = findIndex(this.pages_, page => page.element.id === id); if (pageIndex < 0) { user().error(TAG, - `Story refers to page "${id}", but no such page exists.`); + 'Story refers to page "%s", but no such page exists.', id); } return pageIndex; @@ -1789,7 +1788,7 @@ export class AmpStory extends AMP.BaseElement { getPageById(id) { const pageIndex = this.getPageIndexById(id); return dev().assert(this.pages_[pageIndex], - `Page at index ${pageIndex} exists, but is missing from the array.`); + 'Page at index %s exists, but is missing from the array.', pageIndex); } /** diff --git a/extensions/amp-story/1.0/bookend/amp-story-bookend.js b/extensions/amp-story/1.0/bookend/amp-story-bookend.js index 6849591820942..bcff2e5e0f4ac 100644 --- a/extensions/amp-story/1.0/bookend/amp-story-bookend.js +++ b/extensions/amp-story/1.0/bookend/amp-story-bookend.js @@ -104,7 +104,8 @@ const TAG = 'amp-story-bookend'; const buildReplayButtonTemplate = (title, domainName, imageUrl = undefined) => { return /** @type {!../simple-template.ElementDef} */ ({ tag: 'div', - attrs: dict({'class': 'i-amphtml-story-bookend-replay'}), + attrs: dict({'class': 'i-amphtml-story-bookend-replay ' + + 'i-amphtml-story-bookend-top-level'}), children: [ { tag: 'div', @@ -141,7 +142,8 @@ const buildReplayButtonTemplate = (title, domainName, imageUrl = undefined) => { const buildPromptConsentTemplate = consentId => { return /** @type {!../simple-template.ElementDef} */ ({ tag: 'div', - attrs: dict({'class': 'i-amphtml-story-bookend-consent'}), + attrs: dict({'class': 'i-amphtml-story-bookend-consent ' + + 'i-amphtml-story-bookend-top-level'}), children: [ { tag: 'h3', diff --git a/extensions/amp-story/1.0/bookend/bookend-component.js b/extensions/amp-story/1.0/bookend/bookend-component.js index a8c2d8ffb9159..b29be394afdd6 100644 --- a/extensions/amp-story/1.0/bookend/bookend-component.js +++ b/extensions/amp-story/1.0/bookend/bookend-component.js @@ -180,7 +180,8 @@ export class BookendComponent { static buildContainer(element, doc) { const html = htmlFor(doc); const containerTemplate = - html`
`; + html`
`; element.appendChild(containerTemplate); return element.lastElementChild; } diff --git a/extensions/amp-story/1.0/localization.js b/extensions/amp-story/1.0/localization.js index ebadc734990d2..6a8876cea8ce4 100644 --- a/extensions/amp-story/1.0/localization.js +++ b/extensions/amp-story/1.0/localization.js @@ -25,7 +25,7 @@ import {parseJson} from '../../../src/json'; * - NOT be reused; to deprecate an ID, comment it out and prefix its key with * the string "DEPRECATED_" * - * Next ID: 34 + * Next ID: 35 * * @const @enum {string} */ @@ -44,6 +44,7 @@ export const LocalizedStringId = { AMP_STORY_DOMAIN_DIALOG_HEADING_LINK: '26', AMP_STORY_HINT_UI_NEXT_LABEL: '2', AMP_STORY_HINT_UI_PREVIOUS_LABEL: '3', + AMP_STORY_PAGE_PLAY_VIDEO: '34', AMP_STORY_SHARING_CLIPBOARD_FAILURE_TEXT: '4', AMP_STORY_SHARING_CLIPBOARD_SUCCESS_TEXT: '5', AMP_STORY_SHARING_PROVIDER_NAME_EMAIL: '6', diff --git a/extensions/amp-story/1.0/test/test-amp-story-system-layer.js b/extensions/amp-story/1.0/test/test-amp-story-system-layer.js index 23c518485026c..a524dfbb9424c 100644 --- a/extensions/amp-story/1.0/test/test-amp-story-system-layer.js +++ b/extensions/amp-story/1.0/test/test-amp-story-system-layer.js @@ -14,6 +14,7 @@ * limitations under the License. */ import {Action, AmpStoryStoreService} from '../amp-story-store-service'; +import {LocalizationService} from '../localization'; import {ProgressBar} from '../progress-bar'; import {Services} from '../../../../src/services'; import {SystemLayer} from '../amp-story-system-layer'; @@ -36,6 +37,9 @@ describes.fakeWin('amp-story system layer', {amp: true}, env => { storeService = new AmpStoryStoreService(win); registerServiceBuilder(win, 'story-store', () => storeService); + const localizationService = new LocalizationService(win); + registerServiceBuilder(win, 'localization', () => localizationService); + progressBarRoot = win.document.createElement('div'); progressBarStub = { diff --git a/extensions/amp-story/1.0/test/test-amp-story.js b/extensions/amp-story/1.0/test/test-amp-story.js index 04c39f56e8acf..d7a4842a08649 100644 --- a/extensions/amp-story/1.0/test/test-amp-story.js +++ b/extensions/amp-story/1.0/test/test-amp-story.js @@ -31,6 +31,7 @@ import {MediaType} from '../media-pool'; import {PageState} from '../amp-story-page'; import {PaginationButtons} from '../pagination-buttons'; import {Services} from '../../../../src/services'; +import {createElementWithAttributes} from '../../../../src/dom'; import {registerServiceBuilder} from '../../../../src/service'; @@ -886,8 +887,6 @@ describes.realWin('amp-story', { return story.layoutCallback() .then(() => { - sandbox.stub(story.element, 'querySelectorAll').returns([]); - const expected = { [MediaType.AUDIO]: 2, [MediaType.VIDEO]: 2, @@ -901,13 +900,15 @@ describes.realWin('amp-story', { return story.layoutCallback() .then(() => { - const qsStub = sandbox.stub(story.element, 'querySelectorAll'); - qsStub.withArgs('amp-audio, [background-audio]').returns(['el']); - qsStub.withArgs('amp-video').returns(['el', 'el']); + const ampVideoEl = win.document.createElement('amp-video'); + const ampAudoEl = createElementWithAttributes(win.document, + 'amp-audio', {'background-audio': ''}); + story.element.appendChild(ampVideoEl); + story.element.appendChild(ampAudoEl); const expected = { [MediaType.AUDIO]: 3, - [MediaType.VIDEO]: 4, + [MediaType.VIDEO]: 3, }; expect(story.getMaxMediaElementCounts()).to.deep.equal(expected); }); @@ -918,11 +919,16 @@ describes.realWin('amp-story', { return story.layoutCallback() .then(() => { - const qsStub = sandbox.stub(story.element, 'querySelectorAll'); - qsStub.withArgs('amp-audio, [background-audio]') - .returns(['el', 'el', 'el', 'el', 'el', 'el', 'el']); - qsStub.withArgs('amp-video') - .returns(['el', 'el', 'el', 'el', 'el', 'el', 'el', 'el']); + for (let i = 0; i < 7; i++) { + const el = createElementWithAttributes(win.document, + 'amp-audio', {'background-audio': ''}); + story.element.appendChild(el); + } + + for (let i = 0; i < 8; i++) { + const el = win.document.createElement('amp-video'); + story.element.appendChild(el); + } const expected = { [MediaType.AUDIO]: 4, diff --git a/extensions/amp-subscriptions-google/0.1/amp-subscriptions-google.js b/extensions/amp-subscriptions-google/0.1/amp-subscriptions-google.js index 3ac801a09b76d..32a4aca42ce58 100644 --- a/extensions/amp-subscriptions-google/0.1/amp-subscriptions-google.js +++ b/extensions/amp-subscriptions-google/0.1/amp-subscriptions-google.js @@ -218,12 +218,13 @@ export class GoogleSubscriptionsPlatform { } /** @override */ - activate(entitlement) { + activate(entitlement, grantEntitlement) { + const best = grantEntitlement || entitlement; // Offers or abbreviated offers may need to be shown depending on // whether the access has been granted and whether user is a subscriber. - if (!entitlement.granted) { + if (!best.granted) { this.runtime_.showOffers({list: 'amp'}); - } else if (!entitlement.isSubscriber()) { + } else if (!best.isSubscriber()) { this.runtime_.showAbbrvOffer({list: 'amp'}); } } diff --git a/extensions/amp-subscriptions-google/0.1/test/test-amp-subscriptions-google.js b/extensions/amp-subscriptions-google/0.1/test/test-amp-subscriptions-google.js index 987732874b8fd..8ae3f6b3e5fc7 100644 --- a/extensions/amp-subscriptions-google/0.1/test/test-amp-subscriptions-google.js +++ b/extensions/amp-subscriptions-google/0.1/test/test-amp-subscriptions-google.js @@ -196,12 +196,32 @@ describes.realWin('amp-subscriptions-google', {amp: true}, env => { it('should show abbrv offer on activate when granted non-subscriber', () => { platform.activate(new Entitlement({service: 'subscribe.google.com', - granted: true, subscribed: true})); + granted: true, grantReason: GrantReason.METERING})); expect(methods.showAbbrvOffer).to.be.calledOnce .calledWithExactly({list: 'amp'}); expect(methods.showOffers).to.not.be.called; }); + it('should override show offers with the grant for subscriber', () => { + const entitlement = new Entitlement({service: 'subscribe.google.com', + granted: false}); + const grantEntitlement = new Entitlement({service: 'local', + granted: true, grantReason: GrantReason.SUBSCRIBER}); + platform.activate(entitlement, grantEntitlement); + expect(methods.showOffers).to.not.be.called; + expect(methods.showAbbrvOffer).to.not.be.called; + }); + + it('should override show offers with the grant non-subscriber', () => { + const entitlement = new Entitlement({service: 'subscribe.google.com', + granted: false}); + const grantEntitlement = new Entitlement({service: 'local', + granted: true, grantReason: GrantReason.METERING}); + platform.activate(entitlement, grantEntitlement); + expect(methods.showOffers).to.not.be.called; + expect(methods.showAbbrvOffer).to.be.calledOnce; + }); + it('should start linking flow when requested', () => { serviceAdapterMock.expects('delegateActionToLocal').never(); callback(callbacks.loginRequest)({linkRequested: true}); diff --git a/extensions/amp-subscriptions/0.1/amp-subscriptions.js b/extensions/amp-subscriptions/0.1/amp-subscriptions.js index 55ca77a862c85..05793631ded28 100644 --- a/extensions/amp-subscriptions/0.1/amp-subscriptions.js +++ b/extensions/amp-subscriptions/0.1/amp-subscriptions.js @@ -37,6 +37,7 @@ import {getMode} from '../../../src/mode'; import {getValueForExpr, tryParseJson} from '../../../src/json'; import {getWinOrigin} from '../../../src/url'; import {installStylesForDoc} from '../../../src/style-installer'; +import {isStoryDocument} from '../../../src/utils/story'; /** @const */ const TAG = 'amp-subscriptions'; @@ -294,8 +295,11 @@ export class SubscriptionService { this.fetchEntitlements_(subscriptionPlatform); } ); - this.startAuthorizationFlow_(); + isStoryDocument(this.ampdoc_).then(isStory => { + // Delegates the platform selection and activation call if is story. + this.startAuthorizationFlow_(!isStory /** doPlatformSelection */); + }); }); return this; } @@ -375,15 +379,15 @@ export class SubscriptionService { } /** - * Renders and opens the dialog using the cached entitlements. + * Selects and activates a platform. */ - renderDialogForSelectedPlatform() { + maybeSelectAndActivatePlatform() { this.initialize_().then(() => { if (this.doesViewerProvideAuth_ || this.platformConfig_['alwaysGrant']) { return; } - this.selectAndActivatePlatform_(false /** sendAnalyticsEvents */); + this.selectAndActivatePlatform_(); }); } @@ -404,26 +408,24 @@ export class SubscriptionService { } /** - * @param {boolean=} sendAnalyticsEvents * @return {!Promise} * @private */ - selectAndActivatePlatform_(sendAnalyticsEvents = true) { + selectAndActivatePlatform_() { const requireValuesPromise = Promise.all([ this.platformStore_.getGrantStatus(), this.platformStore_.selectPlatform(), + this.platformStore_.getGrantEntitlement(), ]); return requireValuesPromise.then(resolvedValues => { const selectedPlatform = resolvedValues[1]; + const grantEntitlement = resolvedValues[2]; const selectedEntitlement = this.platformStore_.getResolvedEntitlementFor( selectedPlatform.getServiceId()); + const bestEntitlement = grantEntitlement || selectedEntitlement; - selectedPlatform.activate(selectedEntitlement); - - if (sendAnalyticsEvents === false) { - return; - } + selectedPlatform.activate(selectedEntitlement, grantEntitlement); this.subscriptionAnalytics_.serviceEvent( SubscriptionAnalyticsEvents.PLATFORM_ACTIVATED, @@ -432,10 +434,10 @@ export class SubscriptionService { this.subscriptionAnalytics_.serviceEvent( SubscriptionAnalyticsEvents.PLATFORM_ACTIVATED_DEPRECATED, selectedPlatform.getServiceId()); - if (selectedEntitlement.granted) { + if (bestEntitlement.granted) { this.subscriptionAnalytics_.serviceEvent( SubscriptionAnalyticsEvents.ACCESS_GRANTED, - selectedPlatform.getServiceId()); + bestEntitlement.service); } else { this.subscriptionAnalytics_.serviceEvent( SubscriptionAnalyticsEvents.PAYWALL_ACTIVATED, @@ -485,10 +487,6 @@ export class SubscriptionService { this.platformStore_ = this.platformStore_.resetPlatformStore(); this.renderer_.toggleLoading(true); - this.platformConfig_['services'].forEach(service => { - this.initializeLocalPlatforms_(service); - }); - this.platformStore_.getAvailablePlatforms().forEach( subscriptionPlatform => { this.fetchEntitlements_(subscriptionPlatform); diff --git a/extensions/amp-subscriptions/0.1/platform-store.js b/extensions/amp-subscriptions/0.1/platform-store.js index dbe27757e0501..71f92df4678ed 100644 --- a/extensions/amp-subscriptions/0.1/platform-store.js +++ b/extensions/amp-subscriptions/0.1/platform-store.js @@ -75,9 +75,6 @@ export class PlatformStore { /** @private {?Deferred} */ this.grantStatusPromise_ = null; - /** @private @const {!Observable} */ - this.onGrantStateResolvedCallbacks_ = new Observable(); - /** @private {?Entitlement} */ this.grantStatusEntitlement_ = null; @@ -115,6 +112,11 @@ export class PlatformStore { * @return {PlatformStore} */ resetPlatformStore() { + // Reset individual platforms to ensure their UX clears. + for (const platformKey in this.subscriptionPlatforms_) { + this.subscriptionPlatforms_[platformKey].reset(); + } + // Then create new platform store withe the newply reset platforms in it. return new PlatformStore( this.serviceIds_, this.scoreConfig_, @@ -206,6 +208,9 @@ export class PlatformStore { this.failedPlatforms_.splice(this.failedPlatforms_.indexOf(serviceId)); } // Call all onChange callbacks. + if (entitlement.granted) { + this.saveGrantEntitlement_(entitlement); + } this.onEntitlementResolvedCallbacks_.fire({serviceId, entitlement}); } @@ -257,7 +262,6 @@ export class PlatformStore { // Listen if any upcoming entitlements unblock the reader this.onChange(({entitlement}) => { if (entitlement.granted) { - this.saveGrantEntitlement_(entitlement); this.grantStatusPromise_.resolve(true); } else if (this.areAllPlatformsResolved_()) { this.grantStatusPromise_.resolve(false); @@ -281,7 +285,6 @@ export class PlatformStore { && !this.grantStatusEntitlement_.isSubscriber() && entitlement.isSubscriber())) { this.grantStatusEntitlement_ = entitlement; - this.onGrantStateResolvedCallbacks_.fire(); } } @@ -299,9 +302,10 @@ export class PlatformStore { || this.areAllPlatformsResolved_()) { this.grantStatusEntitlementPromise_.resolve(this.grantStatusEntitlement_); } else { - this.onGrantStateResolvedCallbacks_.add(() => { + this.onEntitlementResolvedCallbacks_.add(() => { // Grant entitlement only if subscriber - if (this.grantStatusEntitlement_.isSubscriber() + if ((this.grantStatusEntitlement_ + && this.grantStatusEntitlement_.isSubscriber()) || this.areAllPlatformsResolved_()) { this.grantStatusEntitlementPromise_.resolve( this.grantStatusEntitlement_); diff --git a/extensions/amp-subscriptions/0.1/subscription-platform.js b/extensions/amp-subscriptions/0.1/subscription-platform.js index 11b5c7798fa6f..408787f163847 100644 --- a/extensions/amp-subscriptions/0.1/subscription-platform.js +++ b/extensions/amp-subscriptions/0.1/subscription-platform.js @@ -40,11 +40,14 @@ export class SubscriptionPlatform { * Activates the subscription platform and hands over the control for * rendering. * @param {!./entitlement.Entitlement} unusedEntitlement + * @param {?./entitlement.Entitlement} unusedGrantEntitlement */ - activate(unusedEntitlement) {} + activate(unusedEntitlement, unusedGrantEntitlement) {} /** * Reset the platform and renderer. + * This should clear dialogs and toasts originating + * from the platform. */ reset() {} diff --git a/extensions/amp-subscriptions/0.1/test/test-amp-subscriptions.js b/extensions/amp-subscriptions/0.1/test/test-amp-subscriptions.js index 3e6478849e5ad..80108c6f3ceea 100644 --- a/extensions/amp-subscriptions/0.1/test/test-amp-subscriptions.js +++ b/extensions/amp-subscriptions/0.1/test/test-amp-subscriptions.js @@ -14,6 +14,7 @@ * limitations under the License. */ +import * as utilsStory from '../../../../src/utils/story'; import {Entitlement, GrantReason} from '../entitlement'; import {LocalSubscriptionPlatform} from '../local-subscription-platform'; import { @@ -38,6 +39,7 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => { let subscriptionService; let configResolver; let analyticsEventStub; + let isStory; const products = ['scenic-2017.appspot.com:news', 'scenic-2017.appspot.com:product2']; @@ -84,6 +86,11 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => { subscriptionService.subscriptionAnalytics_, 'event' ); + // isStoryDocument needs to resolve synchronously because of how some of the + // tests are built. + isStory = false; + sandbox.stub(utilsStory, 'isStoryDocument') + .returns({then: fn => fn(isStory)}); }); it('should call `initialize_` on start', () => { @@ -172,6 +179,24 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => { expect(processStateStub).to.not.be.called; }); }); + + it('should delay the platform selection and activation if story', () => { + isStory = true; + + const processStateStub = sandbox.stub(subscriptionService, + 'processGrantState_'); + const authFlowStub = sandbox.stub(subscriptionService, + 'startAuthorizationFlow_'); + const delegateStub = sandbox.stub(subscriptionService, + 'delegateAuthToViewer_'); + subscriptionService.start(); + return subscriptionService.initialize_().then(() => { + expect(authFlowStub.withArgs(false /** doPlatformActivation*/)) + .to.be.calledOnce; + expect(delegateStub).to.not.be.called; + expect(processStateStub).to.not.be.called; + }); + }); }); describe('getReaderId', () => { @@ -293,12 +318,43 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => { }); describe('selectAndActivatePlatform_', () => { - it('should wait for grantStatus and selectPlatform promise', () => { + function resolveRequiredPromises(entitlementSpec, grantEntitlementSpec) { + entitlementSpec = Object.assign({ + service: 'local', + source: 'local', + raw: 'raw', + }, entitlementSpec); + if (!grantEntitlementSpec && entitlementSpec.granted) { + grantEntitlementSpec = entitlementSpec; + } + const entitlement = new Entitlement(entitlementSpec); + const grantEntitlement = grantEntitlementSpec ? + new Entitlement(grantEntitlementSpec) : null; + const granted = !!grantEntitlementSpec; + const localPlatform = + subscriptionService.platformStore_.getLocalPlatform(); + sandbox.stub(subscriptionService.platformStore_, 'getGrantStatus') + .callsFake(() => Promise.resolve(granted)); + sandbox.stub(subscriptionService.platformStore_, 'getGrantEntitlement') + .callsFake(() => Promise.resolve(grantEntitlement)); + subscriptionService.platformStore_.resolveEntitlement( + entitlementSpec.source, + entitlement); + sandbox.stub( + subscriptionService.platformStore_, + 'selectPlatform' + ).callsFake(() => Promise.resolve(localPlatform)); + } + + it('should wait for grantStatus/ent and selectPlatform promise', () => { sandbox.stub(subscriptionService, 'fetchEntitlements_'); subscriptionService.start(); subscriptionService.viewTrackerPromise_ = Promise.resolve(); return subscriptionService.initialize_().then(() => { - resolveRequiredPromises(subscriptionService, /* subscriber */ true); + resolveRequiredPromises({ + granted: true, + grantReason: GrantReason.SUBSCRIBER, + }); const localPlatform = subscriptionService.platformStore_.getLocalPlatform(); const selectPlatformStub = @@ -307,6 +363,8 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => { expect(localPlatform).to.be.not.null; return subscriptionService.selectAndActivatePlatform_().then(() => { expect(activateStub).to.be.calledOnce; + expect(activateStub.firstCall.args[0].source).to.equal('local'); + expect(activateStub.firstCall.args[1].source).to.equal('local'); expect(selectPlatformStub).to.be.called; expect(analyticsEventStub).to.be.calledWith( SubscriptionAnalyticsEvents.PLATFORM_ACTIVATED, @@ -326,12 +384,57 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => { }); }); + it('should activate with a different grant entitlement', () => { + sandbox.stub(subscriptionService, 'fetchEntitlements_'); + subscriptionService.start(); + subscriptionService.viewTrackerPromise_ = Promise.resolve(); + return subscriptionService.initialize_().then(() => { + resolveRequiredPromises({ + granted: false, + }, { + service: 'other', + source: 'other', + granted: true, + grantReason: GrantReason.SUBSCRIBER, + }); + const localPlatform = + subscriptionService.platformStore_.getLocalPlatform(); + const selectPlatformStub = + subscriptionService.platformStore_.selectPlatform; + const activateStub = sandbox.stub(localPlatform, 'activate'); + expect(localPlatform).to.be.not.null; + return subscriptionService.selectAndActivatePlatform_().then(() => { + expect(activateStub).to.be.calledOnce; + expect(activateStub.firstCall.args[0].source).to.equal('local'); + expect(activateStub.firstCall.args[1].source).to.equal('other'); + expect(selectPlatformStub).to.be.called; + expect(analyticsEventStub).to.be.calledWith( + SubscriptionAnalyticsEvents.PLATFORM_ACTIVATED, + { + 'serviceId': 'local', + } + ); + expect(analyticsEventStub).to.be.calledWith( + SubscriptionAnalyticsEvents.ACCESS_GRANTED, + { + 'serviceId': 'other', + } + ); + expect(analyticsEventStub).to.not.be.calledWith( + SubscriptionAnalyticsEvents.PAYWALL_ACTIVATED); + }); + }); + }); + it('should call selectPlatform with preferViewerSupport config', () => { sandbox.stub(subscriptionService, 'fetchEntitlements_'); subscriptionService.start(); subscriptionService.viewTrackerPromise_ = Promise.resolve(); return subscriptionService.initialize_().then(() => { - resolveRequiredPromises(subscriptionService, /* subscriber */ true); + resolveRequiredPromises({ + granted: true, + grantReason: GrantReason.SUBSCRIBER, + }); const selectPlatformStub = subscriptionService.platformStore_.selectPlatform; subscriptionService.platformConfig_['preferViewerSupport'] = false; @@ -346,7 +449,7 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => { subscriptionService.start(); subscriptionService.viewTrackerPromise_ = Promise.resolve(); return subscriptionService.initialize_().then(() => { - resolveRequiredPromises(subscriptionService, /* subscriber */ false); + resolveRequiredPromises({granted: false}); const localPlatform = subscriptionService.platformStore_.getLocalPlatform(); sandbox.stub(localPlatform, 'activate'); @@ -370,25 +473,6 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, env => { }); }); }); - - function resolveRequiredPromises(subscriptionService, subscriber) { - const entitlement = new Entitlement({ - source: 'local', - raw: 'raw', - granted: subscriber, - grantReason: subscriber ? GrantReason.SUBSCRIBER : null, - }); - const localPlatform = - subscriptionService.platformStore_.getLocalPlatform(); - sandbox.stub(subscriptionService.platformStore_, 'getGrantStatus') - .callsFake(() => Promise.resolve()); - subscriptionService.platformStore_.resolveEntitlement('local', - entitlement); - sandbox.stub( - subscriptionService.platformStore_, - 'selectPlatform' - ).callsFake(() => Promise.resolve(localPlatform)); - } }); describe('startAuthorizationFlow_', () => { diff --git a/extensions/amp-subscriptions/0.1/test/test-dialog.js b/extensions/amp-subscriptions/0.1/test/test-dialog.js index 7f31127416325..e8e5cd09cefc5 100644 --- a/extensions/amp-subscriptions/0.1/test/test-dialog.js +++ b/extensions/amp-subscriptions/0.1/test/test-dialog.js @@ -67,7 +67,7 @@ describes.realWin('AmpSubscriptions Dialog', {amp: true}, env => { const styles = getComputedStyle(dialog.getRoot()); expect(styles.transform).to.contain('17'); return promise; - }).then(() => { + }).then(() => vsync.mutatePromise(() => {})).then(() => { expect(dialog.getRoot()).to.have.display('block'); const styles = getComputedStyle(dialog.getRoot()); expect(styles.transform).to.not.contain('17'); @@ -81,9 +81,10 @@ describes.realWin('AmpSubscriptions Dialog', {amp: true}, env => { const content2 = createElementWithAttributes(doc, 'div', { style: 'height:21px', }); - return dialog.open(content, false).then(() => { - expect(content.parentNode).to.equal(dialog.getRoot()); - return dialog.open(content2, false); + const promise = dialog.open(content2, false); + return vsync.mutatePromise(() => {}).then(() => { + expect(content2.parentNode).to.equal(dialog.getRoot()); + return promise; }).then(() => { expect(content2.parentNode).to.equal(dialog.getRoot()); expect(content.parentNode).to.be.null; diff --git a/extensions/amp-subscriptions/0.1/test/test-platform-store.js b/extensions/amp-subscriptions/0.1/test/test-platform-store.js index 2d4c1407647fa..326a673af4cba 100644 --- a/extensions/amp-subscriptions/0.1/test/test-platform-store.js +++ b/extensions/amp-subscriptions/0.1/test/test-platform-store.js @@ -508,55 +508,60 @@ describes.realWin('Platform store', {}, () => { }); describe('getGrantEntitlement', () => { - const subscribedMeteredEntitlement = new Entitlement({ + const subscribedEntitlement = new Entitlement({ source: 'local', service: 'local', granted: true, grantReason: GrantReason.SUBSCRIBER, }); + const meteringEntitlement = new Entitlement({ + source: 'local', + service: 'local', + granted: true, + grantReason: GrantReason.METERING, + }); + const noEntitlement = new Entitlement({ + source: 'local', + service: 'local', + granted: false, + }); + it('should resolve with existing entitlement with subscriptions', () => { - platformStore.grantStatusEntitlement_ = subscribedMeteredEntitlement; + platformStore.grantStatusEntitlement_ = subscribedEntitlement; return platformStore.getGrantEntitlement().then(entitlement => { - expect(entitlement.json()).to.deep.equal( - subscribedMeteredEntitlement.json()); + expect(entitlement).to.equal(subscribedEntitlement); }); }); it('should resolve with first entitlement with subscriptions', () => { - const meteringEntitlement = new Entitlement({ - source: 'local', - service: 'local', - granted: true, - data: { - metering: { - 'left': 5, - 'total': 10, - 'token': 'token', - }, - }, - }); - platformStore.grantStatusEntitlement_ = meteringEntitlement; - platformStore.saveGrantEntitlement_(subscribedMeteredEntitlement); + platformStore.resolveEntitlement('service1', subscribedEntitlement); return platformStore.getGrantEntitlement().then(entitlement => { - expect(entitlement.json()).to.deep.equal( - subscribedMeteredEntitlement.json()); + expect(entitlement).to.equal(subscribedEntitlement); }); }); it('should resolve with metered entitlement when no ' + 'platform is subscribed', () => { - const meteringEntitlement = new Entitlement({ - source: 'local', - service: 'local', - granted: true, - grantReason: GrantReason.METERING, + platformStore.resolveEntitlement('service1', noEntitlement); + platformStore.resolveEntitlement('service2', meteringEntitlement); + return platformStore.getGrantEntitlement().then(entitlement => { + expect(entitlement).to.equal(meteringEntitlement); }); - sandbox.stub(platformStore, 'areAllPlatformsResolved_') - .callsFake(() => true); - platformStore.saveGrantEntitlement_(meteringEntitlement); + }); + + it('should override metering with subscription', () => { + platformStore.resolveEntitlement('service1', meteringEntitlement); + platformStore.resolveEntitlement('service2', subscribedEntitlement); + return platformStore.getGrantEntitlement().then(entitlement => { + expect(entitlement).to.equal(subscribedEntitlement); + }); + }); + + it('should resolve to null if nothing matched', () => { + platformStore.resolveEntitlement('service1', noEntitlement); + platformStore.resolveEntitlement('service2', noEntitlement); return platformStore.getGrantEntitlement().then(entitlement => { - expect(entitlement.json()).to.deep.equal( - meteringEntitlement.json()); + expect(entitlement).to.be.null; }); }); }); diff --git a/extensions/amp-subscriptions/0.1/url-builder.js b/extensions/amp-subscriptions/0.1/url-builder.js index ee37033cddabc..7222a18522db6 100644 --- a/extensions/amp-subscriptions/0.1/url-builder.js +++ b/extensions/amp-subscriptions/0.1/url-builder.js @@ -19,14 +19,16 @@ import {getValueForExpr} from '../../../src/json'; export class UrlBuilder { - /** * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc * @param {!Promise} readerIdPromise */ constructor(ampdoc, readerIdPromise) { + const {documentElement} = ampdoc.win.document; + /** @private @const {!../../../src/service/url-replacements-impl.UrlReplacements} */ - this.urlReplacements_ = Services.urlReplacementsForDoc(ampdoc); + this.urlReplacements_ = Services.urlReplacementsForDoc(documentElement); + /** @private @const {!Promise} */ this.readerIdPromise_ = readerIdPromise; diff --git a/extensions/amp-user-notification/0.1/amp-user-notification.js b/extensions/amp-user-notification/0.1/amp-user-notification.js index 87477eaec0eee..acf7ae048e3af 100644 --- a/extensions/amp-user-notification/0.1/amp-user-notification.js +++ b/extensions/amp-user-notification/0.1/amp-user-notification.js @@ -138,7 +138,7 @@ export class AmpUserNotification extends AMP.BaseElement { /** @override */ buildCallback() { const ampdoc = this.getAmpDoc(); - this.urlReplacements_ = Services.urlReplacementsForDoc(ampdoc); + this.urlReplacements_ = Services.urlReplacementsForDoc(this.element); this.storagePromise_ = Services.storageForDoc(ampdoc); this.elementId_ = user().assert(this.element.id, diff --git a/extensions/amp-video-docking/0.1/amp-video-docking.js b/extensions/amp-video-docking/0.1/amp-video-docking.js index f088ebeb5f99e..00917425d3534 100644 --- a/extensions/amp-video-docking/0.1/amp-video-docking.js +++ b/extensions/amp-video-docking/0.1/amp-video-docking.js @@ -15,6 +15,8 @@ */ import {ActionTrust} from '../../../src/action-constants'; import {CSS} from '../../../build/amp-video-docking-0.1.css'; +import {Controls} from './controls'; +import {HtmlLiteralTagDef} from './html'; import { PlayingStates, VideoAttributes, @@ -29,9 +31,11 @@ import { PositionObserverFidelity, } from '../../../src/service/position-observer/position-observer-worker'; import {Services} from '../../../src/services'; +import {Timeout} from './timeout'; +import {VideoDockingEvents} from './events'; +import {applyBreakpointClassname} from './breakpoints'; import { childElementByTag, - closestBySelector, escapeCssSelectorIdent, isRTL, removeElement, @@ -46,7 +50,7 @@ import {dev, user} from '../../../src/log'; import {dict} from '../../../src/utils/object'; import {getInternalVideoElementFor} from '../../../src/utils/video'; import {getServiceForDoc} from '../../../src/service'; -import {htmlFor, htmlRefs} from '../../../src/static-template'; +import {htmlFor} from '../../../src/static-template'; import {installStylesForDoc} from '../../../src/style-installer'; import {isExperimentOn} from '../../../src/experiments'; import {isFiniteNumber} from '../../../src/types'; @@ -59,7 +63,6 @@ import { setImportantStyles, setStyles, toggle, - translate, } from '../../../src/style'; import {urls} from '../../../src/config'; @@ -79,12 +82,6 @@ const MIN_VIEWPORT_WIDTH = 320; /** @private @const {number} */ const DOCKING_TIMEOUT = 100; -/** @private @const {number} */ -const CONTROLS_TIMEOUT = 1600; - -/** @private @const {number} */ -const CONTROLS_TIMEOUT_AFTER_IX = 1000; - /** @private @const {number} */ const FLOAT_TOLERANCE = 0.02; @@ -118,21 +115,6 @@ export const Actions = {DOCK: 'dock', UNDOCK: 'undock'}; let DockedDef; -/** - * @struct @typedef {{ - * container: !Element, - * dismissButton: !Element, - * playButton: !Element, - * pauseButton: !Element, - * muteButton: !Element, - * unmuteButton: !Element, - * fullscreenButton: !Element, - * dismissContainer: !Element, - * }} - */ -let ControlsDef; - - /** @typedef {{posX: !RelativeX, posY: !RelativeY}|!Element} */ let DockTargetDef; @@ -157,16 +139,6 @@ let TargetAreaDef; const transform = (x, y, scale) => `translate(${x}px, ${y}px) scale(${scale})`; -/** - * @param {!Element} a - * @param {!Element} b - * @private - */ -function swap(a, b) { - toggle(a, false); - toggle(b, true); -} - /** * @param {!Window} win @@ -229,27 +201,7 @@ function complainAboutPortrait(element) { } -/** - * @param {!Element} element - * @param {number} width - * @param {!Array} breakpoints - */ -function applyBreakpointClassname(element, width, breakpoints) { - // sort by minWidth descending - breakpoints = breakpoints.sort((a, b) => b.minWidth - a.minWidth); - - let maxBreakpoint = -1; - for (let i = 0; i < breakpoints.length; i++) { - const {className, minWidth} = breakpoints[i]; - if (minWidth <= width && - minWidth > maxBreakpoint) { - element.classList.add(className); - maxBreakpoint = minWidth; - } else { - element.classList.remove(className); - } - } -} + // Function should ideally be in `dom.js`, but moving it causes a bunch of ads @@ -264,13 +216,6 @@ export function isElement(obj) { } -/** - * See `src/static-template.js`. - * @typedef {function(!Array):!Element} - */ -let HtmlLiteralTagDef; - - /** * @param {!HtmlLiteralTagDef} html * @return {!Element} @@ -280,15 +225,6 @@ const ShadowLayer = html => html``; -/** - * @param {!HtmlLiteralTagDef} html - * @return {!Element} - * @private - */ -const DockedOverlay = html => - html``; - - /** * @param {!HtmlLiteralTagDef} html * @return {!Element} @@ -304,61 +240,7 @@ const PlaceholderBackground = html => /** - * @param {!HtmlLiteralTagDef} html - * @return {!Element} - * @private - */ -const Controls = html => - // This currently bloats the resulting binary with - // 1. some whitespace and 2. duplicate declarations of equal strings. - // Upcoming fixes: #14657, #14658. - // TODO(alanorozco): Cleanup markup for readability once fixes land. - html``; - - -/** @typedef {{className: string, minWidth: number}} */ -let SyntheticBreakpointDef; - -/** - * @private @const {!Array} - */ -const CONTROLS_BREAKPOINTS = [ - { - className: 'amp-small', - minWidth: 0, - }, - { - className: 'amp-large', - minWidth: 300, - }, -]; - -/** - * @private @const {!Array} + * @private @const {!Array} */ const PLACEHOLDER_ICON_BREAKPOINTS = [ { @@ -379,46 +261,6 @@ const PLACEHOLDER_ICON_SMALL_WIDTH = 32; const PLACEHOLDER_ICON_SMALL_MARGIN = 20; -/** Timeout that can be postponed, repeated or cancelled. */ -class Timeout { - /** - * @param {!Window} win - * @param {!Function} handler - */ - constructor(win, handler) { - /** @private @const {!../../../src/service/timer-impl.Timer} */ - this.timer_ = Services.timerFor(win); - - /** @private @const {!Function} */ - this.handler_ = handler; - - /** @private {?number|?string} */ - this.id_ = null; - } - - /** - * @param {number} time - * @param {...*} args - */ - trigger(time, ...args) { - this.cancel(); - this.id_ = this.timer_.delay(() => this.handler_.apply(null, args), time); - } - - /** @public */ - cancel() { - if (this.id_ !== null) { - this.timer_.cancel(this.id_); - this.id_ = null; - } - } - - /** @return {boolean} */ - isWaiting() { - return this.id_ !== null; - } -} - /** * Manages docking (a.k.a. minimize to corner) for videos that satisfy the @@ -453,10 +295,6 @@ export class VideoDocking { /** @type {!../../../src/video-interface.VideoOrBaseElementDef} */ ( video))); - /** @private @const {function():!Timeout} */ - this.getHideControlsTimeout_ = this.lazyTimeout_(() => - this.hideControls_(/* respectSticky */ true)); - /** @private @const {function():!Timeout} */ this.getUndockingTimeout_ = this.lazyTimeout_(video => this.undock_( @@ -482,14 +320,8 @@ export class VideoDocking { */ this.getShadowLayer_ = once(() => this.append_(ShadowLayer(html))); - /** - * Returns an overlay to be used to capture different user events. - * @private @const {function():!Element} - */ - this.getOverlay_ = once(() => this.installOverlay_(DockedOverlay(html))); - - /** @private @const {function():!ControlsDef} */ - this.getControls_ = once(() => this.installControls_(Controls(html))); + /** @private @const {function():!Controls} */ + this.getControls_ = once(() => this.installControls_()); /** @private @const {function():!Element} */ this.getPlaceholderBackground_ = @@ -507,13 +339,7 @@ export class VideoDocking { this.lastDismissedPosY_ = null; /** - * Unlisteners for the currently minimized video. - * @private {!Array} - */ - this.videoUnlisteners_ = []; - - /** - * Memoizes x, y and scale to prevent useless mutations. + * Memoizes x, y and scale to prevent useless mutations. * @private {?{x: number, y: number, scale: number}} */ this.placedAt_ = null; @@ -527,18 +353,12 @@ export class VideoDocking { /** @private {number} */ this.lastScrollTop_ = this.viewport_.getScrollTop(); - /** @private {boolean} */ - this.stickyControls_ = false; - /** @private {boolean} */ this.isDragging_ = false; /** @private {!Array} */ this.observed_ = []; - /** @private {boolean} */ - this.isTransitioning_ = false; - /** @private @const {!function()} */ // Lazily invoked. this.install_ = once(() => { @@ -554,10 +374,6 @@ export class VideoDocking { /** @private @const {function():?Element} */ this.getSlot_ = once(() => this.querySlot_()); - /** @private */ - this.hideControlsOnTapOutsideOnce_ = - once(() => this.hideControlsOnTapOutside_()); - const dockableSelector = `[${escapeCssSelectorIdent(VideoAttributes.DOCK)}]`; @@ -687,89 +503,6 @@ export class VideoDocking { return dev().assertElement(root.appendChild(element)); } - /** - * @param {!Element} overlay - * * @return {!Element} - * @private - */ - installOverlay_(overlay) { - this.append_(overlay); - return this.installShowControlsOnTapOrHover_( - this.addDragListeners_(overlay)); - } - - /** @private */ - enterFullscreen_() { - const video = this.getDockedVideo_(); - video.fullscreenEnter(); - } - - /** - * @param {!Element} element - * @return {!Element} - * @private - */ - installShowControlsOnTapOrHover_(element) { - const onTapOrHover = () => this.showControlsOnTapOrHover_(); - - listen(element, 'mouseup', onTapOrHover); - listen(element, 'mouseover', onTapOrHover); - - return element; - } - - /** @private */ - showControlsOnTapOrHover_() { - if (this.isDragging_) { - return; - } - if (this.isTransitioning_) { - return; - } - const video = this.getDockedVideo_(); - const { - container, - playButton, - pauseButton, - muteButton, - unmuteButton, - } = this.getControls_(); - const overlay = this.getOverlay_(); - - toggle(container, true); - container.classList.add('amp-video-docked-controls-shown'); - overlay.classList.add('amp-video-docked-controls-bg'); - - if (this.isPlaying_()) { - swap(playButton, pauseButton); - } else { - swap(pauseButton, playButton); - } - - if (this.manager_().isMuted( - /** @type {!../../../src/video-interface.VideoInterface} */ - (video))) { - swap(muteButton, unmuteButton); - } else { - swap(unmuteButton, muteButton); - } - - this.hideControlsOnTimeout_(); - } - - /** @private */ - hideControlsOnTapOutside_() { - listen(this.ampdoc_.getRootNode(), 'mousedown', e => { - if (!this.currentlyDocked_) { - return; - } - if (this.isControlsEventTarget_(dev().assertElement(e.target))) { - return; - } - this.hideControls_(/* respectSticky */ true); - }); - } - /** * @param {!Element} element * @return {!Element} @@ -785,73 +518,29 @@ export class VideoDocking { } /** - * @param {!Element} container - * @return {!ControlsDef} + * @return {!Controls} * @private */ - installControls_(container) { - const controls = htmlRefs(container); - - const { - dismissButton, - playButton, - pauseButton, - unmuteButton, - muteButton, - fullscreenButton, - } = controls; - - Object.assign(controls, {container}); + installControls_() { + const controls = new Controls(this.ampdoc_); + const {container, overlay} = controls; - this.listenWhenNotDragging_(dismissButton, 'click', () => { + listen(container, VideoDockingEvents.DISMISS_ON_TAP, () => { this.dismissOnTap_(); }); - this.listenWhenNotDragging_(playButton, 'click', () => { - this.getDockedVideo_().play(/* auto */ false); - }); - - this.listenWhenNotDragging_(pauseButton, 'click', () => { - this.getDockedVideo_().pause(); - }); - - this.listenWhenNotDragging_(muteButton, 'click', () => { - this.getDockedVideo_().mute(); - }); - - this.listenWhenNotDragging_(unmuteButton, 'click', () => { - this.getDockedVideo_().unmute(); - }); - - this.listenWhenNotDragging_(fullscreenButton, 'click', () => { - this.enterFullscreen_(); - }); - - listen(container, 'mouseup', () => - this.hideControlsOnTimeout_(CONTROLS_TIMEOUT_AFTER_IX)); - this.addDragListeners_(container); - this.append_(container); + this.addDragListeners_(overlay); - return /** @type {!ControlsDef} */ (controls); - } + this.append_(container); + this.append_(overlay); - /** - * @param {!Element} element - * @param {string} eventType - * @param {function(!Event)} handler - */ - listenWhenNotDragging_(element, eventType, handler) { - listen(element, eventType, e => { - if (this.isDragging_) { - return; - } - handler(e); - }); + return controls; } /** @private */ dismissOnTap_() { + this.getControls_().hide(/* respectSticky */ false, /* immediately */ true); this.undock_(this.getDockedVideo_()); } @@ -1420,7 +1109,7 @@ export class VideoDocking { const internalElement = getInternalVideoElementFor(element); const shadowLayer = this.getShadowLayer_(); - const overlay = this.getOverlay_(); + const {overlay} = this.getControls_(); const placeholderIcon = this.getPlaceholderIcon_(); applyBreakpointClassname(placeholderIcon, width, @@ -1470,7 +1159,7 @@ export class VideoDocking { const placeholderIconX = step * (width - placeholderIconWidth - placeholderIconMargin * 2); - this.isTransitioning_ = true; + this.getControls_().disable(); video.mutateElement(() => { internalElement.classList.add(BASE_CLASS_NAME); @@ -1504,12 +1193,14 @@ export class VideoDocking { setOpacity(shadowLayer); - this.positionControlsOnVsync_(scale, x, y, width, height); + this.getControls_().positionOnVsync(scale, x, y, width, height); }); return listenOncePromise(internalElement, 'transitionend') .then(() => this.maybeUpdateStaleYAfterScroll_(video, step)) - .then(() => this.isTransitioning_ = false); + .then(() => { + this.getControls_().enable(); + }); } /** @@ -1561,7 +1252,7 @@ export class VideoDocking { return [ getInternalVideoElementFor(video.element), this.getShadowLayer_(), - this.getOverlay_(), + this.getControls_().overlay, ]; } @@ -1602,35 +1293,6 @@ export class VideoDocking { }); } - /** - * @param {number} scale - * @param {number} x - * @param {number} y - * @param {number} width - * @param {number} height - * @private - */ - positionControlsOnVsync_(scale, x, y, width, height) { - const {container, dismissContainer} = this.getControls_(); - const halfScale = scale / 2; - const centerX = x + (width * halfScale); - const centerY = y + (height * halfScale); - - applyBreakpointClassname(container, scale * width, CONTROLS_BREAKPOINTS); - - setImportantStyles(container, { - 'transform': translate(centerX, centerY), - }); - - const dismissMargin = 4; - const dismissWidth = 40; - const dismissX = width * halfScale - dismissMargin - dismissWidth; - const dismissY = -(height * halfScale - dismissMargin - dismissWidth); - setImportantStyles(dismissContainer, { - 'transform': translate(dismissX, dismissY), - }); - } - /** * @param {number} width * @param {number} height @@ -1662,56 +1324,9 @@ export class VideoDocking { * @param {number} step */ setCurrentlyDocked_(video, target, step) { - if (!this.isCurrentlyDocked_(video)) { - this.updateControlsBasedOn_(video.element); - } - const {triggeredDock} = this.currentlyDocked_ || {triggeredDock: false}; this.currentlyDocked_ = {video, target, step, triggeredDock}; - - this.hideControlsOnTapOutsideOnce_(); - } - - /** - * @param {!Element} video - * @private - */ - updateControlsBasedOn_(video) { - while (this.videoUnlisteners_.length) { - this.videoUnlisteners_.pop().call(); - } - this.videoUnlisteners_ = [ - listen(video, VideoEvents.PLAYING, () => this.onPlay_()), - listen(video, VideoEvents.PAUSE, () => this.onPause_()), - listen(video, VideoEvents.MUTED, () => this.onMute_()), - listen(video, VideoEvents.UNMUTED, () => this.onUnmute_()), - ]; - } - - /** @private */ - onPlay_() { - const {playButton, pauseButton} = this.getControls_(); - this.stickyControls_ = false; - swap(playButton, pauseButton); - } - - /** @private */ - onPause_() { - const {pauseButton, playButton} = this.getControls_(); - this.stickyControls_ = true; - swap(pauseButton, playButton); - } - - /** @private */ - onMute_() { - const {muteButton, unmuteButton} = this.getControls_(); - swap(muteButton, unmuteButton); - } - - /** @private */ - onUnmute_() { - const {unmuteButton, muteButton} = this.getControls_(); - swap(unmuteButton, muteButton); + this.getControls_().setVideo(video); } /** @@ -1870,8 +1485,9 @@ export class VideoDocking { e.preventDefault(); e.stopPropagation(); - this.hideControls_(); + this.getControls_().hide(/* respectSticky */ false, /* immediately */ true); this.isDragging_ = true; + this.getControls_().disable(); this.offset_(offset.x, offset.y); this.updateDismissalAreaStyling_(offset.x, offset.y); } @@ -1890,7 +1506,7 @@ export class VideoDocking { video.mutateElement(() => { const className = 'amp-video-docked-almost-dismissed'; internalElement.classList.toggle(className, inDismissalArea); - this.getOverlay_().classList.toggle(className, inDismissalArea); + this.getControls_().overlay.classList.toggle(className, inDismissalArea); }); } @@ -1919,6 +1535,8 @@ export class VideoDocking { this.isDragging_ = false; + this.getControls_().enable(); + if (this.dismissOnDragEnd_(offset.x, offset.y)) { return; } @@ -2020,15 +1638,6 @@ export class VideoDocking { this.placeAt_(video, x, y, scale, step, /* optTransitionDurationMs */ 200); } - /** - * @param {!Element} target - * @return {boolean} - * @private - */ - isControlsEventTarget_(target) { - return !!closestBySelector(target, '.amp-video-docked-controls'); - } - /** * @param {!../../../src/video-interface.VideoOrBaseElementDef} video * @param {!DockTargetDef} target @@ -2190,25 +1799,26 @@ export class VideoDocking { const internalElement = getInternalVideoElementFor(element); return video.mutateElement(() => { - this.hideControls_(); video.showControls(); internalElement.classList.remove(BASE_CLASS_NAME); const shadowLayer = this.getShadowLayer_(); - const overlay = this.getOverlay_(); + const {overlay} = this.getControls_(); const almostDismissed = 'amp-video-docked-almost-dismissed'; const placeholderIcon = this.getPlaceholderIcon_(); const placeholderBackground = this.getPlaceholderBackground_(); + + // TODO(alanorozco): Remove weird flick-to-dismiss. internalElement.classList.remove(almostDismissed); overlay.classList.remove(almostDismissed); toggle(shadowLayer, false); - toggle(overlay, false); + + this.getControls_().reset(); [ element, internalElement, shadowLayer, - overlay, placeholderBackground, placeholderIcon, ].forEach(el => { @@ -2228,28 +1838,6 @@ export class VideoDocking { }); } - /** - * @param {boolean=} respectSticky - * @private - */ - hideControls_(respectSticky = false) { - if (respectSticky && this.stickyControls_) { - return; - } - const {container} = this.getControls_(); - const overlay = this.getOverlay_(); - overlay.classList.remove('amp-video-docked-controls-bg'); - container.classList.remove('amp-video-docked-controls-shown'); - } - - /** - * @param {number=} time - * @private - */ - hideControlsOnTimeout_(time = CONTROLS_TIMEOUT) { - this.getHideControlsTimeout_().trigger(time); - } - /** * @param {!Element} parent * @private diff --git a/extensions/amp-video-docking/0.1/breakpoints.js b/extensions/amp-video-docking/0.1/breakpoints.js new file mode 100644 index 0000000000000..fe257002efcf2 --- /dev/null +++ b/extensions/amp-video-docking/0.1/breakpoints.js @@ -0,0 +1,40 @@ +/** + * Copyright 2018 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. + */ + +/** @typedef {{className: string, minWidth: number}} */ +export let SyntheticBreakpointDef; + + +/** + * @param {!Element} element + * @param {number} width + * @param {!Array} breakpoints + */ +export function applyBreakpointClassname(element, width, breakpoints) { + // sort by minWidth descending + breakpoints = breakpoints.sort((a, b) => b.minWidth - a.minWidth); + + let maxBreakpoint = -1; + breakpoints.forEach(({className, minWidth}) => { + if (minWidth <= width && + minWidth > maxBreakpoint) { + element.classList.add(className); + maxBreakpoint = minWidth; + } else { + element.classList.remove(className); + } + }); +} diff --git a/extensions/amp-video-docking/0.1/controls.js b/extensions/amp-video-docking/0.1/controls.js new file mode 100644 index 0000000000000..e7692fc4050ca --- /dev/null +++ b/extensions/amp-video-docking/0.1/controls.js @@ -0,0 +1,438 @@ +/** + * Copyright 2018 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 {HtmlLiteralTagDef} from './html'; +import { + PlayingStates, + VideoEvents, +} from '../../../src/video-interface'; +import {Services} from '../../../src/services'; +import {Timeout} from './timeout'; +import {VideoDockingEvents} from './events'; +import {applyBreakpointClassname} from './breakpoints'; +import {closestBySelector} from '../../../src/dom'; +import { + createCustomEvent, + listen, +} from '../../../src/event-helper'; +import {dev} from '../../../src/log'; +import {htmlFor, htmlRefs} from '../../../src/static-template'; +import {once} from '../../../src/utils/function'; +import { + resetStyles, + setImportantStyles, + toggle, + translate, +} from '../../../src/style'; + + +/** @private @const {!Array} */ +const BREAKPOINTS = [ + { + className: 'amp-small', + minWidth: 0, + }, + { + className: 'amp-large', + minWidth: 300, + }, +]; + + +const TIMEOUT = 1000; +const TIMEOUT_AFTER_INTERACTION = 1000; + + +/** + * @param {!Element} a + * @param {!Element} b + * @private + */ +function swap(a, b) { + toggle(a, false); + toggle(b, true); +} + + +/** + * @param {!HtmlLiteralTagDef} html + * @return {!Element} + * @private + */ +const renderDockedOverlay = html => + html``; + + +/** + * @param {!HtmlLiteralTagDef} html + * @return {!Element} + * @private + */ +const renderControls = html => + // This currently bloats the resulting binary with + // 1. some whitespace and 2. duplicate declarations of equal strings. + // Upcoming fixes: #14657, #14658. + // TODO(alanorozco): Cleanup markup for readability once fixes land. + html``; + + +export class Controls { + + /** @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc */ + constructor(ampdoc) { + + /** @private {!../../../src/service/ampdoc-impl.AmpDoc} */ + this.ampdoc_ = ampdoc; + + // TODO(alanorozco): `htmlFor` should work with `ShadowRoot` + const html = htmlFor(dev().assertElement(ampdoc.getBody())); + + /** @public @const {!Element} */ + this.container = renderControls(html); + + /** @public @const {!Element} */ + this.overlay = renderDockedOverlay(html); + + /** @private @const */ + this.manager_ = once(() => Services.videoManagerForDoc(ampdoc)); + + const refs = htmlRefs(this.container); + const assertRef = ref => dev().assertElement(refs[ref]); + + /** @private @const {!Element} */ + this.dismissButton_ = assertRef('dismissButton'); + + /** @private @const {!Element} */ + this.playButton_ = assertRef('playButton'); + + /** @private @const {!Element} */ + this.pauseButton_ = assertRef('pauseButton'); + + /** @private @const {!Element} */ + this.muteButton_ = assertRef('muteButton'); + + /** @private @const {!Element} */ + this.unmuteButton_ = assertRef('unmuteButton'); + + /** @private @const {!Element} */ + this.fullscreenButton_ = assertRef('fullscreenButton'); + + /** @private @const {!Element} */ + this.dismissContainer_ = assertRef('dismissContainer'); // eslint-disable-line + + /** @private {boolean} */ + this.isDisabled_ = false; + + /** @private {boolean} */ + this.isSticky_ = false; + + /** @private {function():!Timeout} */ + this.getHideTimeout_ = + once(() => new Timeout(this.ampdoc_.win, () => this.hide())); + + /** @private @const {!Array} */ + this.unlisteners_ = []; + + /** @private {?../../../src/video-interface.VideoOrBaseElementDef} */ + this.video_ = null; + + this.hideOnTapOutside_(); + this.showOnTapOrHover_(); + } + + /** @public */ + disable() { + this.isDisabled_ = true; + } + + /** @public */ + enable() { + this.isDisabled_ = false; + } + + /** + * @param {!../../../src/video-interface.VideoOrBaseElementDef} video + */ + setVideo(video) { + this.video_ = video; + this.listen_(video); + } + + /** + * @param {!../../../src/video-interface.VideoOrBaseElementDef} video + * @private + */ + listen_(video) { + this.unlisten_(); + + const click = 'click'; + + const {element} = video; + + this.unlisteners_.push( + this.listenWhenEnabled_(this.dismissButton_, click, () => { + this.container.dispatchEvent( + createCustomEvent(this.ampdoc_.win, + VideoDockingEvents.DISMISS_ON_TAP, /* detail */ undefined)); + }), + + this.listenWhenEnabled_(this.playButton_, click, () => { + video.play(/* auto */ false); + }), + + this.listenWhenEnabled_(this.pauseButton_, click, () => { + video.pause(); + }), + + this.listenWhenEnabled_(this.muteButton_, click, () => { + video.mute(); + }), + + this.listenWhenEnabled_(this.unmuteButton_, click, () => { + video.unmute(); + }), + + this.listenWhenEnabled_(this.fullscreenButton_, click, () => { + video.fullscreenEnter(); + }), + + listen(this.container, 'mouseup', () => + this.hideOnTimeout(TIMEOUT_AFTER_INTERACTION)), + + listen(element, VideoEvents.PLAYING, () => this.onPlay_()), + listen(element, VideoEvents.PAUSE, () => this.onPause_()), + listen(element, VideoEvents.MUTED, () => this.onMute_()), + listen(element, VideoEvents.UNMUTED, () => this.onUnmute_())); + } + + /** + * @return {boolean} + * @private + */ + isPlaying_() { + dev().assert(this.video_); + return this.manager_().getPlayingState(this.video_) != PlayingStates.PAUSED; + } + + /** @private */ + onPlay_() { + const {playButton_, pauseButton_} = this; + this.isSticky_ = false; + swap(playButton_, pauseButton_); + } + + /** @private */ + onPause_() { + const {pauseButton_, playButton_} = this; + this.isSticky_ = true; + swap(pauseButton_, playButton_); + } + + /** @private */ + onMute_() { + const {muteButton_, unmuteButton_} = this; + swap(muteButton_, unmuteButton_); + } + + /** @private */ + onUnmute_() { + const {unmuteButton_, muteButton_} = this; + swap(unmuteButton_, muteButton_); + } + + /** + * @param {!Element} element + * @param {string} eventType + * @param {function()} callback + * @return {!UnlistenDef} + * @private + */ + listenWhenEnabled_(element, eventType, callback) { + return listen(element, eventType, () => { + if (this.isDisabled_) { + return; + } + callback(); + }); + } + + /** @private */ + unlisten_() { + while (this.unlisteners_.length > 0) { + this.unlisteners_.pop().call(); + } + } + + /** @param {number=} timeout */ + hideOnTimeout(timeout = TIMEOUT) { + this.getHideTimeout_().trigger(timeout); + } + + /** @private */ + showOnTapOrHover_() { + const onTapOrHover = () => this.show_(); + const {overlay} = this; + + this.listenWhenEnabled_(overlay, 'mouseup', onTapOrHover); + this.listenWhenEnabled_(overlay, 'mouseover', onTapOrHover); + } + + /** @private */ + show_() { + const { + container, + overlay, + playButton_: playButton, + pauseButton_: pauseButton, + muteButton_: muteButton, + unmuteButton_: unmuteButton, + } = this; + + toggle(container, true); + container.classList.add('amp-video-docked-controls-shown'); + overlay.classList.add('amp-video-docked-controls-bg'); + + if (this.isPlaying_()) { + swap(playButton, pauseButton); + } else { + swap(pauseButton, playButton); + } + + if (this.manager_().isMuted(this.video_)) { + swap(muteButton, unmuteButton); + } else { + swap(unmuteButton, muteButton); + } + + this.hideOnTimeout(); + } + + /** + * @param {number} scale + * @param {number} x + * @param {number} y + * @param {number} width + * @param {number} height + */ + positionOnVsync(scale, x, y, width, height) { + const { + container, + dismissContainer_: dismissContainer, + } = this; + const halfScale = scale / 2; + const centerX = x + (width * halfScale); + const centerY = y + (height * halfScale); + + applyBreakpointClassname(container, scale * width, BREAKPOINTS); + + setImportantStyles(container, { + 'transform': translate(centerX, centerY), + }); + + const dismissMargin = 4; + const dismissWidth = 40; + const dismissX = width * halfScale - dismissMargin - dismissWidth; + const dismissY = -(height * halfScale - dismissMargin - dismissWidth); + setImportantStyles(dismissContainer, { + 'transform': translate(dismissX, dismissY), + }); + } + + /** @private */ + hideOnTapOutside_() { + listen(this.ampdoc_.getRootNode(), 'mousedown', e => { + if (this.isControlsTarget_(dev().assertElement(e.target))) { + return; + } + this.hide(/* respectSticky */ true); + }); + } + + /** + * @param {!Element} target + * @return {boolean} + * @private + */ + isControlsTarget_(target) { + return !!closestBySelector(target, '.amp-video-docked-controls'); + } + + /** + * @param {boolean=} respectSticky + * @param {boolean=} immediately Disables transition + * @public + */ + hide(respectSticky = false, immediately = false) { + const ampVideoDockedControlsShown = 'amp-video-docked-controls-shown'; + const {container, overlay} = this; + if (!container.classList.contains(ampVideoDockedControlsShown)) { + return; + } + if (respectSticky && this.isSticky_) { + return; + } + if (immediately) { + toggle(container, false); + toggle(overlay, false); + } + overlay.classList.remove('amp-video-docked-controls-bg'); + container.classList.remove(ampVideoDockedControlsShown); + } + + /** @public */ + reset() { + const {overlay, container} = this; + const els = [overlay, container]; + + toggle(overlay, false); + + this.hide(); + + for (let i = 0; i < els.length; i++) { + const el = els[i]; + resetStyles(el, [ + 'transform', + 'transition', + 'width', + 'height', + ]); + } + } +} diff --git a/extensions/amp-video-docking/0.1/events.js b/extensions/amp-video-docking/0.1/events.js new file mode 100644 index 0000000000000..6089088227d75 --- /dev/null +++ b/extensions/amp-video-docking/0.1/events.js @@ -0,0 +1,20 @@ +/** + * Copyright 2018 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. + */ + +/** @enum {string} */ +export const VideoDockingEvents = { + DISMISS_ON_TAP: 'dock-dismiss-on-tap', +}; diff --git a/extensions/amp-video-docking/0.1/html.js b/extensions/amp-video-docking/0.1/html.js new file mode 100644 index 0000000000000..173e08e2b8103 --- /dev/null +++ b/extensions/amp-video-docking/0.1/html.js @@ -0,0 +1,21 @@ +/** + * Copyright 2018 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. + */ + +/** + * See `src/static-template.js`. + * @typedef {function(!Array):!Element} + */ +export let HtmlLiteralTagDef; diff --git a/extensions/amp-video-docking/0.1/test/test-breakpoints.js b/extensions/amp-video-docking/0.1/test/test-breakpoints.js new file mode 100644 index 0000000000000..eab3bf66b36e7 --- /dev/null +++ b/extensions/amp-video-docking/0.1/test/test-breakpoints.js @@ -0,0 +1,88 @@ +/** + * Copyright 2018 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 {applyBreakpointClassname} from '../breakpoints'; + +describes.realWin('Synthetic Breakpoints', {amp: false}, env => { + const huge = 'foo-huge'; + const large = 'foo-large'; + const small = 'foo-small'; + const tiny = 'foo-tiny'; + + describe('applyBreakpointClassname', () => { + const breakpoints = [ + {minWidth: 0, className: tiny}, + {minWidth: 100, className: small}, + {minWidth: 200, className: large}, + {minWidth: 400, className: huge}, + ]; + + [ + {width: 1, expected: tiny}, + {width: 100, expected: small}, + {width: 120, expected: small}, + {width: 200, expected: large}, + {width: 300, expected: large}, + {width: 400, expected: huge}, + {width: 200000, expected: huge}, + ].forEach(({width, expected}) => { + const unexpected = Object.values(breakpoints) + .filter(({className}) => className != expected) + .map(({className}) => className); + + it(`applies classname (${expected}) for width = ${width} with ` + + 'multiple breakpoints', () => { + const element = env.win.document.createElement('div'); + + applyBreakpointClassname(element, width, breakpoints); + + expect(element.classList.contains(expected)).to.be.true; + }); + + it(`removes classnames that do not apply (${unexpected.join(', ')}) ` + + `for width = ${width} with multiple breakpoints`, () => { + const element = env.win.document.createElement('div'); + + unexpected.forEach(className => { + element.classList.add(className); + }); + + applyBreakpointClassname(element, width, breakpoints); + + unexpected.forEach(className => { + expect(element.classList.contains(className)).to.be.false; + }); + }); + }); + + Object.values(breakpoints).forEach(({className}) => { + [0, 1, 1000, 200000].forEach(width => { + [0, 1, 50, 1000, 1400, 100000].forEach(minWidth => { + const applies = minWidth <= width; + const verb = applies ? 'applies' : 'does not apply'; + it(`${verb} classname (${className}) with single breakpoint ` + + `(${minWidth})`, () => { + const element = env.win.document.createElement('div'); + + applyBreakpointClassname(element, width, [{minWidth, className}]); + + expect(element.classList.contains(className)).to.equal(applies); + }); + }); + }); + }); + }); +}); diff --git a/extensions/amp-video-docking/0.1/test/validator-amp-video-docking-amp-video-iframe.html b/extensions/amp-video-docking/0.1/test/validator-amp-video-docking-amp-video-iframe.html new file mode 100644 index 0000000000000..3ae126e4ce55e --- /dev/null +++ b/extensions/amp-video-docking/0.1/test/validator-amp-video-docking-amp-video-iframe.html @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + diff --git a/extensions/amp-video-docking/0.1/test/validator-amp-video-docking-amp-video-iframe.out b/extensions/amp-video-docking/0.1/test/validator-amp-video-docking-amp-video-iframe.out new file mode 100644 index 0000000000000..6888a84256ba1 --- /dev/null +++ b/extensions/amp-video-docking/0.1/test/validator-amp-video-docking-amp-video-iframe.out @@ -0,0 +1,43 @@ +PASS +| +| +| +| +| +| +| +| +| +| +| +| +| +| +| +| +| +| +| \ No newline at end of file diff --git a/extensions/amp-video-docking/0.1/test/validator-amp-video-docking-no-player.out b/extensions/amp-video-docking/0.1/test/validator-amp-video-docking-no-player.out index 7da19a1aab4a2..08006bd48dfb6 100644 --- a/extensions/amp-video-docking/0.1/test/validator-amp-video-docking-no-player.out +++ b/extensions/amp-video-docking/0.1/test/validator-amp-video-docking-no-player.out @@ -28,9 +28,9 @@ FAIL | | | ->> ^~~~~~~~~ -amp-video-docking/0.1/test/validator-amp-video-docking-no-player.html:29:2 The tag 'amp-video-docking for amp-video' requires including the 'amp-video' extension JavaScript. (see https://www.ampproject.org/docs/reference/components/amp-video-docking) [MANDATORY_AMP_TAG_MISSING_OR_INCORRECT] | | | -| \ No newline at end of file +| +>> ^~~~~~~~~ +amp-video-docking/0.1/test/validator-amp-video-docking-no-player.html:33:6 The extension 'amp-video-docking' was found on this page, but is unused. Please remove this extension. [AMP_TAG_PROBLEM] \ No newline at end of file diff --git a/extensions/amp-video-docking/0.1/timeout.js b/extensions/amp-video-docking/0.1/timeout.js new file mode 100644 index 0000000000000..f6e260de245f0 --- /dev/null +++ b/extensions/amp-video-docking/0.1/timeout.js @@ -0,0 +1,58 @@ +/** + * Copyright 2018 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 {Services} from '../../../src/services'; + + +/** Timeout that can be postponed, repeated or cancelled. */ +export class Timeout { + /** + * @param {!Window} win + * @param {!Function} handler + */ + constructor(win, handler) { + /** @private @const {!../../../src/service/timer-impl.Timer} */ + this.timer_ = Services.timerFor(win); + + /** @private @const {!Function} */ + this.handler_ = handler; + + /** @private {?number|?string} */ + this.id_ = null; + } + + /** + * @param {number} time + * @param {...*} args + */ + trigger(time, ...args) { + this.cancel(); + this.id_ = this.timer_.delay(() => this.handler_.apply(null, args), time); + } + + /** @public */ + cancel() { + if (this.id_ !== null) { + this.timer_.cancel(this.id_); + this.id_ = null; + } + } + + /** @return {boolean} */ + isWaiting() { + return this.id_ !== null; + } +} diff --git a/extensions/amp-video-docking/validator-amp-video-docking.protoascii b/extensions/amp-video-docking/validator-amp-video-docking.protoascii index 7c3b3b07abfb6..2875dfbd7b637 100644 --- a/extensions/amp-video-docking/validator-amp-video-docking.protoascii +++ b/extensions/amp-video-docking/validator-amp-video-docking.protoascii @@ -15,15 +15,13 @@ # tags: { # amp-video-docking - attr_lists: "common-extension-attrs" + html_format: AMP + tag_name: "SCRIPT" + spec_name: "amp-video-docking" extension_spec: { name: "amp-video-docking" - requires_usage: NONE version: "0.1" version: "latest" } - html_format: AMP - requires_extension: "amp-video" - spec_name: "amp-video-docking for amp-video" - tag_name: "SCRIPT" + attr_lists: "common-extension-attrs" } diff --git a/extensions/amp-video-iframe/amp-video-iframe.md b/extensions/amp-video-iframe/amp-video-iframe.md index 381b24e6f1398..5f8bb638da68d 100644 --- a/extensions/amp-video-iframe/amp-video-iframe.md +++ b/extensions/amp-video-iframe/amp-video-iframe.md @@ -9,7 +9,7 @@ Status - Experimental + Experimental. You must turn on the amp-video-iframe experiment to use this component. Required Script @@ -60,6 +60,12 @@ not already have a fragment. Points to an image URL that will be displayed while the video loads. +#### autoplay + +If this attribute is present, and the browser supports autoplay, the video will be automatically +played as soon as it becomes visible. There are some conditions that the component needs to meet +to be played, [which are outlined in the Video in AMP spec](https://github.com/ampproject/amphtml/blob/master/spec/amp-video-interface.md#autoplay). + ##### common attributes This element includes [common attributes](https://www.ampproject.org/docs/reference/common_attributes) extended to AMP components. @@ -72,6 +78,12 @@ Set this attribute if the document inside the iframe implements the [MediaSessio Set this attribute if the document inside the iframe implements rotate-to-fullscreen independently. +#### referrerpolicy + +The [`referrerpolicy`](https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement/referrerPolicy) to be set on the iframe element. + + + ## Integration In order for the video integration to work, the document living inside your @@ -114,6 +126,8 @@ function onAmpIntegrationReady(ampIntegration) { } ``` +**Never play the video inside the frame automatically.** Instead, you should support the integration script and use the `amp-video-iframe` tag with the `autoplay` attribute. The AMP component will autoamtically send the necessary signals to your iframe to autoplay for a better user experience. + ### Custom integrations It's possible to have more fine-grained control over how the video interacts diff --git a/extensions/amp-video-iframe/validator-amp-video-iframe.protoascii b/extensions/amp-video-iframe/validator-amp-video-iframe.protoascii index ab33aa0c49fa1..b1e9edc40704e 100644 --- a/extensions/amp-video-iframe/validator-amp-video-iframe.protoascii +++ b/extensions/amp-video-iframe/validator-amp-video-iframe.protoascii @@ -36,6 +36,10 @@ attr_lists: { name: "autoplay" value: "" } + attrs: { + name: "dock" + requires_extension: "amp-video-docking" + } attrs: { name: "implements-media-session" value: "" diff --git a/extensions/amp-video/0.1/test/test-amp-video.js b/extensions/amp-video/0.1/test/test-amp-video.js index 23340d58ef64f..33737f735348f 100644 --- a/extensions/amp-video/0.1/test/test-amp-video.js +++ b/extensions/amp-video/0.1/test/test-amp-video.js @@ -435,7 +435,8 @@ describes.realWin('amp-video', { }); }); - it('should forward certain events from video to the amp element', () => { + // TODO: unskip the tests in this file #19664 + it.skip('should forward certain events from video to the amp element', () => { return getVideo({ src: '/examples/av/ForBiggerJoyrides.mp4', width: 160, @@ -495,6 +496,9 @@ describes.realWin('amp-video', { */ function getVideoWithBlur(addPlaceholder, addBlurClass) { const v = doc.createElement('amp-video'); + v.setAttribute('layout', 'fixed'); + v.setAttribute('width', '300px'); + v.setAttribute('height', '250px'); const img = doc.createElement('img'); if (addPlaceholder) { img.setAttribute('placeholder', ''); @@ -687,7 +691,7 @@ describes.realWin('amp-video', { }); }); - describe('before visible', () => { + describe.skip('before visible', () => { it('should move cached src to source during prerender', () => { return getVideo({ 'src': 'https://example-com.cdn.ampproject.org/m/s/video.mp4', @@ -774,7 +778,7 @@ describes.realWin('amp-video', { }); }); - describe('after visible', () => { + describe.skip('after visible', () => { it('should add original source after cache one - single src', () => { let ampVideoElement; return getVideo({ diff --git a/gulpfile.js b/gulpfile.js index f32e0b0e616f8..4767171e9d34e 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -42,6 +42,7 @@ const {createModuleCompatibleES5Bundle} = require('./build-system/tasks/create-m const {extensionBundles, aliasBundles} = require('./bundles.config'); const {jsifyCssAsync} = require('./build-system/tasks/jsify-css'); const {serve} = require('./build-system/tasks/serve.js'); +const {thirdPartyFrames} = require('./build-system/config'); const {transpileTs} = require('./build-system/typescript'); const {VERSION: internalRuntimeVersion} = require('./build-system/internal-version') ; @@ -501,28 +502,19 @@ function compile(watch, shouldMinify, opt_preventRemoveAndMakeDir, })); } - promises.push( - thirdPartyBootstrap( - '3p/frame.max.html', 'frame.html', shouldMinify), - thirdPartyBootstrap( - '3p/nameframe.max.html', 'nameframe.html', shouldMinify), - thirdPartyBootstrap( - '3p/recaptcha.max.html', 'recaptcha.html', shouldMinify) - - ); + thirdPartyFrames.forEach(frameObject => { + promises.push( + thirdPartyBootstrap( + frameObject.max, frameObject.min, shouldMinify) + ); + }); if (watch) { - $$.watch('3p/nameframe.max.html', function() { - thirdPartyBootstrap( - '3p/nameframe.max.html', 'nameframe.html', shouldMinify); - }); - $$.watch('3p/frame.max.html', function() { - thirdPartyBootstrap( - '3p/frame.max.html', 'frame.html', shouldMinify); - }); - $$.watch('3p/recaptcha.max.html', function() { - thirdPartyBootstrap( - '3p/recaptcha.max.html', 'recaptcha.html', shouldMinify); + thirdPartyFrames.forEach(frameObject => { + $$.watch(frameObject.max, function() { + thirdPartyBootstrap( + frameObject.max, frameObject.min, shouldMinify); + }); }); } @@ -1265,13 +1257,14 @@ function compileJs(srcDir, srcFilename, destDir, options) { // We don't need an explicit function wrapper like we do for `gulp dist` // because Babel handles that for you. const wrapper = options.wrapper || wrappers.none; + const devWrapper = wrapper.replace('<%= contents %>', '$1'); const lazybuild = lazypipe() .pipe(source, srcFilename) .pipe(buffer) + .pipe($$.sourcemaps.init.bind($$.sourcemaps), {loadMaps: true}) .pipe($$.regexpSourcemaps, /\$internalRuntimeVersion\$/g, internalRuntimeVersion, 'runtime-version') - .pipe($$.wrap, wrapper) - .pipe($$.sourcemaps.init.bind($$.sourcemaps), {loadMaps: true}); + .pipe($$.regexpSourcemaps, /([^]+)/, devWrapper, 'wrapper'); const lazywrite = lazypipe() .pipe($$.sourcemaps.write.bind($$.sourcemaps), './') diff --git a/package.json b/package.json index 706cb7e2625cc..f1b8435f66c2c 100644 --- a/package.json +++ b/package.json @@ -109,7 +109,6 @@ "gulp-uglify": "3.0.1", "gulp-watch": "5.0.1", "gulp-webserver": "0.9.1", - "gulp-wrap": "0.14.0", "gzip-size": "5.0.0", "is-running": "2.1.0", "jest": "23.6.0", diff --git a/src/batched-json.js b/src/batched-json.js index 27348374727b0..6d96b6a923e7e 100644 --- a/src/batched-json.js +++ b/src/batched-json.js @@ -52,7 +52,7 @@ export function batchFetchJsonFor( { assertHttpsUrl(element.getAttribute('src'), element); const xhr = Services.batchedXhrFor(ampdoc.win); - return requestForBatchFetch(ampdoc, element, opt_urlReplacement, opt_refresh) + return requestForBatchFetch(element, opt_urlReplacement, opt_refresh) .then(data => xhr.fetchJson(data.xhrUrl, data.fetchOpt)) .then(res => res.json()) .then(data => { @@ -66,18 +66,17 @@ export function batchFetchJsonFor( /** * Handles url replacement and constructs the FetchInitJsonDef required for a * fetch. - * @param {!./service/ampdoc-impl.AmpDoc} ampdoc * @param {!Element} element * @param {!UrlReplacementPolicy} replacement If ALL, replaces all URL * vars. If OPT_IN, replaces whitelisted URL vars. Otherwise, don't expand. * @param {boolean} refresh Forces refresh of browser cache. * @return {!Promise} */ -export function requestForBatchFetch(ampdoc, element, replacement, refresh) { +export function requestForBatchFetch(element, replacement, refresh) { const url = element.getAttribute('src'); // Replace vars in URL if desired. - const urlReplacements = Services.urlReplacementsForDoc(ampdoc); + const urlReplacements = Services.urlReplacementsForDoc(element); const promise = (replacement >= UrlReplacementPolicy.OPT_IN) ? urlReplacements.expandUrlAsync(url) : Promise.resolve(url); diff --git a/src/polyfills.js b/src/polyfills.js index 7c86eb2908fac..c718e669fb482 100644 --- a/src/polyfills.js +++ b/src/polyfills.js @@ -40,7 +40,7 @@ installDocContains(self); installArrayIncludes(self); // isExperimentOn() must be called after Object.assign polyfill is installed. if (isExperimentOn(self, 'custom-elements-v1') || getMode().test) { - installCustomElements(self, class {}); + installCustomElements(self); } else { installRegisterElement(self, 'auto'); } diff --git a/src/polyfills/custom-elements.js b/src/polyfills/custom-elements.js index fdc58a4e024f7..61d9b6aed7a05 100644 --- a/src/polyfills/custom-elements.js +++ b/src/polyfills/custom-elements.js @@ -52,6 +52,14 @@ const INVALID_NAMES = [ 'missing-glyph', ]; +/** + * A MutationObserverInit dictionary to track subtree modifications. + */ +const TRACK_SUBTREE = { + 'childList': true, + 'subtree': true, +}; + /** * Asserts that the custom element name conforms to the spec. * @@ -91,6 +99,17 @@ function isPatched(win) { return tag.indexOf('[native code]') === -1; } +/** + * Throws the error outside the current event loop. + * + * @param {!Error} error + */ +function rethrowAsync(error) { + new /*OK*/Promise(() => { + throw error; + }); +} + /** * The public Custom Elements API. */ @@ -238,6 +257,14 @@ class Registry { * @private {MutationObserver} */ this.mutationObserver_ = null; + + /** + * All the observed DOM trees, including shadow trees. This is cleared out + * when the mutation observer is created. + * + * @private @const {!Array} + */ + this.observed_ = [win.document]; } /** @@ -396,11 +423,15 @@ class Registry { // run on the node. The node itself is already constructed, so the return // value is just the node. this.current_ = node; - const el = new ctor(); + try { + const el = new ctor(); - if (el !== node) { - throw new this.win_.Error( - 'Constructor illegally returned a different instance.'); + if (el !== node) { + throw new this.win_.Error( + 'Constructor illegally returned a different instance.'); + } + } catch (e) { + rethrowAsync(e); } } @@ -422,7 +453,11 @@ class Registry { // TODO(jridgewell): I should be calling the definitions connectedCallback // with node as the context. if (node.connectedCallback) { - node.connectedCallback(); + try { + node.connectedCallback(); + } catch (e) { + rethrowAsync(e); + } } } @@ -435,7 +470,11 @@ class Registry { // TODO(jridgewell): I should be calling the definitions connectedCallback // with node as the context. if (node.disconnectedCallback) { - node.disconnectedCallback(); + try { + node.disconnectedCallback(); + } catch (e) { + rethrowAsync(e); + } } } @@ -463,19 +502,34 @@ class Registry { this.query_ = name; // The first registered name starts the mutation observer. - this.mutationObserver_ = new this.win_.MutationObserver(records => { + const mo = new this.win_.MutationObserver(records => { if (records) { this.handleRecords_(records); } }); - this.mutationObserver_.observe(this.doc_, { - childList: true, - subtree: true, + this.mutationObserver_ = mo; + + this.observed_.forEach(tree => { + mo.observe(tree, TRACK_SUBTREE); }); + this.observed_.length = 0; installPatches(this.win_, this); } + /** + * Adds the shadow tree to be observed by the polyfill. + * + * @param {!Node} tree + */ + observe(tree) { + if (this.mutationObserver_) { + this.mutationObserver_.observe(tree, TRACK_SUBTREE); + } else { + this.observed_.push(tree); + } + } + /** * This causes a synchronous handling of all the Mutation Observer's tracked * mutations. This does nothing until the mutation observer is actually @@ -618,7 +672,7 @@ function installPatches(win, registry) { * @param {!Window} win */ function polyfill(win) { - const {HTMLElement, Object, document} = win; + const {Element, HTMLElement, Object, document} = win; const {createElement} = document; const registry = new Registry(win); @@ -634,6 +688,41 @@ function polyfill(win) { value: customElements, }); + // Have to patch shadow methods now, since there's no way to find shadow trees + // later. + const elProto = Element.prototype; + const {attachShadow, createShadowRoot} = elProto; + if (attachShadow) { + /** + * @param {!{mode: string}} unused + * @return {!ShadowRoot} + */ + elProto.attachShadow = function(unused) { + const shadow = attachShadow.apply(this, arguments); + registry.observe(shadow); + return shadow; + }; + // Necessary for Shadow AMP + elProto.attachShadow.toString = function() { + return attachShadow.toString(); + }; + } + if (createShadowRoot) { + /** + * @return {!ShadowRoot} + */ + elProto.createShadowRoot = function() { + const shadow = createShadowRoot.apply(this, arguments); + registry.observe(shadow); + return shadow; + }; + // Necessary for Shadow AMP + elProto.createShadowRoot.toString = function() { + return createShadowRoot.toString(); + }; + } + + /** * You can't use the real HTMLElement constructor, because you can't subclass * it without using native classes. So, mock its approximation using @@ -729,21 +818,22 @@ function subClass(Object, superClass, subClass) { } /** - * Polyfills Custom Elements v1 API. This has 4 modes: + * Polyfills Custom Elements v1 API. This has 5 modes: * * 1. Custom elements v1 already supported, using native classes * 2. Custom elements v1 already supported, using transpiled classes * 3. Custom elements v1 not supported, using native classes * 4. Custom elements v1 not supported, using transpiled classes + * 5. No sample class constructor provided * * In mode 1, nothing is done. In mode 2, a minimal polyfill is used to support - * extending the HTMLElement base class. In mode 3 and 4, a full polyfill is + * extending the HTMLElement base class. In mode 3, 4, and 5 a full polyfill is * done. * * @param {!Window} win - * @param {!Function} ctor + * @param {!Function=} opt_ctor */ -export function install(win, ctor) { +export function install(win, opt_ctor) { if (isPatched(win)) { return; } @@ -751,7 +841,7 @@ export function install(win, ctor) { let install = true; let installWrapper = false; - if (hasCustomElements(win)) { + if (opt_ctor && hasCustomElements(win)) { // If ctor is constructable without new, it's a function. That means it was // compiled down, and we need to do the minimal polyfill because all you // cannot extend HTMLElement without native classes. @@ -759,8 +849,8 @@ export function install(win, ctor) { const {Object, Reflect} = win; // "Construct" ctor using ES5 idioms - const instance = Object.create(ctor.prototype); - ctor.call(instance); + const instance = Object.create(opt_ctor.prototype); + opt_ctor.call(instance); // If that succeeded, we're in a transpiled environment // Let's find out if we can wrap HTMLElement and avoid a full patch. diff --git a/src/service/extensions-impl.js b/src/service/extensions-impl.js index fb62ca495a164..c6115949f4990 100644 --- a/src/service/extensions-impl.js +++ b/src/service/extensions-impl.js @@ -689,8 +689,8 @@ export function stubLegacyElements(win) { function installPolyfillsInChildWindow(parentWin, childWin) { installDocContains(childWin); installDOMTokenListToggle(childWin); - if (isExperimentOn(parentWin, 'custom-elements-v1')) { - installCustomElements(childWin, class {}); + if (isExperimentOn(parentWin, 'custom-elements-v1') || getMode().test) { + installCustomElements(childWin); } else { installRegisterElement(childWin, 'auto'); } diff --git a/src/service/navigation.js b/src/service/navigation.js index 93e422917f252..af9bc702c3a02 100644 --- a/src/service/navigation.js +++ b/src/service/navigation.js @@ -609,7 +609,7 @@ function maybeExpandUrlParams(ampdoc, e) { return e.pageY; }, }; - const newHref = Services.urlReplacementsForDoc(ampdoc).expandUrlSync( + const newHref = Services.urlReplacementsForDoc(target).expandUrlSync( hrefToExpand, vars, undefined, /* opt_whitelist */ { // For now we only allow to replace the click location vars // and nothing else. diff --git a/src/service/url-replacements-impl.js b/src/service/url-replacements-impl.js index 636d77b2ce26d..99c74197cb6b3 100644 --- a/src/service/url-replacements-impl.js +++ b/src/service/url-replacements-impl.js @@ -113,6 +113,8 @@ export class GlobalVariableSource extends VariableSource { /** @override */ initialize() { + const {win} = this.ampdoc; + /** @const {!./viewport/viewport-impl.Viewport} */ const viewport = Services.viewportForDoc(this.ampdoc); @@ -154,7 +156,7 @@ export class GlobalVariableSource extends VariableSource { const referrerHostname = parseUrlDeprecated(getSourceUrl(referrer)) .hostname; const currentHostname = - WindowInterface.getHostname(this.ampdoc.win); + WindowInterface.getHostname(win); return referrerHostname === currentHostname ? null : referrer; }); })); @@ -163,26 +165,25 @@ export class GlobalVariableSource extends VariableSource { this.set('TITLE', () => { // The environment may override the title and set originalTitle. Prefer // that if available. - return this.ampdoc.win.document['originalTitle'] || - this.ampdoc.win.document.title; + const doc = win.document; + return doc['originalTitle'] || doc.title; }); // Returns the URL for this AMP document. this.set('AMPDOC_URL', () => { return removeFragment( - this.addReplaceParamsIfMissing_( - this.ampdoc.win.location.href)); + this.addReplaceParamsIfMissing_(win.location.href)); }); // Returns the host of the URL for this AMP document. this.set('AMPDOC_HOST', () => { - const url = parseUrlDeprecated(this.ampdoc.win.location.href); + const url = parseUrlDeprecated(win.location.href); return url && url.host; }); // Returns the hostname of the URL for this AMP document. this.set('AMPDOC_HOSTNAME', () => { - const url = parseUrlDeprecated(this.ampdoc.win.location.href); + const url = parseUrlDeprecated(win.location.href); return url && url.hostname; }); @@ -356,8 +357,7 @@ export class GlobalVariableSource extends VariableSource { // Returns the IANA timezone code this.set('TIMEZONE_CODE', () => { let tzCode; - if ('Intl' in this.ampdoc.win && - 'DateTimeFormat' in this.ampdoc.win.Intl) { + if ('Intl' in win && 'DateTimeFormat' in win.Intl) { // It could be undefined (i.e. IE11) tzCode = new Intl.DateTimeFormat().resolvedOptions().timeZone; } @@ -383,8 +383,7 @@ export class GlobalVariableSource extends VariableSource { // Returns the viewport width. this.set('VIEWPORT_WIDTH', () => viewport.getWidth()); - - const {screen} = this.ampdoc.win; + const {screen} = win; // Returns screen.width. this.set('SCREEN_WIDTH', screenProperty(screen, 'width')); @@ -402,21 +401,20 @@ export class GlobalVariableSource extends VariableSource { // Returns document characterset. this.set('DOCUMENT_CHARSET', () => { - const doc = this.ampdoc.win.document; + const doc = win.document; return doc.characterSet || doc.charset; }); // Returns the browser language. this.set('BROWSER_LANGUAGE', () => { - const nav = this.ampdoc.win.navigator; + const nav = win.navigator; return (nav.language || nav.userLanguage || nav.browserLanguage || '') .toLowerCase(); }); // Returns the user agent. this.set('USER_AGENT', () => { - const nav = this.ampdoc.win.navigator; - return nav.userAgent; + return win.navigator.userAgent; }); // Returns the time it took to load the whole page. (excludes amp-* elements @@ -497,7 +495,7 @@ export class GlobalVariableSource extends VariableSource { user().assert(startAttribute, 'The first argument to NAV_TIMING, the ' + 'start attribute name, is required'); return getTimingDataSync( - this.ampdoc.win, + win, /**@type {string}*/(startAttribute), /**@type {string}*/(endAttribute)); }); @@ -505,17 +503,17 @@ export class GlobalVariableSource extends VariableSource { user().assert(startAttribute, 'The first argument to NAV_TIMING, the ' + 'start attribute name, is required'); return getTimingDataAsync( - this.ampdoc.win, + win, /**@type {string}*/(startAttribute), /**@type {string}*/(endAttribute)); }); this.set('NAV_TYPE', () => { - return getNavigationData(this.ampdoc.win, 'type'); + return getNavigationData(win, 'type'); }); this.set('NAV_REDIRECT_COUNT', () => { - return getNavigationData(this.ampdoc.win, 'redirectCount'); + return getNavigationData(win, 'redirectCount'); }); // returns the AMP version number @@ -543,21 +541,23 @@ export class GlobalVariableSource extends VariableSource { this.setAsync('FIRST_CONTENTFUL_PAINT', () => { return tryResolve(() => - Services.performanceFor(this.ampdoc.win).getFirstContentfulPaint()); + Services.performanceFor(win).getFirstContentfulPaint()); }); this.setAsync('FIRST_VIEWPORT_READY', () => { return tryResolve(() => - Services.performanceFor(this.ampdoc.win).getFirstViewportReady()); + Services.performanceFor(win).getFirstViewportReady()); }); this.setAsync('MAKE_BODY_VISIBLE', () => { return tryResolve(() => - Services.performanceFor(this.ampdoc.win).getMakeBodyVisible()); + Services.performanceFor(win).getMakeBodyVisible()); }); this.setAsync('AMP_STATE', key => { - return Services.bindForDocOrNull(this.ampdoc).then(bind => { + // This is safe since AMP_STATE is not an A4A whitelisted variable. + const {documentElement} = win.document; + return Services.bindForDocOrNull(documentElement).then(bind => { if (!bind) { return ''; } @@ -780,8 +780,13 @@ export class UrlReplacements { */ expandStringSync(source, opt_bindings, opt_collectVars, opt_whiteList) { return /** @type {string} */ ( - new Expander(this.variableSource_, opt_bindings, opt_collectVars, - /* opt_sync */ true, opt_whiteList)./*OK*/expand(source)); + new Expander( + this.variableSource_, + opt_bindings, + opt_collectVars, + /* opt_sync */ true, + opt_whiteList, + /* opt_noEncode */ true)./*OK*/expand(source)); } /** diff --git a/src/services.js b/src/services.js index e675b6f30cfd9..83bfdb7bf40b7 100644 --- a/src/services.js +++ b/src/services.js @@ -158,13 +158,13 @@ export class Services { } /** - * @param {!Element|!./service/ampdoc-impl.AmpDoc} elementOrAmpDoc + * @param {!Element} element * @return {!Promise} */ - static bindForDocOrNull(elementOrAmpDoc) { + static bindForDocOrNull(element) { return /** @type {!Promise} */ ( getElementServiceIfAvailableForDocInEmbedScope( - elementOrAmpDoc, 'bind', 'amp-bind')); + element, 'bind', 'amp-bind')); } /** @@ -474,15 +474,12 @@ export class Services { } /** - * Unlike most service getters, passing `Node` is necessary for some FIE-scope - * services since sometimes we only have the FIE Document for context. - * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrDoc + * @param {!Element} element * @return {!./service/url-replacements-impl.UrlReplacements} */ - static urlReplacementsForDoc(nodeOrDoc) { + static urlReplacementsForDoc(element) { return /** @type {!./service/url-replacements-impl.UrlReplacements} */ ( - getExistingServiceForDocInEmbedScope( - nodeOrDoc, 'url-replace', /* opt_fallbackToTopWin */ true)); + getExistingServiceForDocInEmbedScope(element, 'url-replace')); } /** diff --git a/src/utils/story.js b/src/utils/story.js index 5acdf167930a5..7f3952c5d9233 100644 --- a/src/utils/story.js +++ b/src/utils/story.js @@ -14,7 +14,7 @@ * limitations under the License. */ -import {closestByTag} from '../dom'; +import {closestByTag, waitForChild} from '../dom'; /** @@ -28,3 +28,19 @@ import {closestByTag} from '../dom'; export function descendsFromStory(element) { return !!closestByTag(element, 'amp-story-page'); } + +/** + * Returns true if the document is an amp-story. + * @param {!../service/ampdoc-impl.AmpDoc} ampdoc + * @return {!Promise} + */ +export function isStoryDocument(ampdoc) { + return new Promise(resolve => { + ampdoc.whenBodyAvailable().then(() => { + const body = ampdoc.getBody(); + waitForChild(body, () => !!body.firstElementChild, () => { + resolve(body.firstElementChild.tagName === 'AMP-STORY'); + }); + }); + }); +} diff --git a/src/web-components.js b/src/web-components.js index cb03629b2f03c..c58617f84ac2a 100644 --- a/src/web-components.js +++ b/src/web-components.js @@ -78,7 +78,7 @@ export function isShadowCssSupported() { /** * Returns `true` if the passed function is native to the browser, and is not * polyfilled - * @param {function()|undefined} func A function that is attatched to a JS + * @param {Function|undefined} func A function that is attatched to a JS * object. * @return {boolean} */ diff --git a/test/functional/test-friendly-iframe-embed.js b/test/functional/test-friendly-iframe-embed.js index acb03b31beecc..21b455e27cdd4 100644 --- a/test/functional/test-friendly-iframe-embed.js +++ b/test/functional/test-friendly-iframe-embed.js @@ -31,7 +31,6 @@ import {isAnimationNone} from '../../testing/test-helper'; import {layoutRectLtwh} from '../../src/layout-rect'; import {loadPromise} from '../../src/event-helper'; - describe('friendly-iframe-embed', () => { let sandbox; @@ -66,7 +65,8 @@ describe('friendly-iframe-embed', () => { }); } - it('should follow main install steps', () => { + // TODO: fix tests. #19650 + it.skip('should follow main install steps', () => { // Resources are not involved. extensionsMock.expects('preloadExtension').never(); @@ -115,7 +115,7 @@ describe('friendly-iframe-embed', () => { }); }); - it('should write doc if srcdoc is not available', () => { + it.skip('should write doc if srcdoc is not available', () => { setSrcdocSupportedForTesting(false); const embedPromise = installFriendlyIframeEmbed(iframe, document.body, { @@ -195,7 +195,7 @@ describe('friendly-iframe-embed', () => { }); }); - it('should install and dispose services', () => { + it.skip('should install and dispose services', () => { const disposeSpy = sandbox.spy(); const embedService = { dispose: disposeSpy, @@ -237,7 +237,7 @@ describe('friendly-iframe-embed', () => { }); }); - it('should support host', () => { + it.skip('should support host', () => { const host = document.createElement('amp-host'); const hostSignals = new Signals(); host.signals = () => hostSignals; @@ -255,7 +255,7 @@ describe('friendly-iframe-embed', () => { }); }); - it('should await initial load', () => { + it.skip('should await initial load', () => { resourcesMock .expects('getResourcesInRect') .withExactArgs( @@ -281,7 +281,7 @@ describe('friendly-iframe-embed', () => { }); }); - it('should await initial with host', () => { + it.skip('should await initial with host', () => { const rect = layoutRectLtwh(10, 10, 100, 200); const host = document.createElement('amp-host'); const hostSignals = new Signals(); @@ -470,7 +470,7 @@ describe('friendly-iframe-embed', () => { }); }); - describe('child document ready and loaded states', () => { + describe.skip('child document ready and loaded states', () => { it('should wait until ready', () => { const embedPromise = installFriendlyIframeEmbed(iframe, document.body, { diff --git a/test/functional/test-navigation.js b/test/functional/test-navigation.js index ac397fb5b2f2c..d02d6dc6a4af3 100644 --- a/test/functional/test-navigation.js +++ b/test/functional/test-navigation.js @@ -60,19 +60,26 @@ describes.sandboxed('Navigation', {}, () => { beforeEach(() => { win = env.win; doc = win.document; + const {documentElement} = doc; handler = Services.navigationForDoc(doc); handler.isIframed_ = true; + decorationSpy = sandbox.spy(Impression, 'getExtraParamsUrl'); + handleNavSpy = sandbox.spy(handler, 'handleNavClick_'); + handleCustomProtocolSpy = sandbox.spy(handler, 'handleCustomProtocolClick_'); + win.open = function() {}; winOpenStub = sandbox.stub(win, 'open').callsFake(() => { return {}; }); + const viewport = Services.viewportForDoc(doc); scrollIntoViewStub = sandbox.stub(viewport, 'scrollIntoView'); + const history = Services.historyForDoc(doc); replaceStateForTargetPromise = Promise.resolve(); replaceStateForTargetStub = sandbox.stub( @@ -84,6 +91,10 @@ describes.sandboxed('Navigation', {}, () => { doc.body.appendChild(anchor); event.target = anchor; + const urlReplacements = Services.urlReplacementsForDoc(documentElement); + sandbox.stub(Services, 'urlReplacementsForDoc') + .withArgs(anchor).returns(urlReplacements); + elementWithId = doc.createElement('div'); elementWithId.id = 'test'; doc.body.appendChild(elementWithId); @@ -661,6 +672,13 @@ describes.sandboxed('Navigation', {}, () => { doc.body.appendChild(anchor); event.target = anchor; + // Navigation uses the UrlReplacements service scoped to the event + // target, but for testing stub in the top-level service for simplicity. + const {documentElement} = parentWin.document; + const urlReplacements = Services.urlReplacementsForDoc(documentElement); + sandbox.stub(Services, 'urlReplacementsForDoc') + .withArgs(anchor).returns(urlReplacements); + elementWithId = doc.createElement('div'); elementWithId.id = 'test'; doc.body.appendChild(elementWithId); diff --git a/test/functional/test-url-replacements.js b/test/functional/test-url-replacements.js index 16d6771be6c6f..140a62285edfa 100644 --- a/test/functional/test-url-replacements.js +++ b/test/functional/test-url-replacements.js @@ -1749,6 +1749,15 @@ describes.sandboxed('UrlReplacements', {}, () => { expect(expanded).to.match(/abc:\/\/example\.com\/\?r=(\d+(\.\d+)?)$/); }); + it('should not encode values returned by expandStringSync', () => { + const win = getFakeWindow(); + const urlReplacements = Services.urlReplacementsForDoc(win.ampdoc); + const expanded = urlReplacements.expandStringSync( + 'title=TITLE', + {'TITLE': 'test with spaces'}); + expect(expanded).to.equal('title=test with spaces'); + }); + it('should not check protocol changes with expandStringAsync', () => { const win = getFakeWindow(); const urlReplacements = Services.urlReplacementsForDoc(win.ampdoc); diff --git a/test/integration/test-amp-ad-fake.js b/test/integration/test-amp-ad-fake.js new file mode 100644 index 0000000000000..e3f2095e6d1ed --- /dev/null +++ b/test/integration/test-amp-ad-fake.js @@ -0,0 +1,63 @@ +/** + * Copyright 2018 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 {RequestBank} from '../../testing/test-helper'; +import {parseQueryString} from '../../src/url'; + +describe.configure().skipIfPropertiesObfuscated().run('A4A', function() { + this.timeout(15000); + + describes.integration('AMPHTML ads rendered on AMP page', { + body: ` + +
Loading...
+
Could not display the fake ad :(
+
+ `, + extensions: ['amp-ad'], + }, () => { + it('should layout amp-img, amp-pixel, amp-analytics', () => { + // See amp4test.js for creative content + return Promise.all([ + RequestBank.withdraw('image'), + RequestBank.withdraw('pixel'), + RequestBank.withdraw('analytics'), + ]).then(reqs => { + const imageReq = reqs[0]; + const pixelReq = reqs[1]; + const analyticsReq = reqs[2]; + expect(imageReq.url).to.equal('/'); + expect(pixelReq.url).to.equal('/foo?cid='); + expect(analyticsReq.url).to.match(/^\/bar\?/); + const queries = + parseQueryString(analyticsReq.url.substr('/bar'.length)); + expect(queries).to.include({ + title: 'AMP TEST', // ${title}, + cid: '', // ${clientId(a)} + adNavTiming: '0', // ${adNavTiming(requestStart,requestStart)} + adNavType: '0', // ${adNavType} + adRedirectCount: '0', // ${adRedirectCount} + }); + expect(queries['ampdocUrl']).to.contain('http://localhost:9876/amp4test/compose-doc?'); + expect(queries['canonicalUrl']).to.equal('http://nonblocking.io/'); + expect(queries['img']).to.contain('/deposit/image'); // ${htmlAttr(amp-img,src)} + }); + }); + }); +}); diff --git a/test/integration/test-amp-analytics.js b/test/integration/test-amp-analytics.js index e85d6accc4789..9300eccb9694c 100644 --- a/test/integration/test-amp-analytics.js +++ b/test/integration/test-amp-analytics.js @@ -15,26 +15,23 @@ */ import {RequestBank} from '../../testing/test-helper'; - -const RequestId = { - BASIC: 'analytics-basic', - BATCH: 'analytics-batch', - USE_BODY: 'analytics-use-body', - BATCH_BODY: 'analytics-batch-use-body', - NO_REFERRER: 'analytics-no-referrer', -}; +import {parseQueryString} from '../../src/url'; describe.configure().skipIfPropertiesObfuscated().run('amp' + '-analytics', function() { this.timeout(15000); describes.integration('amp-analytics basic request', { - body: - ` + body: ` + + `, extensions: ['amp-analytics'], }, () => { + afterEach(() => { + // clean up written _cid cookie + document.cookie = '_cid=;expires=' + new Date(0).toUTCString(); + }); + it('should send request', () => { - return RequestBank.withdraw(RequestId.BASIC).then(req => { - expect(req.url).to.equal('/?a=2&b=AMP%20TEST'); + return RequestBank.withdraw().then(req => { + expect(req.url).to.equal('/?a=2&b=AMP%20TEST&cid=amp-12345'); expect(req.headers.referer, 'should keep referrer if no referrerpolicy specified').to.be.ok; }); }); }); + describes.integration('amp-analytics CLIENT_ID new user', { + body: ` + + + + + + + + `, + extensions: ['amp-analytics'], + }, () => { + afterEach(() => { + // clean up written _cid cookie + document.cookie = '_cid=;expires=' + new Date(0).toUTCString(); + }); + + it('should assign new cid', () => { + return Promise.all([ + RequestBank.withdraw(1), + RequestBank.withdraw(2), + ]).then(reqs => { + const req1 = reqs[0]; + const req2 = reqs[1]; + expect(req1.url).to.match(/^\/\?cid=/); + expect(req2.url).to.match(/^\/\?cid=/); + const cid1 = req1.url.substr('/?cid='.length); + const cid2 = req2.url.substr('/?cid='.length); + expect(cid1).to.match(/^amp-/); + expect(cid2).to.equal(cid1); + expect(document.cookie).to.contain('_cid=' + cid1); + }); + }); + }); + describes.integration('amp-analytics batch', { body: ` @@ -70,7 +135,7 @@ describe.configure().skipIfPropertiesObfuscated().run('amp' + { "requests": { "endpoint": { - "baseUrl": "${RequestBank.getUrl(RequestId.BATCH)}", + "baseUrl": "${RequestBank.getUrl()}", "batchInterval": 1 } }, @@ -102,7 +167,7 @@ describe.configure().skipIfPropertiesObfuscated().run('amp' + extensions: ['amp-analytics'], }, () => { it('should send request in batch', () => { - return RequestBank.withdraw(RequestId.BATCH).then(req => { + return RequestBank.withdraw().then(req => { expect(req.url).to.equal('/?a=1&b=AMP%20TEST&a=1&b=AMP%20TEST'); }); }); @@ -114,7 +179,7 @@ describe.configure().skipIfPropertiesObfuscated().run('amp' + + + + `, + extensions: ['amp-analytics'], + }, () => { + afterEach(() => { + // clean up written _ga cookie + document.cookie = '_ga=;expires=' + new Date(0).toUTCString(); + }); + + it('should send request', () => { + return RequestBank.withdraw().then(req => { + expect(req.url).to.match(/^\/r\/collect\?/); + const queries = parseQueryString(req.url.substr('/r/collect'.length)); + // see vendors/googleanalytics.js "pageview" request for config + expect(queries).to.include({ + _v: 'a1', + _r: '1', + v: '1', + cid: '1427830804.1524174812', + dr: '', + ds: 'AMP', + dt: 'AMP TEST', + tid: 'UA-67833617-1', + t: 'pageview', + }); + const isNumber = /^\d+$/; + const isRandomNumber = /^0\.\d+$/; + expect(queries['dl']).to.contain('/amp4test/compose-doc?'); // ${documentLocation} + expect(queries['_s']).to.match(isNumber); // ${requestCount} + expect(queries['_utmht']).to.match(isNumber); // ${timestamp} + expect(queries['sr']).to.match(/^\d+x\d+$/); // ${screenWidth}x${screenHeight} + expect(queries['sd']).to.match(isNumber); // ${screenColorDepth} + expect(queries['ul']).to.be.ok; // ${browserLanguage} + expect(queries['de']).to.be.ok; // ${documentCharset} + expect(queries['jid']).to.match(isRandomNumber); // ${random} + expect(queries['a']).to.match(isNumber); // ${pageViewId} + expect(queries['z']).to.match(isRandomNumber); // ${random} + }); }); }); }); diff --git a/test/integration/test-amp-bind.js b/test/integration/test-amp-bind.js index cb562ade247ee..0661418c879a1 100644 --- a/test/integration/test-amp-bind.js +++ b/test/integration/test-amp-bind.js @@ -17,7 +17,7 @@ import {AmpEvents} from '../../src/amp-events'; import {BindEvents} from '../../extensions/amp-bind/0.1/bind-events'; import {FormEvents} from '../../extensions/amp-form/0.1/form-events'; import {Services} from '../../src/services'; -import {createFixtureIframe} from '../../testing/iframe'; +import {createFixtureIframe, poll} from '../../testing/iframe'; describe.configure().ifNewChrome().run('amp-bind', function() { // Give more than default 2000ms timeout for local testing. @@ -49,12 +49,21 @@ describe.configure().ifNewChrome().run('amp-bind', function() { return createFixtureIframe(fixtureLocation).then(f => { fixture = f; // Most fixtures have a single AMP element that will be laid out. - const loadStartsToExpect = + const numberOfAmpComponents = (opt_numberOfAmpElements === undefined) ? 1 : opt_numberOfAmpElements; - return Promise.all([ + const promises = [ fixture.awaitEvent(BindEvents.INITIALIZE, 1), - fixture.awaitEvent(AmpEvents.LOAD_START, loadStartsToExpect), - ]); + ]; + if (numberOfAmpComponents > 0) { + promises.push( + poll('All AMP components are laid out', () => { + const laidOutElements = + fixture.doc.querySelectorAll('.i-amphtml-layout').length; + return laidOutElements === numberOfAmpComponents; + }) + ); + } + return Promise.all(promises); }); } @@ -70,7 +79,8 @@ describe.configure().ifNewChrome().run('amp-bind', function() { return fixture.awaitEvent(BindEvents.RESCAN_TEMPLATE, ++numTemplated); } - describe('with [text] and [class]', () => { + // TODO(choumx, #19647): Times out on SL Chrome 71. + describe.skip('with [text] and [class]', () => { beforeEach(() => { return setupWithFixture('test/fixtures/bind-basic.html'); }); @@ -98,7 +108,8 @@ describe.configure().ifNewChrome().run('amp-bind', function() { // TODO(choumx, #9759): Seems like old browsers give up when hitting // expected user errors due to illegal bindings in the form's template. - describe.configure().ifChrome().run('with ', () => { + // TODO(choumx, #19647): Times out on SL Chrome 71. + describe.configure().ifChrome().skip('with ', () => { beforeEach(() => { //
is not an AMP element. return setupWithFixture('test/fixtures/bind-form.html', 0) @@ -220,7 +231,8 @@ describe.configure().ifNewChrome().run('amp-bind', function() { }); }); - it('should change slides when the slide attribute binding changes', + // TODO(choumx, #19647): Times out on SL Chrome 71. + it.skip('should change slides when the slide attribute binding changes', () => { const carousel = fixture.doc.getElementById('carousel'); const slides = @@ -539,7 +551,8 @@ describe.configure().ifNewChrome().run('amp-bind', function() { }); }); - describe('with ', () => { + // TODO(choumx, #19647): Times out on SL Chrome 71. + describe.skip('with ', () => { beforeEach(() => { return setupWithFixture('test/fixtures/bind-list.html', 1); }); diff --git a/test/integration/test-amp-pixel.js b/test/integration/test-amp-pixel.js index c71c7b36784b2..8dc421f7160df 100644 --- a/test/integration/test-amp-pixel.js +++ b/test/integration/test-amp-pixel.js @@ -19,37 +19,28 @@ import {RequestBank} from '../../testing/test-helper'; import {Services} from '../../src/services'; import {createElementWithAttributes} from '../../src/dom'; -const RequestId = { - MACRO: 'pixel-macro', - HAS_REFERRER: 'pixel-has-referrer', - NO_REFERRER: 'pixel-no-referrer', -}; - describe.configure().skipIfPropertiesObfuscated().run('amp-pixel', function() { - this.timeout(15000); - describes.integration('amp-pixel macro integration test', { - body: ``, + body: ``, params: { a: 123, }, }, () => { it('should expand the TITLE macro', () => { - return RequestBank.withdraw(RequestId.MACRO) - .then(req => { - expect(req.url) - .to.equal('/hello-world?title=AMP%20TEST&qp=123'); - expect(req.headers.host).to.be.ok; - }); + return RequestBank.withdraw().then(req => { + expect(req.url) + .to.equal('/hello-world?title=AMP%20TEST&qp=123'); + expect(req.headers.host).to.be.ok; + }); }); }); describes.integration('amp-pixel referrer integration test', { - body: ``, + body: ``, }, () => { it('should keep referrer if no referrerpolicy specified', () => { - return RequestBank.withdraw(RequestId.HAS_REFERRER).then(req => { + return RequestBank.withdraw().then(req => { expect(req.url).to.equal('/'); expect(req.headers.referer).to.be.ok; }); @@ -57,11 +48,11 @@ describe.configure().skipIfPropertiesObfuscated().run('amp-pixel', function() { }); describes.integration('amp-pixel no-referrer integration test', { - body: ``, }, () => { it('should remove referrer if referrerpolicy=no-referrer', () => { - return RequestBank.withdraw(RequestId.NO_REFERRER).then(req => { + return RequestBank.withdraw().then(req => { expect(req.url).to.equal('/'); expect(req.headers.referer).to.not.be.ok; }); diff --git a/test/integration/test-user-error-reporting.js b/test/integration/test-user-error-reporting.js index 1b2c0e39eb34d..e65fe6cd1f5b5 100644 --- a/test/integration/test-user-error-reporting.js +++ b/test/integration/test-user-error-reporting.js @@ -16,12 +16,6 @@ import {RequestBank} from '../../testing/test-helper'; -const RequestId = { - PIXEL: 'user-error-amp-pixel', - IMG: 'user-error-amp-img', - THIRD_PARTY: 'user-error-3p', -}; - describe.configure().skipIfPropertiesObfuscated() .skipSafari().skipEdge().run('user-error', function() { //TODO(zhouyx, #11459): Unskip the test on safari. @@ -35,7 +29,7 @@ describe.configure().skipIfPropertiesObfuscated() + + + + + + + + + + + +
+ +
+ +

AMP #0

+ +

Please Read me

+

+ This page is to test if amp-analytics from ads creative gets laid out correctly. + +

+
+ Try scrolling at different speed. amp-analytics is placed at the bottom of the page within a FIE +
+ +

+
+ Look for "nowwhere" request in Network tab +
+ +

+ +

+ + Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. + +

+

+ + Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. + +

+

+ + Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. + +

+

+ + Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. + +

+

+ + Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. + +

+

+ + Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. + +

+

+ + Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. + +

+

+ + Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. + +

+

+ + Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. + +

+ + + +
Loading...
+
Could not display the fake ad :(
+
+ + +
+ Go back +
+

+ +

SVG

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

+ Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. Propriae tincidunt id nec, elit nusquam te mea, ius noster platonem in. Mea an idque minim, sit sale deleniti apeirian et. Omnium legendos tractatos cu mea. Vix in stet dolorem accusamus. Iisque rationibus consetetur in cum, quo unum nulla legere ut. Simul numquam saperet no sit. +

+

+ Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. Propriae tincidunt id nec, elit nusquam te mea, ius noster platonem in. Mea an idque minim, sit sale deleniti apeirian et. Omnium legendos tractatos cu mea. Vix in stet dolorem accusamus. Iisque rationibus consetetur in cum, quo unum nulla legere ut. Simul numquam saperet no sit. +

+

+ Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. Propriae tincidunt id nec, elit nusquam te mea, ius noster platonem in. Mea an idque minim, sit sale deleniti apeirian et. Omnium legendos tractatos cu mea. Vix in stet dolorem accusamus. Iisque rationibus consetetur in cum, quo unum nulla legere ut. Simul numquam saperet no sit. +

+

+ Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. Propriae tincidunt id nec, elit nusquam te mea, ius noster platonem in. Mea an idque minim, sit sale deleniti apeirian et. Omnium legendos tractatos cu mea. Vix in stet dolorem accusamus. Iisque rationibus consetetur in cum, quo unum nulla legere ut. Simul numquam saperet no sit. +

+

+ Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. Propriae tincidunt id nec, elit nusquam te mea, ius noster platonem in. Mea an idque minim, sit sale deleniti apeirian et. Omnium legendos tractatos cu mea. Vix in stet dolorem accusamus. Iisque rationibus consetetur in cum, quo unum nulla legere ut. Simul numquam saperet no sit. +

+ +
+
+
+
+ + diff --git a/test/manual/amp-flying-carpet-amp-ad.amp.html b/test/manual/amp-flying-carpet-amp-ad.amp.html new file mode 100644 index 0000000000000..89461c6c207f2 --- /dev/null +++ b/test/manual/amp-flying-carpet-amp-ad.amp.html @@ -0,0 +1,117 @@ + + + + + AMP flying carpet with amp ads + + + + + + + + + + + + + +
One viewport before flying carpet.
Scroll down to see demo.
+ +

Should collapse, Doubleclick / Doesn't exits Ad

+
↓ advertisement ↓
+ + +
+ + + +
+
+
+
↑ advertisement ↑
+ +
Keep Scrolling for another ad
+ +

Should Collapse, Collapsing Test _ping_ Ad

+
↓ advertisement ↓
+ + + + +
↑ advertisement ↑
+ + +
Keep Scrolling for another ad
+ +

Should not collapse, even though _ping_ Ad Collapses

+
↓ advertisement ↓
+ + + + + + +
↑ advertisement ↑
+ +
Keep Scrolling for another ad
+ +

Should not collapse, even though _ping_ Ad Collapses, in a complex case

+
↓ advertisement ↓
+ +
+
+ + +
+
+ I am a text node in the flying carpet +
+ + +
+
+
↑ advertisement ↑
+ + + +
One viewport after flying carpet
+ + + diff --git a/test/manual/amp-linker-simple.amp.html b/test/manual/amp-linker-simple.amp.html new file mode 100644 index 0000000000000..0211bc6d2766e --- /dev/null +++ b/test/manual/amp-linker-simple.amp.html @@ -0,0 +1,50 @@ + + + + + AMP Linker Simple + + + + + + + + + + + + +

AMP Linker Simple

+ + + diff --git a/test/manual/amp-list/infinite-scroll-1.amp.html b/test/manual/amp-list/infinite-scroll-1.amp.html index ead35a3df322d..eaf1749787522 100644 --- a/test/manual/amp-list/infinite-scroll-1.amp.html +++ b/test/manual/amp-list/infinite-scroll-1.amp.html @@ -7,42 +7,48 @@ @@ -66,36 +72,39 @@

AMP List

-
- +
+ OVERFLOW
-
- FAILED +
+ SEE MORE
LOADING
+
+ LOAD FAILED +
- + LOAD ENDED
- FALLBACK + FALLBACK
+ \ No newline at end of file diff --git a/test/manual/amp-list/infinite-scroll-2.amp.html b/test/manual/amp-list/infinite-scroll-2.amp.html index bfd0b411e3185..b4e2fdd205b4f 100644 --- a/test/manual/amp-list/infinite-scroll-2.amp.html +++ b/test/manual/amp-list/infinite-scroll-2.amp.html @@ -5,39 +5,52 @@ AMP #0 - + @@ -53,21 +66,52 @@ + + + + + + + +

AMP List

-

This is a flexbox amp-list with tiles, something like an infinite image gallery.

+

Infinite scroll with amp-bind.

- + + + + +
@@ -76,6 +120,15 @@

AMP List

SEE MORE
+
+ LOADING +
+
+ LOAD FAILED +
+
+ LOAD ENDED +
FALLBACK
diff --git a/testing/describes.js b/testing/describes.js index fccd44f6f2e57..b03323d678f0e 100644 --- a/testing/describes.js +++ b/testing/describes.js @@ -87,6 +87,7 @@ import { FakeWindow, interceptEventListeners, } from './fake-dom'; +import {RequestBank, stubService} from './test-helper'; import {Services} from '../src/services'; import {addParamsToUrl} from '../src/url'; import { @@ -113,7 +114,6 @@ import { resetScheduledElementForTesting, } from '../src/service/custom-element-registry'; import {setStyles} from '../src/style'; -import {stubService} from './test-helper'; import fetchMock from 'fetch-mock'; /** Should have something in the name, otherwise nothing is shown. */ @@ -344,14 +344,17 @@ function describeEnv(factory) { afterEach(() => { // Tear down all fixtures. + let teardown = Promise.resolve(); fixtures.slice(0).reverse().forEach(fixture => { - fixture.teardown(env); + teardown = teardown.then(() => fixture.teardown(env)); }); - // Delete all other keys. - for (const key in env) { - delete env[key]; - } + return teardown.then(() => { + // Delete all other keys. + for (const key in env) { + delete env[key]; + } + }); }); describe(SUB, function() { @@ -483,6 +486,7 @@ class IntegrationFixture { if (env.iframe.parentNode) { env.iframe.parentNode.removeChild(env.iframe); } + return RequestBank.tearDown(); } } @@ -583,7 +587,7 @@ class RealWinFixture { get: () => customElements, }); } else { - installCustomElements(win, class {}); + installCustomElements(win); } // Intercept event listeners diff --git a/testing/iframe.js b/testing/iframe.js index 10efbc2aaac84..53e8512037b79 100644 --- a/testing/iframe.js +++ b/testing/iframe.js @@ -237,7 +237,7 @@ export function createIframePromise(opt_runtimeOff, opt_beforeLayoutCallback) { Services.ampdocServiceFor(iframe.contentWindow).getAmpDoc(); installExtensionsService(iframe.contentWindow); installRuntimeServices(iframe.contentWindow); - installCustomElements(iframe.contentWindow, class {}); + installCustomElements(iframe.contentWindow); installAmpdocServices(ampdoc); Services.resourcesForDoc(ampdoc).ampInitComplete(); // Act like no other elements were loaded by default. @@ -477,8 +477,9 @@ export function expectBodyToBecomeVisible(win, opt_timeout) { * @param {!Window} win */ export function doNotLoadExternalResourcesInTest(win) { - const {createElement} = win.document; - win.document.createElement = function(tagName) { + const {prototype} = win.Document; + const {createElement} = prototype; + prototype.createElement = function(tagName) { const element = createElement.apply(this, arguments); tagName = tagName.toLowerCase(); if (tagName == 'iframe' || tagName == 'img') { diff --git a/testing/test-helper.js b/testing/test-helper.js index 5b7697f5de154..6fd6d192b22cd 100644 --- a/testing/test-helper.js +++ b/testing/test-helper.js @@ -135,16 +135,13 @@ export function assertScreenReaderElement(element) { expect(computedStyle.getPropertyValue('visibility')).to.equal('visible'); } - +// Use a browserId to avoid cross-browser race conditions +// when testing in Saucelabs. /** @const {string} */ -const REQUEST_URL = '//localhost:9876/amp4test/request-bank'; +const browserId = (Date.now() + Math.random()).toString(32); -/** - * Append user agent to request-bank deposit/withdraw IDs to avoid - * cross-browser race conditions when testing in Saucelabs. - * @const {string} - */ -const userAgent = encodeURIComponent(window.navigator.userAgent); +/** @const {string} */ +const REQUEST_URL = '//localhost:9876/amp4test/request-bank/' + browserId; /** * A server side temporary request storage which is useful for testing @@ -152,17 +149,18 @@ const userAgent = encodeURIComponent(window.navigator.userAgent); */ export class RequestBank { + static getBrowserId() { + return browserId; + } + /** * Returns the URL for depositing a request. * - * @param id an unique identifier of the request. + * @param {number|string|undefined} requestId * @returns {string} */ - static getUrl(id) { - if (id.indexOf('/') >= 0) { - throw new Error('ID "' + id + '" should not contain "/"'); - } - return `${REQUEST_URL}/deposit/${userAgent}-${id}/`; + static getUrl(requestId) { + return `${REQUEST_URL}/deposit/${requestId}/`; } /** @@ -173,20 +171,26 @@ export class RequestBank { * headers: JsonObject * body: string * } - * @param id + * @param {number|string|undefined} requestId * @returns {Promise} */ - static withdraw(id) { - if (id.indexOf('/') >= 0) { - throw new Error('ID "' + id + '" should not contain "/"'); - } - const url = `${REQUEST_URL}/withdraw/${userAgent}-${id}/`; + static withdraw(requestId) { + const url = `${REQUEST_URL}/withdraw/${requestId}/`; return xhrServiceForTesting(window).fetchJson(url, { method: 'GET', ampCors: false, credentials: 'omit', }).then(res => res.json()); } + + static tearDown() { + const url = `${REQUEST_URL}/teardown/`; + return xhrServiceForTesting(window).fetchJson(url, { + method: 'GET', + ampCors: false, + credentials: 'omit', + }); + } } export function createPointerEvent(type, x, y) { diff --git a/validator/engine/validator.js b/validator/engine/validator.js index 92d82c8556b46..98782c964b01d 100644 --- a/validator/engine/validator.js +++ b/validator/engine/validator.js @@ -535,6 +535,11 @@ class ParsedTagSpec { * @private */ this.shouldRecordTagspecValidated_ = shouldRecordTagspecValidated; + /** + * @type {boolean} + * @private + */ + this.attrsCanSatisfyExtension_ = false; /** * ParsedAttributes keyed by name. * @type {!Object} @@ -667,6 +672,9 @@ class ParsedTagSpec { if (spec.valueUrl) { this.containsUrl_ = true; } + if (spec.requiresExtension.length > 0) { + this.attrsCanSatisfyExtension_ = true; + } } } @@ -795,6 +803,11 @@ class ParsedTagSpec { return this.referencePoints_; } + /** @return {boolean} */ + attrsCanSatisfyExtension() { + return this.attrsCanSatisfyExtension_; + } + /** * @param {string} name * @return {boolean} @@ -2182,6 +2195,16 @@ class ExtensionsContext { return out; } + /** + * Records extensions that are used within the document. + * @param {!Array} extensions + */ + recordUsedExtensions(extensions) { + for (const extension of extensions) { + this.extensionsUsed_[extension] = true; + } + } + /** * Returns a list of unused extensions which produce validation errors * when unused. @@ -2235,8 +2258,7 @@ class ExtensionsContext { // Record presence of a tag, such as which requires the usage // of an amp extension. - for (const requiredExtension of tagSpec.requiresExtension) - {this.extensionsUsed_[requiredExtension] = true;} + this.recordUsedExtensions(tagSpec.requiresExtension); } } @@ -2441,10 +2463,46 @@ class Context { encounteredTag, referencePointResult, tagResult, this.getRules(), this.getLineCol()); + this.recordAttrRequiresExtension_(encounteredTag, referencePointResult); + this.recordAttrRequiresExtension_(encounteredTag, tagResult); this.updateFromTagResult_(referencePointResult); this.updateFromTagResult_(tagResult); } + /** + * Record when an encountered tag's attribute that requires an extension + * that it also satisfies that the requied extension is used. + * @param {!amp.htmlparser.ParsedHtmlTag} encounteredTag + * @param {!ValidateTagResult} tagResult + * @private + */ + recordAttrRequiresExtension_(encounteredTag, tagResult) { + if (tagResult.bestMatchTagSpec === null) { + return; + } + const parsedTagSpec = tagResult.bestMatchTagSpec; + if (!parsedTagSpec.attrsCanSatisfyExtension()) { + return; + } + const attrsByName = parsedTagSpec.getAttrsByName(); + const extensionsCtx = this.extensions_; + for (const attr of encounteredTag.attrs()) { + if (attr.name in attrsByName) { + const attrId = attrsByName[attr.name]; + // negative attr ids are simple attrs (only name set). + if (attrId < 0) { + continue; + } + const parsedAttrSpec = + this.rules_.getParsedAttrSpecs().getByAttrSpecId(attrId); + if (parsedAttrSpec.getSpec().requiresExtension.length > 0) { + extensionsCtx.recordUsedExtensions( + parsedAttrSpec.getSpec().requiresExtension); + } + } + } + } + /** * Record document-level conditions which have been satisfied. * @param {!ParsedTagSpec} parsedTagSpec diff --git a/validator/testdata/feature_tests/ads.html b/validator/testdata/feature_tests/ads.html index 59f4e4752d3a1..1a15b07bc34ec 100644 --- a/validator/testdata/feature_tests/ads.html +++ b/validator/testdata/feature_tests/ads.html @@ -108,6 +108,7 @@ + @@ -451,6 +452,12 @@

Atomx

data-id="1234"> +

Baidu

+ + +

Bidtellect

amoad | | +| | | | @@ -452,6 +453,12 @@ PASS | data-id="1234"> | | +|

Baidu

+| +| +| |

Bidtellect

|