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

Improvements to ad load behavior #4022

Closed
cramforce opened this issue Jul 12, 2016 · 7 comments
Closed

Improvements to ad load behavior #4022

cramforce opened this issue Jul 12, 2016 · 7 comments

Comments

@cramforce
Copy link
Member

Currently we consider an ad loaded when the iframe loaded. For some networks (such as DFP) we can observe when actual ad render start and potentially keep showing the fallback until this time.

Similarly we can expose a promise from amp-ad that resolves when an ad was rendered and whether the slot was filled. This could be used by amp-sticky-ad to only ever show the banner at this time.

CC @jasti

@jasti
Copy link
Contributor

jasti commented Jul 15, 2016

@cramforce two things.

  1. "for showing fallback until this time" - since a fallback can be basically anything, including "We have no ads to show for you" - I don't know if we can do this by default.
  2. I like the promise idea - do ad networks have to implement some some sort of ad render start callback to invoke to trigger this promise?

@cramforce
Copy link
Member Author

Not all ad network do this. But we can make it work only where we know it is supported. Your new Team will help to announce this being supported.

@zhouyx
Copy link
Contributor

zhouyx commented Jul 19, 2016

#4124

@lannka
Copy link
Contributor

lannka commented Jul 28, 2016

Seems work is done.
For the follow up work that to make use of this event, we can open new issue to track.

@lannka
Copy link
Contributor

lannka commented Aug 19, 2016

We're making some changes to the APIs.

Right now, render-start is always expected to be called, either by 3P code or integration.js. After that, no-content might be called by 3P code.

This is bad because:

  • no-content after render-start will trigger layout twice.
  • Runtime has no way to differentiate if the render-start is called by integration.js which means only the bootstrap iframe is loaded, or by 3P code which means the ad is ready. It causes trouble for us to correctly measure ads loading latency.

The changes are:

  • Differentiate render-start called by integration.js and 3P. integration.js will call bootstrapped (naming TBD) instead. And it'll be always called.
  • Runtime will listen to render-start and no-content only once. Both APIs are supposed to be called by 3P. And the 2 events will race. Runtime will only handle whichever arrives first.
  • Runtime maintains a list of 3Ps that has implemented render-start API. For those 3P, runtime will change visibility base on render-start, otherwise base on bootstrapped.

@zhouyx

@zhouyx
Copy link
Contributor

zhouyx commented Sep 9, 2016

render-start API is re-implemented. It provides ways for multi-size ads to let AMP runtime resize. #3176. It also provides info about ad status to AMP runtime.

@jasti
Copy link
Contributor

jasti commented Oct 5, 2016

Closing this issue since render-start is implemented and publicly announced to devs.

@jasti jasti closed this as completed Oct 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants