From 2f19ce9a9ed7efd4b0dc2958378b05be8731bb55 Mon Sep 17 00:00:00 2001 From: Corey Masanto Date: Mon, 13 Jun 2022 12:47:43 -0400 Subject: [PATCH 1/4] Move firstLayoutCompleted() logic to onVideoLoaded_() --- extensions/amp-video/0.1/amp-video.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/extensions/amp-video/0.1/amp-video.js b/extensions/amp-video/0.1/amp-video.js index 3042aebec1f0..a4f0e9d38f5e 100644 --- a/extensions/amp-video/0.1/amp-video.js +++ b/extensions/amp-video/0.1/amp-video.js @@ -793,6 +793,11 @@ export class AmpVideo extends AMP.BaseElement { /** @private */ onVideoLoaded_() { + console.log('onvideoloaded_'); + if (!this.hideBlurryPlaceholder_()) { + this.togglePlaceholder(false); + } + this.removePosterForAndroidBug_(); dispatchCustomEvent(this.element, VideoEvents_Enum.LOAD); } @@ -980,10 +985,8 @@ export class AmpVideo extends AMP.BaseElement { * @override */ firstLayoutCompleted() { - if (!this.hideBlurryPlaceholder_()) { - this.togglePlaceholder(false); - } - this.removePosterForAndroidBug_(); + console.log('firstLayoutCompleted'); + } /** @@ -991,6 +994,7 @@ export class AmpVideo extends AMP.BaseElement { * @private */ removePosterForAndroidBug_() { + console.log('removePosterForAndroidBug_()'); const poster = this.element.querySelector('i-amphtml-poster'); if (!poster) { return; From 094a20f2d2c5373860a123c40481285cadae87ab Mon Sep 17 00:00:00 2001 From: Corey Masanto Date: Mon, 13 Jun 2022 13:29:05 -0400 Subject: [PATCH 2/4] Remove console logs and the no-op firstLayoutCompleted() override --- extensions/amp-video/0.1/amp-video.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/extensions/amp-video/0.1/amp-video.js b/extensions/amp-video/0.1/amp-video.js index a4f0e9d38f5e..f58f60155449 100644 --- a/extensions/amp-video/0.1/amp-video.js +++ b/extensions/amp-video/0.1/amp-video.js @@ -793,7 +793,6 @@ export class AmpVideo extends AMP.BaseElement { /** @private */ onVideoLoaded_() { - console.log('onvideoloaded_'); if (!this.hideBlurryPlaceholder_()) { this.togglePlaceholder(false); } @@ -980,21 +979,11 @@ export class AmpVideo extends AMP.BaseElement { return ranges; } - /** - * Called when video is first loaded. - * @override - */ - firstLayoutCompleted() { - console.log('firstLayoutCompleted'); - - } - /** * See `createPosterForAndroidBug_`. * @private */ removePosterForAndroidBug_() { - console.log('removePosterForAndroidBug_()'); const poster = this.element.querySelector('i-amphtml-poster'); if (!poster) { return; From 61d1899a06e3bf66a3e937bf9dd3636cc36729f4 Mon Sep 17 00:00:00 2001 From: Corey Masanto Date: Mon, 13 Jun 2022 14:14:58 -0400 Subject: [PATCH 3/4] Update firstLayoutCompleted() calls in test-amp-video to onVideoLoaded_() --- extensions/amp-video/0.1/test/test-amp-video.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 bc710f98491f..a535724b6e96 100644 --- a/extensions/amp-video/0.1/test/test-amp-video.js +++ b/extensions/amp-video/0.1/test/test-amp-video.js @@ -1043,7 +1043,7 @@ describes.realWin( let impl = await getVideoWithBlur(true, true); impl.buildCallback(); impl.layoutCallback(); - impl.firstLayoutCompleted(); + impl.onVideoLoaded_(); let el = impl.element; let img = el.firstChild; expect(img.style.opacity).to.equal('0'); @@ -1052,7 +1052,7 @@ describes.realWin( impl = await getVideoWithBlur(true, false); impl.buildCallback(); impl.layoutCallback(); - impl.firstLayoutCompleted(); + impl.onVideoLoaded_(); el = impl.element; img = el.firstChild; expect(img.style.opacity).to.be.equal(''); @@ -1061,7 +1061,7 @@ describes.realWin( impl = await getVideoWithBlur(false, true); impl.buildCallback(); impl.layoutCallback(); - impl.firstLayoutCompleted(); + impl.onVideoLoaded_(); el = impl.element; img = el.firstChild; expect(img.style.opacity).to.be.equal(''); @@ -1070,7 +1070,7 @@ describes.realWin( impl = await getVideoWithBlur(false, false); impl.buildCallback(); impl.layoutCallback(); - impl.firstLayoutCompleted(); + impl.onVideoLoaded_(); el = impl.element; img = el.firstChild; expect(impl.togglePlaceholder).to.have.been.calledWith(false); @@ -1080,7 +1080,7 @@ describes.realWin( const impl = await getVideoWithBlur(true, true); impl.buildCallback(); impl.layoutCallback(); - impl.firstLayoutCompleted(); + impl.onVideoLoaded_(); const el = impl.element; const img = el.firstChild; expect(img.style.opacity).to.equal('0'); From ee3a41f1c4e624def39659b3f8430b10336f1301 Mon Sep 17 00:00:00 2001 From: Corey Masanto Date: Mon, 13 Jun 2022 14:48:40 -0400 Subject: [PATCH 4/4] Add back firstLayoutCompleted(), solely with its blurry placeholder logic; revert related test changes --- extensions/amp-video/0.1/amp-video.js | 15 +++++++++++---- extensions/amp-video/0.1/test/test-amp-video.js | 10 +++++----- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/extensions/amp-video/0.1/amp-video.js b/extensions/amp-video/0.1/amp-video.js index f58f60155449..2ffdeef241aa 100644 --- a/extensions/amp-video/0.1/amp-video.js +++ b/extensions/amp-video/0.1/amp-video.js @@ -793,11 +793,8 @@ export class AmpVideo extends AMP.BaseElement { /** @private */ onVideoLoaded_() { - if (!this.hideBlurryPlaceholder_()) { - this.togglePlaceholder(false); - } - this.removePosterForAndroidBug_(); dispatchCustomEvent(this.element, VideoEvents_Enum.LOAD); + this.removePosterForAndroidBug_(); } /** @override */ @@ -979,6 +976,16 @@ export class AmpVideo extends AMP.BaseElement { return ranges; } + /** + * Called when video is first loaded. + * @override + */ + firstLayoutCompleted() { + if (!this.hideBlurryPlaceholder_()) { + this.togglePlaceholder(false); + } + } + /** * See `createPosterForAndroidBug_`. * @private 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 a535724b6e96..bc710f98491f 100644 --- a/extensions/amp-video/0.1/test/test-amp-video.js +++ b/extensions/amp-video/0.1/test/test-amp-video.js @@ -1043,7 +1043,7 @@ describes.realWin( let impl = await getVideoWithBlur(true, true); impl.buildCallback(); impl.layoutCallback(); - impl.onVideoLoaded_(); + impl.firstLayoutCompleted(); let el = impl.element; let img = el.firstChild; expect(img.style.opacity).to.equal('0'); @@ -1052,7 +1052,7 @@ describes.realWin( impl = await getVideoWithBlur(true, false); impl.buildCallback(); impl.layoutCallback(); - impl.onVideoLoaded_(); + impl.firstLayoutCompleted(); el = impl.element; img = el.firstChild; expect(img.style.opacity).to.be.equal(''); @@ -1061,7 +1061,7 @@ describes.realWin( impl = await getVideoWithBlur(false, true); impl.buildCallback(); impl.layoutCallback(); - impl.onVideoLoaded_(); + impl.firstLayoutCompleted(); el = impl.element; img = el.firstChild; expect(img.style.opacity).to.be.equal(''); @@ -1070,7 +1070,7 @@ describes.realWin( impl = await getVideoWithBlur(false, false); impl.buildCallback(); impl.layoutCallback(); - impl.onVideoLoaded_(); + impl.firstLayoutCompleted(); el = impl.element; img = el.firstChild; expect(impl.togglePlaceholder).to.have.been.calledWith(false); @@ -1080,7 +1080,7 @@ describes.realWin( const impl = await getVideoWithBlur(true, true); impl.buildCallback(); impl.layoutCallback(); - impl.onVideoLoaded_(); + impl.firstLayoutCompleted(); const el = impl.element; const img = el.firstChild; expect(img.style.opacity).to.equal('0');