amp-nexxtv-player updates #32617

neko-fire · 2021-02-11T17:20:34Z

amp-nexxtv-player updates & maintenance

♻️

optimized iframe creation
added paramter data-exit-mode
renamed album -> audioalbum
data-domain can be used now as data-client
added consent parameter from amp-consent
added load event

🚮 removed optional parameters: data-origin, data-seek-to, playlist-mask, delay
📖 updated readme and code sample

- added consent data for plugin - added playerReady State - reworked func layoutCallback

- optimized iframe creation function

- reworked param origin - removed delay - reworked client - edit docs

- updated protoascii

amp-owners-bot · 2021-02-11T17:20:42Z

Hey @alanorozco! These files were changed:

extensions/amp-nexxtv-player/0.1/amp-nexxtv-player.js
extensions/amp-nexxtv-player/0.1/test/test-amp-nexxtv-player.js
extensions/amp-nexxtv-player/amp-nexxtv-player.md

Hey @ampproject/wg-caching! These files were changed:

extensions/amp-nexxtv-player/0.1/test/validator-amp-nexxtv-player.html
extensions/amp-nexxtv-player/0.1/test/validator-amp-nexxtv-player.out
extensions/amp-nexxtv-player/validator-amp-nexxtv-player.protoascii

CLAassistant · 2021-02-11T17:20:43Z

All committers have signed the CLA.

honeybadgerdontcare · 2021-02-11T17:59:58Z

@twifkak for validator review

alanorozco

Thanks @neko-fire

extensions/amp-nexxtv-player/0.1/amp-nexxtv-player.js

extensions/amp-nexxtv-player/validator-amp-nexxtv-player.protoascii

extensions/amp-nexxtv-player/0.1/amp-nexxtv-player.js

twifkak

Thanks! Looks good for validator changes; left a couple of small requests.

extensions/amp-nexxtv-player/validator-amp-nexxtv-player.protoascii

neko-fire · 2021-03-01T10:54:51Z

Anything else that needs to be changed? @alanorozco

alanorozco

Sorry @neko-fire, I failed to clicked submit on this review previously 😳

extensions/amp-nexxtv-player/0.1/amp-nexxtv-player.js

alanorozco · 2021-02-19T17:29:59Z

extensions/amp-nexxtv-player/0.1/amp-nexxtv-player.js

+          'disableAds': disableAds,
+          'streamingFilter': streamingFilter,
+          'exitMode': exitMode,
+          'consentString': this.consentString_,


Take consentString as parameter.

Suggested change

'consentString': this.consentString_,

'consentString': consentString,

@alanorozco I've changed all your requests regarding consentString. But unit test keeps now failing due to timeout, I don't know how to fix this.

neko-fire · 2021-03-26T09:35:29Z

Any news on this topic @alanorozco

alanorozco

Looks good to me, thank you.

alanorozco · 2021-03-31T17:55:03Z

@neko-fire Please ensure that the previous tests pass. I can't merge this otherwise.

These are probably timing out since we're now waiting for a consent response before resolving layoutCallback. You should probably mock/stub the consent responses.

amp-nexxtv-player
  
  ✗ renders nexxtv video player
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

  ✗ removes iframe after unlayoutCallback
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

  ✗ should forward events from nexxtv-player to the amp element
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

  ✗ should pass consent value to iframe
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

https://app.circleci.com/pipelines/github/ampproject/amphtml/4686/workflows/10beb7b2-696f-426d-9ddc-14f4c6ea852c/jobs/61269

neko-fire · 2021-04-16T13:34:59Z

@neko-fire Please ensure that the previous tests pass. I can't merge this otherwise.

These are probably timing out since we're now waiting for a consent response before resolving layoutCallback. You should probably mock/stub the consent responses.
amp-nexxtv-player
  
  ✗ renders nexxtv video player
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

  ✗ removes iframe after unlayoutCallback
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

  ✗ should forward events from nexxtv-player to the amp element
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

  ✗ should pass consent value to iframe
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
https://app.circleci.com/pipelines/github/ampproject/amphtml/4686/workflows/10beb7b2-696f-426d-9ddc-14f4c6ea852c/jobs/61269

Hi @alanorozco , I've fixed the issue with the test.

alanorozco · 2021-04-16T15:28:26Z

@neko-fire: Since we removed playerReadyPromise to fix the test, your player might miss calls to autoplay etc. Did you manualy test with a throttled network?

neko-fire · 2021-04-16T18:07:44Z

@neko-fire: Since we removed playerReadyPromise to fix the test, your player might miss calls to autoplay etc. Did you manualy test with a throttled network?

Yes I do during my frontend test, since the player also checks the connection to play out the right quality. Also our player does not support autoplay in iframe context. I did not run into any issues here.

neko-fire · 2021-04-21T09:12:44Z

Hi @alanorozco , do you want me to change something before merge?

alanorozco · 2021-04-21T16:06:02Z

Ok, if it doesn't do autoplay then we're probably fine. Thanks for the update.

* cl/369469304 Revision bump for #33848 * cl/369546330 Revision bump for #33926 * cl/369928825 Revision bump for #32617 * cl/369967220 Require amp-video extension if amp-video is used on the page. Change from a warning. Co-authored-by: Greg Grothaus <greggrothaus@google.com>

♻️ - optimized iframe creation - added paramter data-exit-mode - renamed album -> audioalbum - data-domain can be used now as data-client - added consent parameter from amp-consent - added load event 🚮 removed optional parameters: data-origin, data-seek-to, playlist-mask, delay 📖 updated readme and code sample

* cl/369469304 Revision bump for ampproject#33848 * cl/369546330 Revision bump for ampproject#33926 * cl/369928825 Revision bump for ampproject#32617 * cl/369967220 Require amp-video extension if amp-video is used on the page. Change from a warning. Co-authored-by: Greg Grothaus <greggrothaus@google.com>

neko-fire added 4 commits February 8, 2021 18:12

nexx.tv player maintenance

d6cb7f5

- added consent data for plugin - added playerReady State - reworked func layoutCallback

nexx player added consent changes for iframe

1103729

- optimized iframe creation function

nexx player parameter changes

c5b7fa3

- reworked param origin - removed delay - reworked client - edit docs

nexx player test adjustments

326b6c7

- updated protoascii

amp-owners-bot bot requested a review from micajuine-ho February 11, 2021 17:20

honeybadgerdontcare requested a review from twifkak February 11, 2021 17:59

alanorozco requested changes Feb 11, 2021

View reviewed changes

twifkak added the WG: caching label Feb 16, 2021

neko-fire added 2 commits February 18, 2021 18:28

amp-nexxtv-player pr fixes (ampproject#32617)

70a8f73

amp-nexxtv-player added test and fixes (ampproject#32617)

707a739

honeybadgerdontcare requested review from banaag and removed request for twifkak February 18, 2021 20:33

twifkak approved these changes Feb 18, 2021

View reviewed changes

extensions/amp-nexxtv-player/validator-amp-nexxtv-player.protoascii Outdated Show resolved Hide resolved

extensions/amp-nexxtv-player/validator-amp-nexxtv-player.protoascii Outdated Show resolved Hide resolved

neko-fire requested a review from alanorozco February 19, 2021 09:03

amp-nexxtv-player alphabetize lists of values (ampproject#32617)

1e4931f

alanorozco reviewed Mar 1, 2021

View reviewed changes

neko-fire added 3 commits March 15, 2021 16:01

Merge branch 'master' into nexxtv-v9

245dad8

amp-nexxtv-player consentString changes (ampproject#32617)

c3605c4

amp-nexxtv-player updated validator (ampproject#32617)

c459130

neko-fire requested a review from alanorozco March 16, 2021 17:01

alanorozco approved these changes Mar 31, 2021

View reviewed changes

Merge branch 'master' into nexxtv-v10

8b6e126

antiphoton added the WG: Caching: Triaged Initial triage from wg-caching complete. Remove label if new input required. label Apr 12, 2021

Merge branch 'main' into nexxtv-v10

e3aa0e2

neko-fire added 2 commits April 16, 2021 13:43

amp-nexxtv-player minor test update (ampproject#32617)

a9d4e63

Merge branch 'main' into nexxtv-v10

071fee7

alanorozco merged commit 08dc823 into ampproject:main Apr 21, 2021

MichaelRybak added a commit to MichaelRybak/amphtml that referenced this pull request Apr 26, 2021

cl/369928825 Revision bump for ampproject#32617

a55bff4

ampprojectbot added the PR use: In Beta / Experimental label Apr 27, 2021

ampprojectbot added the PR use: In Stable label May 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

amp-nexxtv-player updates #32617

amp-nexxtv-player updates #32617

neko-fire commented Feb 11, 2021

amp-owners-bot bot commented Feb 11, 2021 •

edited by ampprojectbot

CLAassistant commented Feb 11, 2021 •

edited

honeybadgerdontcare commented Feb 11, 2021

alanorozco left a comment

twifkak left a comment

neko-fire commented Mar 1, 2021 •

edited

alanorozco left a comment

alanorozco Feb 19, 2021

neko-fire Mar 16, 2021

neko-fire commented Mar 26, 2021

alanorozco left a comment

alanorozco commented Mar 31, 2021

neko-fire commented Apr 16, 2021

alanorozco commented Apr 16, 2021

neko-fire commented Apr 16, 2021 •

edited

neko-fire commented Apr 21, 2021

alanorozco commented Apr 21, 2021

	'consentString': this.consentString_,
	'consentString': consentString,

amp-nexxtv-player updates #32617

amp-nexxtv-player updates #32617

Conversation

neko-fire commented Feb 11, 2021

amp-nexxtv-player updates & maintenance

amp-owners-bot bot commented Feb 11, 2021 • edited by ampprojectbot

CLAassistant commented Feb 11, 2021 • edited

honeybadgerdontcare commented Feb 11, 2021

alanorozco left a comment

Choose a reason for hiding this comment

twifkak left a comment

Choose a reason for hiding this comment

neko-fire commented Mar 1, 2021 • edited

alanorozco left a comment

Choose a reason for hiding this comment

alanorozco Feb 19, 2021

Choose a reason for hiding this comment

neko-fire Mar 16, 2021

Choose a reason for hiding this comment

neko-fire commented Mar 26, 2021

alanorozco left a comment

Choose a reason for hiding this comment

alanorozco commented Mar 31, 2021

neko-fire commented Apr 16, 2021

alanorozco commented Apr 16, 2021

neko-fire commented Apr 16, 2021 • edited

neko-fire commented Apr 21, 2021

alanorozco commented Apr 21, 2021

amp-owners-bot bot commented Feb 11, 2021 •

edited by ampprojectbot

CLAassistant commented Feb 11, 2021 •

edited

neko-fire commented Mar 1, 2021 •

edited

neko-fire commented Apr 16, 2021 •

edited