Skip to content

Commit

Permalink
Validate auto-advance-after element. (#27574)
Browse files Browse the repository at this point in the history 
* Validate auto-advance-after element.

* Unit tests.
  • Loading branch information
@gmajoulet
gmajoulet committed Apr 6, 2020
1 parent 1b0d4cd commit 30de6bf
Show file tree
Hide file tree
Showing 2 changed files with 207 additions and 8 deletions.
32 changes: 24 additions & 8 deletions extensions/amp-story/1.0/page-advancement.js
View file
Expand Up @@ -237,7 +237,7 @@ export class AdvancementConfig {
* Always provides a progress of 1.0. Advances when the user taps the
* corresponding section, depending on language settings.
*/
class ManualAdvancement extends AdvancementConfig {
export class ManualAdvancement extends AdvancementConfig {
/**
* @param {!Window} win The Window object.
* @param {!Element} element The element that, when clicked, can cause
Expand Down Expand Up @@ -657,7 +657,7 @@ class ManualAdvancement extends AdvancementConfig {
* Provides progress and advancement based on a fixed duration of time,
* specified in either seconds or milliseconds.
*/
class TimeBasedAdvancement extends AdvancementConfig {
export class TimeBasedAdvancement extends AdvancementConfig {
/**
* @param {!Window} win The Window object.
* @param {number} delayMs The duration to wait before advancing.
Expand Down Expand Up @@ -799,7 +799,7 @@ class TimeBasedAdvancement extends AdvancementConfig {
* having been executed before the amp-story-page buildCallback, which is not
* guaranteed.
*/
class MediaBasedAdvancement extends AdvancementConfig {
export class MediaBasedAdvancement extends AdvancementConfig {
/**
* @param {!Window} win
* @param {!Array<!Element>} elements
Expand Down Expand Up @@ -909,7 +909,7 @@ class MediaBasedAdvancement extends AdvancementConfig {
return this.element_;
} else if (
this.element_.hasAttribute('background-audio') &&
(tagName === 'amp-story' || tagName === 'amp-story-page')
tagName === 'amp-story-page'
) {
return this.element_.querySelector('.i-amphtml-story-background-audio');
} else if (tagName === 'amp-audio') {
Expand Down Expand Up @@ -1051,10 +1051,26 @@ class MediaBasedAdvancement extends AdvancementConfig {
*/
static fromAutoAdvanceString(autoAdvanceStr, win, element) {
try {
const elements = element.querySelectorAll(
`[data-id=${escapeCssSelectorIdent(autoAdvanceStr)}],
#${escapeCssSelectorIdent(autoAdvanceStr)}`
// amp-video, amp-audio, as well as amp-story-page with a background audio
// are eligible for media based auto advance.
const elements = toArray(
element.querySelectorAll(
`amp-video[data-id=${escapeCssSelectorIdent(autoAdvanceStr)}],
amp-video#${escapeCssSelectorIdent(autoAdvanceStr)},
amp-audio[data-id=${escapeCssSelectorIdent(autoAdvanceStr)}],
amp-audio#${escapeCssSelectorIdent(autoAdvanceStr)}`
)
);
if (
matches(
element,
`amp-story-page[background-audio]#${escapeCssSelectorIdent(
autoAdvanceStr
)}`
)
) {
elements.push(element);
}
if (!elements.length) {
if (autoAdvanceStr) {
user().warn(
Expand All @@ -1066,7 +1082,7 @@ class MediaBasedAdvancement extends AdvancementConfig {
return null;
}

return new MediaBasedAdvancement(win, toArray(elements));
return new MediaBasedAdvancement(win, elements);
} catch (e) {
return null;
}
Expand Down
183 changes: 183 additions & 0 deletions extensions/amp-story/1.0/test/test-page-advancement.js
View file
@@ -0,0 +1,183 @@
/**
* Copyright 2020 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 {
AdvancementConfig,
ManualAdvancement,
MediaBasedAdvancement,
TimeBasedAdvancement,
} from '../page-advancement';
import {htmlFor} from '../../../../src/static-template';

describes.realWin('page-advancement', {amp: true}, (env) => {
let html;
let win;

beforeEach(() => {
html = htmlFor(env.win.document);
win = env.win;
});

describe('forElement', () => {
describe('ManualAdvancement', () => {
it('should return a manual advancement for an amp-story element', () => {
const storyEl = win.document.createElement('amp-story');
const advancement = AdvancementConfig.forElement(win, storyEl);

expect(advancement).to.be.instanceOf(ManualAdvancement);
});

it('should not return a manual advancement for a non amp-story element', () => {
const pageEl = win.document.createElement('amp-story-page');
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.not.be.instanceOf(ManualAdvancement);
});
});

describe('TimeBasedAdvancement', () => {
it('should return a time advancement from ms unit', () => {
const pageEl = html`
<amp-story-page auto-advance-after="1000ms"> </amp-story-page>
`;
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.be.instanceOf(TimeBasedAdvancement);
});

it('should return a time advancement from s unit', () => {
const pageEl = html`
<amp-story-page auto-advance-after="3s"> </amp-story-page>
`;
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.be.instanceOf(TimeBasedAdvancement);
});

it('should not return a time advancement if no unit', () => {
const pageEl = html`
<amp-story-page auto-advance-after="3"> </amp-story-page>
`;
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.not.be.instanceOf(TimeBasedAdvancement);
});

it('should not return a time advancement if random string', () => {
const pageEl = html`
<amp-story-page auto-advance-after="idkwhatimdoing"> </amp-story-page>
`;
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.not.be.instanceOf(TimeBasedAdvancement);
});
});

describe('MediaBasedAdvancement', () => {
it('should return a media advancement for amp-video with id', () => {
const pageEl = html`
<amp-story-page auto-advance-after="video-id">
<amp-story-grid-layer>
<amp-video id="video-id"></amp-video>
</amp-story-grid-layer>
</amp-story-page>
`;
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.be.instanceOf(MediaBasedAdvancement);
});

it('should return a media advancement for amp-video with data-id', () => {
const pageEl = html`
<amp-story-page auto-advance-after="video-id">
<amp-story-grid-layer>
<amp-video data-id="video-id"></amp-video>
</amp-story-grid-layer>
</amp-story-page>
`;
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.be.instanceOf(MediaBasedAdvancement);
});

it('should return a media advancement for amp-audio with id', () => {
const pageEl = html`
<amp-story-page auto-advance-after="audio-id">
<amp-story-grid-layer>
<amp-audio id="audio-id"></amp-audio>
</amp-story-grid-layer>
</amp-story-page>
`;
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.be.instanceOf(MediaBasedAdvancement);
});

it('should return a media advancement for amp-audio with data-id', () => {
const pageEl = html`
<amp-story-page auto-advance-after="audio-id">
<amp-story-grid-layer>
<amp-audio data-id="audio-id"></amp-audio>
</amp-story-grid-layer>
</amp-story-page>
`;
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.be.instanceOf(MediaBasedAdvancement);
});

it('should return a media advancement for amp-story-page with background audio', () => {
const pageEl = html`
<amp-story-page
id="story-id"
auto-advance-after="story-id"
background-audio="foo.mp3"
>
</amp-story-page>
`;
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.be.instanceOf(MediaBasedAdvancement);
});

it('should not return a media based advancement for other elements', () => {
const pageEl = html`
<amp-story-page auto-advance-after="div-id">
<amp-story-grid-layer>
<div-id="div-id"></div>
</amp-story-grid-layer>
</amp-story-page>
`;
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.not.be.instanceOf(MediaBasedAdvancement);
});

it('should not return a media based advancement if id does not exist', () => {
const pageEl = html`
<amp-story-page auto-advance-after="unknown-id">
<amp-story-grid-layer>
<amp-video id="video-id"></amp-video>
</amp-story-grid-layer>
</amp-story-page>
`;
const advancement = AdvancementConfig.forElement(win, pageEl);

expect(advancement).to.not.be.instanceOf(MediaBasedAdvancement);
});
});
});
});

0 comments on commit 30de6bf

Please sign in to comment.