Skip to content

Commit

Permalink
Fix localization service retrieval. (#28440)
Browse files Browse the repository at this point in the history
Story navigation was completely blocked after prerendering the education screens, because somehow the `pointer-events: none;` was not respected and the `amp-story-education` element was acting as a click shield. Hiding it entirely works though, and does not change the animations.

I've also seen race conditions where we try to access the ampdoc too early through the `getLocalizationService` method.

To verify the fix:
1. Open the link in mobile emulation
2. In the console, copy/paste `AMP.viewer.receiveMessage('visibilitychange', {'state': 'visible'})`
3. Try to navigate to the next page

[This story has the fix](https://stamp-storytime-le.firebaseapp.com/examples/s20/body-painting/index.html?amp_js_v=0.1#origin=https://www.google.com&prerenderSize=1&visibilityState=prerender&paddingTop=0&p2r=0&horizontalScrolling=0&csi=1&aoh=15526927198732&viewerUrl=https://www.google.com&history=1&storage=1&cid=1&cap=swipe,navigateTo,cid,fragment,replaceUrl,education)
[This story should fail 80% of the time (no fix)](https://stamp-storytime-le-fail.firebaseapp.com/examples/s20/body-painting/index.html?amp_js_v=0.1#origin=https://www.google.com&prerenderSize=1&visibilityState=prerender&paddingTop=0&p2r=0&horizontalScrolling=0&csi=1&aoh=15526927198732&viewerUrl=https://www.google.com&history=1&storage=1&cid=1&cap=swipe,navigateTo,cid,fragment,replaceUrl,education)

(cherry picked from commit 6f561cb)
  • Loading branch information
gmajoulet committed May 15, 2020
1 parent a9d71b7 commit 5394b47
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions extensions/amp-story-education/0.1/amp-story-education.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export class AmpStoryEducation extends AMP.BaseElement {
/** @override */
buildCallback() {
this.containerEl_.classList.add('i-amphtml-story-education');
toggle(this.element, false);
toggle(this.containerEl_, false);
this.startListening_();
// Extra host to reset inherited styles and further enforce shadow DOM style
Expand Down Expand Up @@ -198,6 +199,7 @@ export class AmpStoryEducation extends AMP.BaseElement {
this.storeService_.dispatch(Action.TOGGLE_EDUCATION, false);
this.mutateElement(() => {
removeChildren(this.containerEl_);
toggle(this.element, false);
toggle(this.containerEl_, false);
this.storeService_.dispatch(
Action.TOGGLE_PAUSED,
Expand Down Expand Up @@ -270,6 +272,7 @@ export class AmpStoryEducation extends AMP.BaseElement {

this.mutateElement(() => {
removeChildren(this.containerEl_);
toggle(this.element, true);
toggle(this.containerEl_, true);
this.containerEl_.appendChild(template);
});
Expand Down

0 comments on commit 5394b47

Please sign in to comment.