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
🐛 amp-next-page: Fix rendering within shadow documents #16789
Conversation
If `amp-next-page` tries to load in a shadow document, the service gets registered against the AmpDoc rather than the window. Fixes ampproject#16666
8c01cb3
to
a4fb307
Compare
}); | ||
this.register_(configJson, separator); | ||
} | ||
getServicePromiseForDoc(this.getAmpDoc(), SERVICE_ID).then(service => { |
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.
Please return
the promise to eliminate any possibility of a race between buildCallback
and layoutCallback
(it is a little know fact that buildCallback
can return a promise
)
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.
Oh cool! Done.
@peterjosling Just some unit test failures related to the component. Will merge when green. Thanks! |
Done. |
Codecov Report
@@ Coverage Diff @@
## master #16789 +/- ##
==========================================
- Coverage 78.15% 78.14% -0.01%
==========================================
Files 560 561 +1
Lines 40635 40755 +120
==========================================
+ Hits 31757 31847 +90
- Misses 8878 8908 +30
Continue to review full report at Codecov.
|
If
amp-next-page
tries to load in a shadow document, the service gets registered against the AmpDoc rather than the window, andgetService()
tries to read it from the window.Fixes #16666