Merge pull request #1571 from dvoytenko/fixes27

Fixes for history state due to nesting of services
ampproject · Jan 26, 2016 · 5a29bc4 · 5a29bc4
2 parents d3c7ed1 + 96b9019
commit 5a29bc4
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 15 deletions.
diff --git a/src/service/history-impl.js b/src/service/history-impl.js
@@ -273,9 +273,11 @@ export class HistoryBindingNatural_ {
     let pushState, replaceState;
     if (history.pushState && history.replaceState) {
       /** @private @const {function(*, string=, string=)|undefined} */
-      this.origPushState_ = history.pushState.bind(history);
+      this.origPushState_ = history.originalPushState ||
+          history.pushState.bind(history);
       /** @private @const {function(*, string=, string=)|undefined} */
-      this.origReplaceState_ = history.replaceState.bind(history);
+      this.origReplaceState_ = history.originalReplaceState ||
+          history.replaceState.bind(history);
       pushState = (state, opt_title, opt_url) => {
         this.unsupportedState_ = state;
         this.origPushState_(state, opt_title, opt_url);
@@ -290,6 +292,12 @@ export class HistoryBindingNatural_ {
           this.origReplaceState_(state, opt_title);
         }
       };
+      if (!history.originalPushState) {
+        history.originalPushState = this.origPushState_;
+      }
+      if (!history.originalReplaceState) {
+        history.originalReplaceState = this.origReplaceState_;
+      }
     } else {
       pushState = (state, opt_title, opt_url) => {
         this.unsupportedState_ = state;

diff --git a/test/functional/test-history.js b/test/functional/test-history.js
@@ -143,9 +143,7 @@ describe('HistoryBindingNatural', () => {
     expect(onStackIndexUpdated.callCount).to.equal(0);
   });
 
-  // TODO(@dvoytenko): Unskip. Broke after change in service initialization
-  // sequence.
-  it.skip('should initialize correctly with preexisting state', () => {
+  it('should initialize correctly with preexisting state', () => {
     history.origPushState_({'AMP.History': window.history.length}, undefined);
     history.origReplaceState_({'AMP.History': window.history.length - 2},
         undefined);
@@ -181,17 +179,19 @@ describe('HistoryBindingNatural', () => {
   // This prevents IE11/Edge from coercing undefined to become the new url
   it('should not pass in `url` argument to original replace state if ' +
     'parameter is undefined', () => {
-    let argumentLength = 0;
-    const origReplace = window.history.replaceState;
-    window.history.replaceState = function() {
-      argumentLength = arguments.length;
+    const replaceStateSpy = sinon.spy();
+    const windowStub = {
+      history: {
+        replaceState: replaceStateSpy,
+        pushState: () => {},
+        state: {},
+        length: 11,
+      },
+      addEventListener: () => {},
     };
-
-    new HistoryBindingNatural_(window);
-
-    expect(argumentLength).to.equal(2);
-
-    window.history.replaceState = origReplace;
+    new HistoryBindingNatural_(windowStub);
+    expect(replaceStateSpy.callCount).to.be.greaterThan(0);
+    expect(replaceStateSpy.lastCall.args.length).to.equal(2);
   });
 
   it('should push new state in the window.history and notify', () => {