Skip to content

Append ad iframe _after_ registering listeners#2942

Merged
erwinmombay merged 1 commit intoampproject:masterfrom
jridgewell:fix-ad-listener
Apr 19, 2016
Merged

Append ad iframe _after_ registering listeners#2942
erwinmombay merged 1 commit intoampproject:masterfrom
jridgewell:fix-ad-listener

Conversation

@jridgewell
Copy link
Copy Markdown
Contributor

When the ad's iframe is already cached, we can end up running it's code
before we register our event listeners on it. That really stinks for
ads, because they need to send a render-start event to us for us to
show them and we're not even listening for it when they send. :sadpanda:

Fixes #2933.

When the ad's iframe is already cached, we can end up running it's code
before we register our event listeners on it. That really stinks for
ads, because they need to send a `render-start` event to us for us to
show them and we're not even listening for it when they send. :sadpanda:
@cramforce
Copy link
Copy Markdown
Member

Great catch!

LGTM

@erwinmombay erwinmombay merged commit 2a7b33c into ampproject:master Apr 19, 2016
@camelburrito
Copy link
Copy Markdown
Contributor

wow!!
LGTM

please add an assert on iframe_helper to warn when this happens!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants