🐛 Defer the initialization of HighlightHandler until win.document bec…

…omes ready. (ampproject#20883)
noranazmy · Mar 22, 2019 · a09f268 · a09f268
1 parent 8dcf228
commit a09f268
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 1 deletion.
diff --git a/extensions/amp-viewer-integration/0.1/highlight-handler.js b/extensions/amp-viewer-integration/0.1/highlight-handler.js
@@ -22,6 +22,7 @@ import {moveLayoutRect} from '../../../src/layout-rect';
 import {parseJson} from '../../../src/json';
 import {parseQueryString} from '../../../src/url';
 import {resetStyles, setInitialDisplay, setStyles} from '../../../src/style';
+import {whenDocumentReady} from '../../../src/document-ready';
 
 /**
  * The message name sent by viewers to dismiss highlights.
@@ -143,7 +144,9 @@ export class HighlightHandler {
     /** @private {?Array<!Element>} */
     this.highlightedNodes_ = null;
 
-    this.initHighlight_(highlightInfo);
+    whenDocumentReady(ampdoc.win.document).then(() => {
+      this.initHighlight_(highlightInfo);
+    });
   }
 
   /**

diff --git a/extensions/amp-viewer-integration/0.1/test/test-highlight-handler.js b/extensions/amp-viewer-integration/0.1/test/test-highlight-handler.js
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import * as docready from '../../../../src/document-ready';
 import {HighlightHandler, getHighlightParam} from '../highlight-handler';
 import {Messaging, WindowPortEmulator} from '../messaging/messaging';
 import {Services} from '../../../../src/services';
@@ -100,6 +101,7 @@ describes.realWin('HighlightHandler', {
   },
 }, env => {
   let root = null;
+  let docreadyCb = null;
   beforeEach(() => {
     const {document} = env.win;
     root = document.createElement('div');
@@ -110,6 +112,12 @@ describes.realWin('HighlightHandler', {
     const div1 = document.createElement('div');
     div1.textContent = 'highlighted text';
     root.appendChild(div1);
+
+    sandbox.stub(docready, 'whenDocumentReady').returns({
+      then: cb => {
+        docreadyCb = cb;
+      },
+    });
   });
 
   it('initialize with visibility=visible', () => {
@@ -125,6 +133,12 @@ describes.realWin('HighlightHandler', {
     const handler = new HighlightHandler(
         ampdoc,{sentences: ['amp', 'highlight']});
 
+    // initHighlight_ is not called before document become ready.
+    expect(handler.highlightedNodes_).to.be.null;
+    docreadyCb();
+    // initHighlight_ was called in docreadyCb() and highlightedNodes_ is set.
+    expect(handler.highlightedNodes_).not.to.be.null;
+
     expect(setScrollTop).to.be.calledOnce;
     expect(setScrollTop.firstCall.args.length).to.equal(1);
 
@@ -173,6 +187,7 @@ describes.realWin('HighlightHandler', {
 
     new HighlightHandler(ampdoc,
         {sentences: ['amp', 'highlight'], skipRendering: true});
+    docreadyCb();
 
     expect(scrollStub).not.to.be.called;
 
@@ -202,6 +217,7 @@ describes.realWin('HighlightHandler', {
         Services.viewerForDoc(ampdoc), 'sendMessage');
 
     new HighlightHandler(ampdoc, {sentences: ['amp', 'highlight']});
+    docreadyCb();
 
     expect(scrollStub).not.to.be.called;
 
@@ -230,6 +246,7 @@ describes.realWin('HighlightHandler', {
         .returns(VisibilityState.PRERENDER);
 
     new HighlightHandler(ampdoc,{sentences: ['amp', 'highlight']});
+    docreadyCb();
 
     expect(setScrollTop).to.be.calledOnce;
     expect(setScrollTop.firstCall.args.length).to.equal(1);
@@ -244,6 +261,7 @@ describes.realWin('HighlightHandler', {
 
   it('calcTopToCenterHighlightedNodes_ center elements', () => {
     const handler = new HighlightHandler(env.ampdoc, {sentences: ['amp']});
+    docreadyCb();
     expect(handler.highlightedNodes_).not.to.be.null;
 
     const viewport = Services.viewportForDoc(env.ampdoc);
@@ -259,6 +277,7 @@ describes.realWin('HighlightHandler', {
 
   it('calcTopToCenterHighlightedNodes_ too tall element', () => {
     const handler = new HighlightHandler(env.ampdoc, {sentences: ['amp']});
+    docreadyCb();
     expect(handler.highlightedNodes_).not.to.be.null;
 
     const viewport = Services.viewportForDoc(env.ampdoc);
@@ -276,6 +295,7 @@ describes.realWin('HighlightHandler', {
 
   it('mayAdjustTop_', () => {
     const handler = new HighlightHandler(env.ampdoc, {sentences: ['amp']});
+    docreadyCb();
     expect(handler.highlightedNodes_).not.to.be.null;
 
     // Set up an environment where calcTopToCenterHighlightedNodes_