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
Fixes amp-analytics iframeTransport regression. #25917
Conversation
…ndering, the iframeTransport should be injected in the parent window instead of iframe window.
// This behavior will be changed by ampdoc-fie launch. | ||
// TODO: this will be a blocker to launch ampdoc-fie. | ||
// We should find a different way to get the parent win in case of FIE. | ||
this.getAmpDoc().win, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you instead use getTopWindow(this.win)
? The getTopWindow
is defined in the service.js.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we do in a separate PR as this PR will be cherry picked?
Or you think it's safe enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok to move to a separate PR.
* Add an example for botguard * Fixes amp-analytics iframeTransport regression. In the case of FIE rendering, the iframeTransport should be injected in the parent window instead of iframe window. * more comments
* Add an example for botguard * Fixes amp-analytics iframeTransport regression. In the case of FIE rendering, the iframeTransport should be injected in the parent window instead of iframe window. * more comments
In the case of FIE rendering, the iframeTransport should be injected in the parent window instead of iframe window.
The regression was introduced here:
https://github.com/ampproject/amphtml/pull/25769/files#r354981761