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.

Sign up for GitHub

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

Jump to bottom

A4A resolve layoutCallback early when renderViaCrossDomainIframe #5472

Closed
zhouyx opened this issue Oct 8, 2016 · 0 comments
Closed

A4A resolve layoutCallback early when renderViaCrossDomainIframe #5472

zhouyx opened this issue Oct 8, 2016 · 0 comments
Assignees
@keithwrightbos @tdrl
Labels
Type: Bug WG: monetization
Milestone
Pending Triage

Comments

@zhouyx
Copy link
Contributor

zhouyx commented Oct 8, 2016

I found that if an A4A decide to render as 3p ad it will call this.renderViaCrossDomainIframe_ and resolve the #layoutCallback immediately after that.

However in 3PAdImpl, we will only resolve #layoutCallback when this apiHandler.startUp promise is resolved. https://github.com/ampproject/amphtml/blob/master/extensions/amp-ad/0.1/amp-ad-3p-impl.js#L217

I think we should keep the timestamp same in those two cases.
@ericlindley-g ericlindley-g added this to the Pending Triage milestone Oct 13, 2016
@zhouyx zhouyx mentioned this issue Oct 26, 2016
Closed
@keithwrightbos keithwrightbos mentioned this issue Oct 28, 2016
Merged
lannka pushed a commit that referenced this issue Oct 31, 2016
@keithwrightbos @lannka
Fix A4A issues #5472 & #5273 (#5882)
399124d 
* Update amp-ad-network-doubleclick-impl.md

* Reverting doc changes

The previous change was made in error (wrong branch).

* Address issues #5472 (ensure AmpAdXOriginIframeHandler.init promise is returned as part of layoutCallback) & #5273 (only increment 3p ad throttling when creative known not to be AMP)

* address PR feedback

* Fix lint errors

* fix type check errors

* Add tag to dev info
Lith pushed a commit to Lith/amphtml that referenced this issue Dec 22, 2016
@keithwrightbos
Fix A4A issues ampproject#5472 & ampproject#5273 (ampproject#5882)
e445faa 
* Update amp-ad-network-doubleclick-impl.md

* Reverting doc changes

The previous change was made in error (wrong branch).

* Address issues ampproject#5472 (ensure AmpAdXOriginIframeHandler.init promise is returned as part of layoutCallback) & ampproject#5273 (only increment 3p ad throttling when creative known not to be AMP)

* address PR feedback

* Fix lint errors

* fix type check errors

* Add tag to dev info
Lith pushed a commit to Lith/amphtml that referenced this issue Dec 22, 2016
@keithwrightbos
Fix A4A issues ampproject#5472 & ampproject#5273 (ampproject#5882)
f9f908b 
* Update amp-ad-network-doubleclick-impl.md

* Reverting doc changes

The previous change was made in error (wrong branch).

* Address issues ampproject#5472 (ensure AmpAdXOriginIframeHandler.init promise is returned as part of layoutCallback) & ampproject#5273 (only increment 3p ad throttling when creative known not to be AMP)

* address PR feedback

* Fix lint errors

* fix type check errors

* Add tag to dev info
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@keithwrightbos keithwrightbos

@tdrl tdrl

Labels
Type: Bug WG: monetization
Projects
None yet
Milestone
Pending Triage
Development

No branches or pull requests
4 participants
@keithwrightbos @zhouyx @ericlindley-g @tdrl