From b1d401ec61e660c775338c30f3908b9d7848c957 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Wed, 11 Jul 2018 18:12:57 -0400 Subject: [PATCH] Lint: isExperimentOn must be passed an explicit string The root cause of #16671 (from #16528) is difficutly grepping for used experiment flags. So, make them very grepable. ```js // BAD const EXP = 'experiment-name'; isExperimentOn(win, EXP); // GOOD isExperimentOn(win, 'experiment-name'); --- .eslintrc | 1 + ads/google/a4a/utils.js | 5 +-- build-system/eslint-rules/is-experiment-on.js | 43 +++++++++++++++++++ .../amp-access/0.1/amp-access-server-jwt.js | 2 +- .../amp-access/0.1/amp-access-server.js | 2 +- extensions/amp-access/0.1/signin.js | 2 +- .../amp-analytics/0.1/test/test-requests.js | 5 +-- .../amp-analytics/0.1/test/test-variables.js | 5 +-- extensions/amp-analytics/0.1/variables.js | 3 +- extensions/amp-auto-ads/0.1/amp-auto-ads.js | 3 +- extensions/amp-consent/0.1/amp-consent.js | 8 +--- .../amp-consent/0.1/consent-policy-manager.js | 15 +++++-- .../0.1/amp-google-document-embed.js | 4 +- .../0.1/amp-google-vrview-image.js | 18 ++------ extensions/amp-next-page/0.1/amp-next-page.js | 3 +- extensions/amp-playbuzz/0.1/amp-playbuzz.js | 6 +-- .../0.1/amp-share-tracking.js | 2 +- extensions/amp-story/0.1/amp-story.js | 2 +- extensions/amp-viz-vega/0.1/amp-viz-vega.js | 7 +-- src/experiments.js | 4 +- src/service/url-replacements-impl.js | 5 +-- src/service/viewport/viewport-impl.js | 8 +--- test/functional/test-experiments.js | 2 +- tools/experiments/experiments.js | 2 +- 24 files changed, 90 insertions(+), 67 deletions(-) create mode 100644 build-system/eslint-rules/is-experiment-on.js diff --git a/.eslintrc b/.eslintrc index 864a227baa7c..19bb72d8bfb3 100644 --- a/.eslintrc +++ b/.eslintrc @@ -62,6 +62,7 @@ "amphtml-internal/dict-string-keys": 2, "amphtml-internal/enforce-private-props": 2, "amphtml-internal/html-template": 2, + "amphtml-internal/is-experiment-on": 2, "amphtml-internal/no-array-destructuring": 2, "amphtml-internal/no-deep-destructuring": 2, "amphtml-internal/no-es2015-number-props": 2, diff --git a/ads/google/a4a/utils.js b/ads/google/a4a/utils.js index 0cdef4427ef1..8fb98139c833 100644 --- a/ads/google/a4a/utils.js +++ b/ads/google/a4a/utils.js @@ -149,14 +149,13 @@ export function isReportingEnabled(ampElement) { // a no-op (sends no pings). const type = ampElement.element.getAttribute('type'); const {win} = ampElement; - const experimentName = 'a4aProfilingRate'; // In local dev mode, neither the canary nor prod config files is available, // so manually set the profiling rate, for testing/dev. if (getMode(ampElement.win).localDev && !getMode(ampElement.win).test) { - toggleExperiment(win, experimentName, true, true); + toggleExperiment(win, 'a4aProfilingRate', true, true); } return (type == 'doubleclick' || type == 'adsense') && - isExperimentOn(win, experimentName); + isExperimentOn(win, 'a4aProfilingRate'); } /** diff --git a/build-system/eslint-rules/is-experiment-on.js b/build-system/eslint-rules/is-experiment-on.js new file mode 100644 index 000000000000..3dfb978b408f --- /dev/null +++ b/build-system/eslint-rules/is-experiment-on.js @@ -0,0 +1,43 @@ +/** + * 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. + */ +'use strict'; + +module.exports = function(context) { + const isExperimentOn = 'CallExpression[callee.name=isExperimentOn]'; + const message = 'isExperimentOn must be passed an explicit string'; + + return { + [isExperimentOn](node) { + const arg = node.arguments[1]; + if (!arg) { + context.report({node, message}); + return; + } + + const comments = context.getCommentsBefore(arg); + const ok = comments.some(comment => comment.value === 'OK'); + if (ok) { + return; + } + + if (arg.type === 'Literal' && typeof arg.value === 'string') { + return; + } + + context.report({node, message}); + }, + }; +}; diff --git a/extensions/amp-access/0.1/amp-access-server-jwt.js b/extensions/amp-access/0.1/amp-access-server-jwt.js index c680727b523b..38607820e4a6 100644 --- a/extensions/amp-access/0.1/amp-access-server-jwt.js +++ b/extensions/amp-access/0.1/amp-access-server-jwt.js @@ -109,7 +109,7 @@ export class AccessServerJwtAdapter { this.serverState_ = stateElement ? stateElement.getAttribute('content') : null; - const isInExperiment = isExperimentOn(ampdoc.win, TAG); + const isInExperiment = isExperimentOn(ampdoc.win, 'amp-access-server-jwt'); /** @private @const {boolean} */ this.isProxyOrigin_ = isProxyOrigin(ampdoc.win.location) || isInExperiment; diff --git a/extensions/amp-access/0.1/amp-access-server.js b/extensions/amp-access/0.1/amp-access-server.js index 186574f1dd08..a505cf1cf8de 100644 --- a/extensions/amp-access/0.1/amp-access-server.js +++ b/extensions/amp-access/0.1/amp-access-server.js @@ -92,7 +92,7 @@ export class AccessServerAdapter { this.serverState_ = stateElement ? stateElement.getAttribute('content') : null; - const isInExperiment = isExperimentOn(ampdoc.win, TAG); + const isInExperiment = isExperimentOn(ampdoc.win, 'amp-access-server'); /** @private @const {boolean} */ this.isProxyOrigin_ = isProxyOrigin(ampdoc.win.location) || isInExperiment; diff --git a/extensions/amp-access/0.1/signin.js b/extensions/amp-access/0.1/signin.js index 8fcb26460d95..c3ee443e93ea 100644 --- a/extensions/amp-access/0.1/signin.js +++ b/extensions/amp-access/0.1/signin.js @@ -75,7 +75,7 @@ export class SignInProtocol { /** @private @const {boolean} */ this.isEnabled_ = - isExperimentOn(ampdoc.win, TAG) && + isExperimentOn(ampdoc.win, 'amp-access-signin') && this.viewer_.isEmbedded() && this.viewer_.getParam('signin') == '1'; diff --git a/extensions/amp-analytics/0.1/test/test-requests.js b/extensions/amp-analytics/0.1/test/test-requests.js index c9e7f52d7c1f..a62d1984a545 100644 --- a/extensions/amp-analytics/0.1/test/test-requests.js +++ b/extensions/amp-analytics/0.1/test/test-requests.js @@ -16,7 +16,6 @@ import * as lolex from 'lolex'; import {ExpansionOptions, installVariableService} from '../variables'; -import {REPLACEMENT_EXP_NAME} from '../../../../src/service/url-replacements-impl'; import {RequestHandler, expandConfigRequest} from '../requests'; import {dict} from '../../../../src/utils/object'; import {macroTask} from '../../../../testing/yield'; @@ -481,7 +480,7 @@ describes.realWin('Requests', {amp: 1}, env => { it('should replace bindings with v2 flag', function* () { - toggleExperiment(env.win, REPLACEMENT_EXP_NAME, true); + toggleExperiment(env.win, 'url-replacement-v2', true); const spy = sandbox.spy(); const r = { 'baseUrl': 'r1&${extraUrlParams}&BASE_VALUE&foo=${foo}', @@ -512,6 +511,6 @@ describes.realWin('Requests', {amp: 1}, env => { expect(spy).to.be.calledOnce; expect(spy.args[0][0]).to.equal( 'r1&key1=val1&key2=val2&key3=val3&val_base&foo=ZM9V'); - toggleExperiment(env.win, REPLACEMENT_EXP_NAME); + toggleExperiment(env.win, 'url-replacement-v2'); }); }); diff --git a/extensions/amp-analytics/0.1/test/test-variables.js b/extensions/amp-analytics/0.1/test/test-variables.js index 38eb42c18932..1bfcfcc92ce2 100644 --- a/extensions/amp-analytics/0.1/test/test-variables.js +++ b/extensions/amp-analytics/0.1/test/test-variables.js @@ -21,7 +21,6 @@ import { installVariableService, variableServiceFor, } from '../variables'; -import {REPLACEMENT_EXP_NAME} from '../../../../src/service/url-replacements-impl'; import {Services} from '../../../../src/services'; import {adopt} from '../../../../src/runtime'; import {toggleExperiment} from '../../../../src/experiments'; @@ -143,14 +142,14 @@ describe('amp-analytics.VariableService', function() { beforeEach(() => { ampdoc = env.ampdoc; win = env.win; - toggleExperiment(env.win, REPLACEMENT_EXP_NAME, true); + toggleExperiment(env.win, 'url-replacement-v2', true); installVariableService(win); variables = variableServiceFor(win); urlReplacementService = Services.urlReplacementsForDoc(ampdoc); }); afterEach(() => { - toggleExperiment(env.win, REPLACEMENT_EXP_NAME); + toggleExperiment(env.win, 'url-replacement-v2'); }); function check(input, output) { diff --git a/extensions/amp-analytics/0.1/variables.js b/extensions/amp-analytics/0.1/variables.js index 496bef0832e6..3f72c9806491 100644 --- a/extensions/amp-analytics/0.1/variables.js +++ b/extensions/amp-analytics/0.1/variables.js @@ -14,7 +14,6 @@ * limitations under the License. */ -import {REPLACEMENT_EXP_NAME} from '../../../src/service/url-replacements-impl'; import {Services} from '../../../src/services'; import {dev, user} from '../../../src/log'; import {getService, registerServiceBuilder} from '../../../src/service'; @@ -151,7 +150,7 @@ export class VariableService { */ getMacros() { const isV2ExpansionOn = this.win_ && isExperimentOn(this.win_, - REPLACEMENT_EXP_NAME); + 'url-replacement-v2'); return isV2ExpansionOn ? this.macros_ : {}; } diff --git a/extensions/amp-auto-ads/0.1/amp-auto-ads.js b/extensions/amp-auto-ads/0.1/amp-auto-ads.js index 2a22d1620317..157f027750ba 100644 --- a/extensions/amp-auto-ads/0.1/amp-auto-ads.js +++ b/extensions/amp-auto-ads/0.1/amp-auto-ads.js @@ -39,7 +39,8 @@ export class AmpAutoAds extends AMP.BaseElement { /** @override */ buildCallback() { - user().assert(isExperimentOn(this.win, TAG), 'Experiment is off'); + user().assert(isExperimentOn(this.win, 'amp-auto-ads'), + 'Experiment is off'); const type = this.element.getAttribute('type'); user().assert(type, 'Missing type attribute'); diff --git a/extensions/amp-consent/0.1/amp-consent.js b/extensions/amp-consent/0.1/amp-consent.js index e46428ab0fae..302e4e52ced4 100644 --- a/extensions/amp-consent/0.1/amp-consent.js +++ b/extensions/amp-consent/0.1/amp-consent.js @@ -17,10 +17,7 @@ import {CONSENT_ITEM_STATE, ConsentStateManager} from './consent-state-manager'; import {CONSENT_POLICY_STATE} from '../../../src/consent-state'; import {CSS} from '../../../build/amp-consent-0.1.css'; -import { - ConsentPolicyManager, - MULTI_CONSENT_EXPERIMENT, -} from './consent-policy-manager'; +import {ConsentPolicyManager} from './consent-policy-manager'; import {Deferred} from '../../../src/utils/promise'; import { NOTIFICATION_UI_MANAGER, @@ -38,7 +35,6 @@ import {dict, map} from '../../../src/utils/object'; import {getData} from '../../../src/event-helper'; import {getServicePromiseForDoc} from '../../../src/service'; import {isEnumValue} from '../../../src/types'; -import {isExperimentOn} from '../../../src/experiments'; import {parseJson} from '../../../src/json'; import {setImportantStyles, toggle} from '../../../src/style'; @@ -128,7 +124,7 @@ export class AmpConsent extends AMP.BaseElement { /** @override */ buildCallback() { - this.isMultiSupported_ = isExperimentOn(this.win, MULTI_CONSENT_EXPERIMENT); + this.isMultiSupported_ = ConsentPolicyManager.isMultiSupported(this.win); user().assert(this.element.getAttribute('id'), 'amp-consent should have an id'); diff --git a/extensions/amp-consent/0.1/consent-policy-manager.js b/extensions/amp-consent/0.1/consent-policy-manager.js index db5d61a079d4..1001892953a3 100644 --- a/extensions/amp-consent/0.1/consent-policy-manager.js +++ b/extensions/amp-consent/0.1/consent-policy-manager.js @@ -24,7 +24,6 @@ import {isExperimentOn} from '../../../src/experiments'; import {isFiniteNumber} from '../../../src/types'; import {isObject} from '../../../src/types'; -export const MULTI_CONSENT_EXPERIMENT = 'multi-consent'; const CONSENT_STATE_MANAGER = 'consentStateManager'; const TAG = 'consent-policy-manager'; @@ -68,6 +67,16 @@ export class ConsentPolicyManager { } + /** + * Is Multi-consent experiment enabled? + * + * @param {!Window} win + * @return {boolean} + */ + static isMultiSupported(win) { + return isExperimentOn(win, 'multi-consent'); + } + /** * Register the policy instance * Example policy config format: @@ -150,7 +159,7 @@ export class ConsentPolicyManager { * @return {!Promise} */ whenPolicyResolved(policyId) { - if (!isExperimentOn(this.ampdoc_.win, MULTI_CONSENT_EXPERIMENT)) { + if (!ConsentPolicyManager.isMultiSupported(this.ampdoc_.win)) { // If customized policy is not supported if (!WHITELIST_POLICY[policyId]) { user().error(TAG, `can not find defined policy ${policyId}, ` + @@ -171,7 +180,7 @@ export class ConsentPolicyManager { * @return {!Promise} */ whenPolicyUnblock(policyId) { - if (!isExperimentOn(this.ampdoc_.win, MULTI_CONSENT_EXPERIMENT)) { + if (!ConsentPolicyManager.isMultiSupported(this.ampdoc_.win)) { // If customized policy is not supported if (!WHITELIST_POLICY[policyId]) { user().error(TAG, `can not find defined policy ${policyId}, ` + diff --git a/extensions/amp-google-document-embed/0.1/amp-google-document-embed.js b/extensions/amp-google-document-embed/0.1/amp-google-document-embed.js index edef27565ede..cea420f02190 100644 --- a/extensions/amp-google-document-embed/0.1/amp-google-document-embed.js +++ b/extensions/amp-google-document-embed/0.1/amp-google-document-embed.js @@ -75,8 +75,8 @@ export class AmpDriveViewer extends AMP.BaseElement { /** @override */ buildCallback() { - user().assert( - isExperimentOn(this.win, TAG), `Experiment ${TAG} is disabled`); + user().assert(isExperimentOn(this.win, 'amp-google-document-embed'), + 'Experiment amp-google-document-embed is disabled'); user().assert( this.element.getAttribute('src'), 'The src attribute is required for %s', diff --git a/extensions/amp-google-vrview-image/0.1/amp-google-vrview-image.js b/extensions/amp-google-vrview-image/0.1/amp-google-vrview-image.js index 9ab4e245e5e4..ba6604799fdd 100644 --- a/extensions/amp-google-vrview-image/0.1/amp-google-vrview-image.js +++ b/extensions/amp-google-vrview-image/0.1/amp-google-vrview-image.js @@ -15,9 +15,9 @@ */ import {addParamToUrl, assertHttpsUrl} from '../../../src/url'; -import {dev} from '../../../src/log'; import {isExperimentOn} from '../../../src/experiments'; import {isLayoutSizeDefined} from '../../../src/layout'; +import {user} from '../../../src/log'; /** @const */ const TAG = 'amp-google-vrview-image'; @@ -29,9 +29,6 @@ class AmpGoogleVrviewImage extends AMP.BaseElement { constructor(element) { super(element); - /** @private {boolean} */ - this.isExperimentOn_ = false; - /** @private {string} */ this.imageSrc_ = ''; @@ -47,11 +44,8 @@ class AmpGoogleVrviewImage extends AMP.BaseElement { /** @override */ buildCallback() { - this.isExperimentOn_ = isExperimentOn(this.win, TAG); - if (!this.isExperimentOn_) { - dev().warn(TAG, `TAG ${TAG} disabled`); - return; - } + user().assert(isExperimentOn(this.win, 'amp-google-vrview-image'), + 'TAG amp-google-vrview-image disabled'); this.imageSrc_ = assertHttpsUrl(this.element.getAttribute('src'), this.element); @@ -90,12 +84,6 @@ class AmpGoogleVrviewImage extends AMP.BaseElement { /** @override */ layoutCallback() { - this.isExperimentOn_ = isExperimentOn(this.win, TAG); - if (!this.isExperimentOn_) { - dev().warn(TAG, `TAG ${TAG} disabled`); - return Promise.resolve(); - } - const iframe = this.element.ownerDocument.createElement('iframe'); iframe.onload = () => { // Chrome does not reflect the iframe readystate. diff --git a/extensions/amp-next-page/0.1/amp-next-page.js b/extensions/amp-next-page/0.1/amp-next-page.js index d82ca39999a4..21a1bef8c7fc 100644 --- a/extensions/amp-next-page/0.1/amp-next-page.js +++ b/extensions/amp-next-page/0.1/amp-next-page.js @@ -50,7 +50,8 @@ export class AmpNextPage extends AMP.BaseElement { /** @override */ buildCallback() { - user().assert(isExperimentOn(this.win, TAG), `Experiment ${TAG} disabled`); + user().assert(isExperimentOn(this.win, 'amp-next-page'), + 'Experiment amp-next-page disabled'); if (this.service_.isActive()) { return; diff --git a/extensions/amp-playbuzz/0.1/amp-playbuzz.js b/extensions/amp-playbuzz/0.1/amp-playbuzz.js index d18cdafdc992..4125c289d39f 100644 --- a/extensions/amp-playbuzz/0.1/amp-playbuzz.js +++ b/extensions/amp-playbuzz/0.1/amp-playbuzz.js @@ -53,8 +53,6 @@ import {isExperimentOn} from '../../../src/experiments'; import {logo, showMoreArrow} from './images'; import {removeElement} from '../../../src/dom'; import {user} from '../../../src/log'; -/** @const */ -const EXPERIMENT = 'amp-playbuzz'; class AmpPlaybuzz extends AMP.BaseElement { @@ -105,8 +103,8 @@ class AmpPlaybuzz extends AMP.BaseElement { buildCallback() { // EXPERIMENT // AMP.toggleExperiment(EXPERIMENT, true); //for dev - user().assert(isExperimentOn(this.win, EXPERIMENT), - `Enable ${EXPERIMENT} experiment`); + user().assert(isExperimentOn(this.win, 'amp-playbuzz'), + 'Enable amp-playbuzz experiment'); const e = this.element; const src = e.getAttribute('src'); diff --git a/extensions/amp-share-tracking/0.1/amp-share-tracking.js b/extensions/amp-share-tracking/0.1/amp-share-tracking.js index 271b31bb61ee..5404e013d5de 100644 --- a/extensions/amp-share-tracking/0.1/amp-share-tracking.js +++ b/extensions/amp-share-tracking/0.1/amp-share-tracking.js @@ -51,7 +51,7 @@ export class AmpShareTracking extends AMP.BaseElement { * @private */ isExperimentOn_() { - return isExperimentOn(this.win, TAG); + return isExperimentOn(this.win, 'amp-share-tracking'); } /** @override */ diff --git a/extensions/amp-story/0.1/amp-story.js b/extensions/amp-story/0.1/amp-story.js index d1bea32c1415..18a79d4075c9 100644 --- a/extensions/amp-story/0.1/amp-story.js +++ b/extensions/amp-story/0.1/amp-story.js @@ -678,7 +678,7 @@ export class AmpStory extends AMP.BaseElement { /** @private */ isAmpStoryEnabled_() { - if (isExperimentOn(this.win, TAG) || getMode().test || + if (isExperimentOn(this.win, 'amp-story') || getMode().test || this.win.location.protocol === 'file:') { return true; } diff --git a/extensions/amp-viz-vega/0.1/amp-viz-vega.js b/extensions/amp-viz-vega/0.1/amp-viz-vega.js index 1830981cf7ad..c71c9b71dc02 100644 --- a/extensions/amp-viz-vega/0.1/amp-viz-vega.js +++ b/extensions/amp-viz-vega/0.1/amp-viz-vega.js @@ -24,9 +24,6 @@ import {isFiniteNumber, isObject} from '../../../src/types'; import {isLayoutSizeDefined} from '../../../src/layout'; import {tryParseJson} from '../../../src/json'; -/** @const */ -const EXPERIMENT = 'amp-viz-vega'; - export class AmpVizVega extends AMP.BaseElement { /** @param {!AmpElement} element */ @@ -74,8 +71,8 @@ export class AmpVizVega extends AMP.BaseElement { /** @override */ buildCallback() { - user().assert(isExperimentOn(this.win, EXPERIMENT), - `Experiment ${EXPERIMENT} disabled`); + user().assert(isExperimentOn(this.win, 'amp-viz-vega'), + 'Experiment amp-viz-vega disabled'); /** * Global vg (and implicitly d3) are required and they are created by diff --git a/src/experiments.js b/src/experiments.js index 0c20d9ed9015..dbb22daa96b3 100644 --- a/src/experiments.js +++ b/src/experiments.js @@ -88,7 +88,7 @@ export function isExperimentOn(win, experimentId) { */ export function toggleExperiment(win, experimentId, opt_on, opt_transientExperiment) { - const currentlyOn = isExperimentOn(win, experimentId); + const currentlyOn = isExperimentOn(win, /*OK*/experimentId); const on = !!(opt_on !== undefined ? opt_on : !currentlyOn); if (on != currentlyOn) { const toggles = experimentToggles(win); @@ -316,7 +316,7 @@ export function randomlySelectUnsetExperiments(win, experiments) { // experiment branch (e.g., via a test setup), then randomize the branch // choice. if (!win.experimentBranches[experimentName] && - isExperimentOn(win, experimentName)) { + isExperimentOn(win, /*OK*/experimentName)) { const {branches} = experiments[experimentName]; win.experimentBranches[experimentName] = selectRandomItem(branches); selectedExperiments[experimentName] = diff --git a/src/service/url-replacements-impl.js b/src/service/url-replacements-impl.js index 35b7e8184d66..5efc7dcca121 100644 --- a/src/service/url-replacements-impl.js +++ b/src/service/url-replacements-impl.js @@ -53,9 +53,6 @@ const GEO_DELIM = ','; const ORIGINAL_HREF_PROPERTY = 'amp-original-href'; const ORIGINAL_VALUE_PROPERTY = 'amp-original-value'; -/** @const {string} */ -export const REPLACEMENT_EXP_NAME = 'url-replacement-v2'; - /** * Returns a encoded URI Component, or an empty string if the value is nullish. * @param {*} val @@ -1048,7 +1045,7 @@ export class UrlReplacements { */ expand_(url, opt_bindings, opt_collectVars, opt_sync, opt_whiteList) { const isV2ExperimentOn = isExperimentOn(this.ampdoc.win, - REPLACEMENT_EXP_NAME); + 'url-replacement-v2'); if (isV2ExperimentOn) { // TODO(ccordy) support opt_collectVars && opt_whitelist return this.expander_./*OK*/expand(url, opt_bindings, opt_collectVars, diff --git a/src/service/viewport/viewport-impl.js b/src/service/viewport/viewport-impl.js index d0f405190885..a7e2b642a609 100644 --- a/src/service/viewport/viewport-impl.js +++ b/src/service/viewport/viewport-impl.js @@ -47,10 +47,6 @@ import {setStyle} from '../../style'; const TAG_ = 'Viewport'; -/** @const {string} */ -const A4A_LIGHTBOX_EXPERIMENT = 'amp-lightbox-a4a-proto'; - - /** * @typedef {{ * relayoutAll: boolean, @@ -628,7 +624,7 @@ export class Viewport { * @visibleForTesting */ isLightboxExperimentOn() { - return isExperimentOn(this.ampdoc.win, A4A_LIGHTBOX_EXPERIMENT); + return isExperimentOn(this.ampdoc.win, 'amp-lightbox-a4a-proto'); } /** @@ -642,7 +638,7 @@ export class Viewport { if (fieOptional) { dev().assert(this.isLightboxExperimentOn(), 'Lightbox mode for A4A is only available when ' + - `'${A4A_LIGHTBOX_EXPERIMENT}' experiment is on`); + "'amp-lightbox-a4a-proto' experiment is on"); dev().assert(fieOptional).enterFullOverlayMode(); } diff --git a/test/functional/test-experiments.js b/test/functional/test-experiments.js index 79e857d4c527..f78056cac5d7 100644 --- a/test/functional/test-experiments.js +++ b/test/functional/test-experiments.js @@ -122,7 +122,7 @@ describe('isExperimentOn', () => { function expectExperiment(cookieString, experimentId) { resetExperimentTogglesForTesting(win); win.document.cookie = cookieString; - return expect(isExperimentOn(win, experimentId)); + return expect(isExperimentOn(win, /*OK*/experimentId)); } describe('with only cookie flag', () => { diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index 2beb133de25e..af44dcce28d3 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -445,7 +445,7 @@ function isExperimentOn_(id) { if (id == CANARY_EXPERIMENT_ID) { return getCookie(window, 'AMP_CANARY') == '1'; } - return isExperimentOn(window, id); + return isExperimentOn(window, /*OK*/id); } /**