Merge pull request #3104 from hypothesis/291-fix-tab-state

Fix tab state being set to invalid value when opening a new tab
hypothesis · Mar 17, 2016 · 3ad27db · 3ad27db
2 parents 78fd99b + 1a8595d
commit 3ad27db
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 11 deletions.
diff --git a/h/browser/chrome/lib/hypothesis-chrome-extension.js b/h/browser/chrome/lib/hypothesis-chrome-extension.js
@@ -183,7 +183,7 @@ function HypothesisChromeExtension(dependencies) {
     if (changeInfo.status === TAB_STATUS_LOADING) {
       resetTabState(tabId, tab.url);
     } else if (changeInfo.status === TAB_STATUS_COMPLETE) {
-      var newActiveState = state.getState(tabId);
+      var newActiveState = state.getState(tabId).state;
       if (urlHasAnnotationFragment(tab.url)) {
         newActiveState = TabState.states.ACTIVE;
       }

diff --git a/h/browser/chrome/lib/polyfills.js b/h/browser/chrome/lib/polyfills.js
@@ -1,6 +1,12 @@
+'use strict';
+
 // Polyfills for APIs which are not present on all supported
 // versions of Chrome
 
-// polyfill for the fetch() API for Chrome < 42,
+// ES2015+ polyfills
+require('core-js/fn/object/assign'); // Available: Chrome >= 45
+require('core-js/fn/object/values'); // Available: Chrome issue #4663
+
+// Polyfill for the fetch() API for Chrome < 42,
 // also used by our PhantomJS tests
 require('whatwg-fetch');
diff --git a/h/browser/chrome/lib/tab-state.js b/h/browser/chrome/lib/tab-state.js
@@ -1,6 +1,5 @@
 'use strict';
 
-var assign = require('core-js/modules/$.object-assign');
 var isShallowEqual = require('is-equal-shallow');
 
 var uriInfo = require('./uri-info');
@@ -63,7 +62,7 @@ function TabState(initialState, onchange) {
   this.load = function (newState) {
     var newCurrentState = {};
     Object.keys(newState).forEach(function (tabId) {
-      newCurrentState[tabId] = assign({}, DEFAULT_STATE, newState[tabId]);
+      newCurrentState[tabId] = Object.assign({}, DEFAULT_STATE, newState[tabId]);
     });
     currentState = newCurrentState;
   };
@@ -96,7 +95,7 @@ function TabState(initialState, onchange) {
 
   this.annotationCount = function(tabId) {
     return this.getState(tabId).annotationCount;
-  }
+  };
 
   this.isTabActive = function (tabId) {
     return this.getState(tabId).state === states.ACTIVE;
@@ -121,7 +120,7 @@ function TabState(initialState, onchange) {
   this.setState = function (tabId, stateUpdate) {
     var newState;
     if (stateUpdate) {
-      newState = assign({}, this.getState(tabId), stateUpdate);
+      newState = Object.assign({}, this.getState(tabId), stateUpdate);
       if (newState.state !== states.ERRORED) {
         newState.error = undefined;
       }
@@ -136,7 +135,7 @@ function TabState(initialState, onchange) {
     if (_this.onchange) {
       _this.onchange(tabId, newState);
     }
-  }
+  };
 
   /**
    * Query the server for the annotation count for a URL
@@ -152,7 +151,7 @@ function TabState(initialState, onchange) {
       self.setState(tabId, { annotationCount: result.total });
     }).catch(function (err) {
       self.setState(tabId, { annotationCount: 0 });
-      console.error('Failed to fetch annotation count for %s: %s', tabUrl, err)
+      console.error('Failed to fetch annotation count for %s: %s', tabUrl, err);
     });
   };
 

diff --git a/h/browser/chrome/test/hypothesis-chrome-extension-test.js b/h/browser/chrome/test/hypothesis-chrome-extension-test.js
@@ -1,6 +1,5 @@
 'use strict';
 
-var assign = require('core-js/modules/$.object-assign');
 var proxyquire = require('proxyquire');
 
 var toResult = require('../../../static/scripts/test/promise-util').toResult;
@@ -25,6 +24,14 @@ function FakeListener() {
   };
 }
 
+/**
+ * Return true if a tab state is valid
+ * @param {TabState} state
+ */
+function isValidState(state) {
+  return Object.values(TabState.states).indexOf(state.state) !== -1;
+}
+
 describe('HypothesisChromeExtension', function () {
   var sandbox = sinon.sandbox.create();
   var HypothesisChromeExtension;
@@ -196,7 +203,7 @@ describe('HypothesisChromeExtension', function () {
       var tabState = {};
       function createTab(initialState) {
         var tabId = 1;
-        tabState[tabId] = assign({
+        tabState[tabId] = Object.assign({
           state: TabState.states.INACTIVE,
           annotationCount: 0,
           ready: false,
@@ -216,7 +223,8 @@ describe('HypothesisChromeExtension', function () {
           return tabState[tabId];
         };
         fakeTabState.setState = function (tabId, state) {
-          tabState[tabId] = assign(tabState[tabId], state);
+          tabState[tabId] = Object.assign(tabState[tabId], state);
+          assert(isValidState(tabState[tabId]));
         };
         ext.listen({addEventListener: sandbox.stub()});
       });