Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove amp-youtube from AMPHTML Ads spec #21340

Closed
8 tasks done
lannka opened this issue Mar 8, 2019 · 11 comments
Closed
8 tasks done

Remove amp-youtube from AMPHTML Ads spec #21340

lannka opened this issue Mar 8, 2019 · 11 comments

Comments

@lannka
Copy link
Contributor

lannka commented Mar 8, 2019

The current amp-youtube implementation relies on a cross-origin iframe which brings uncertainty to the performance of the ad. In general, AMPHTML ads tries to avoid any use of cross-origin iframe. Here we propose to deprecate the use of amp-youtube in AMPHTML ads. To follow the deprecation policy, here is the list of checkboxes:

  • Allow for at least 2 weeks of open discussion.
  • Wait for 3 approvals from core committers.
  • Update spec
  • Announce change on the mailing list.
  • Start warning for ads that might break via the developer console.
  • Give developers 6 weeks to apply changes. (8/23/2019)
  • Update validator
  • Remove the warning

/cc @keithwrightbos @aghassemi @jasti @honeybadgerdontcare

@honeybadgerdontcare
Copy link
Contributor

Is this the i2i to satisfy the deprecation policy?

@lannka lannka changed the title Remove amp-youtube from A4A spec Remove amp-youtube from AMPHTML Ads spec Mar 8, 2019
@lannka
Copy link
Contributor Author

lannka commented Mar 8, 2019

@honeybadgerdontcare good point. Updated the issue to follow the process.

@lannka
Copy link
Contributor Author

lannka commented Mar 8, 2019

@ampproject/wg-ads @ampproject/a4a @ampproject/ads

@yazdotdev
Copy link

@lannka @honeybadgerdontcare by the way, the placeholder image for amp youtube is jpg and not webp. any chance of changing this? it might be more a youtube team rather than amp team issue though.

@lannka
Copy link
Contributor Author

lannka commented Mar 11, 2019

@yazdotdev thanks for bringing it up. I created a separate issue to discuss this #21357

@honeybadgerdontcare
Copy link
Contributor

@lannka I approve this of this change

@lannka
Copy link
Contributor Author

lannka commented Mar 12, 2019

thanks @honeybadgerdontcare
collecting more stamps: @keithwrightbos @aghassemi @jasti @zhouyx

@jasti
Copy link
Contributor

jasti commented Mar 18, 2019

Approved!

jasti pushed a commit that referenced this issue Mar 19, 2019
Will be properly retired as part of #21340 but removing documentation so it doesn't cause further developer confusion.

CC @ampproject/a4a
zhouyx pushed a commit that referenced this issue Mar 29, 2019
Will be properly retired as part of #21340 but removing documentation so it doesn't cause further developer confusion.

CC @ampproject/a4a
@jeffkaufman
Copy link
Contributor

strongly in favor

@calebcordry
Copy link
Member

@lannka Are we ready to announce this change on the mailing list?

@calebcordry calebcordry added this to Needs triage in Ads & Analytics issue triaging via automation Jul 2, 2019
@calebcordry calebcordry moved this from Needs triage to Triaged in Ads & Analytics issue triaging Jul 2, 2019
@lannka lannka added the INTENT TO DEPRECATE Proposes deprecating an existing AMP feature. label Jul 11, 2019
@lannka
Copy link
Contributor Author

lannka commented Jul 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

6 participants