Merge pull request #432 from hypothesis/extract-isBrowserExtension

Extract isBrowserExtension() into a separate settings function

src/annotator/config/index.js

@@ -24,15 +24,7 @@ function configFrom(window_) {
     annotations: settings.annotations(window_.location.href),
   };
 
-  var chromeExt = 'chrome-extension://';
-  var mozExt = 'moz-extension://';
-  var edgeExt = 'ms-browser-extension://';
-
-  // If the client is injected by the browser extension, ignore
-  // the rest of the host page config.
-  if (config.app.indexOf(chromeExt) === 0 ||
-    config.app.indexOf(mozExt) === 0||
-    config.app.indexOf(edgeExt) === 0) {
+  if (settings.isBrowserExtension(config)) {
     return config;
   }
 

src/annotator/config/settings.js

@@ -102,9 +102,22 @@ function configFuncSettingsFrom(window_) {
   return window_.hypothesisConfig();
 }
 
+/**
+ * Return true if the client is from a browser extension.
+ *
+ * @returns {boolean} true if this instance of the Hypothesis client is one
+ *   distributed in a browser extension, false if it's one embedded in a
+ *   website.
+ *
+ */
+function isBrowserExtension(config) {
+  return !(config.app.startsWith('http://') || config.app.startsWith('https://'));
+}
+
 module.exports = {
   app: app,
   annotations: annotations,
   query: query,
   configFuncSettingsFrom: configFuncSettingsFrom,
+  isBrowserExtension: isBrowserExtension,
 };

src/annotator/config/test/index-test.js

@@ -18,7 +18,7 @@ function fakeWindow() {
   };
 }
 
-describe('annotator.config', function() {
+describe('annotator.config.index', function() {
   beforeEach('reset fakeSharedSettings', function() {
     fakeSharedSettings.jsonConfigsFrom = sinon.stub().returns({});
   });
@@ -28,6 +28,7 @@ describe('annotator.config', function() {
     fakeSettings.annotations = sinon.stub().returns(null);
     fakeSettings.query = sinon.stub().returns(null);
     fakeSettings.configFuncSettingsFrom = sinon.stub().returns({});
+    fakeSettings.isBrowserExtension = sinon.stub().returns(false);
   });
 
   it('gets the config.app setting', function() {
@@ -188,39 +189,38 @@ describe('annotator.config', function() {
   });
 
   context('when the client is injected by the browser extension', function() {
-    beforeEach(function() {
-      fakeSettings.annotations.returns('SOME_ANNOTATION_ID');
-      fakeSharedSettings.jsonConfigsFrom.returns({foo: 'bar'});
+    beforeEach('configure a browser extension client', function() {
+      fakeSettings.isBrowserExtension.returns(true);
     });
 
-    it('ignores the host page config on chrome', function() {
-      fakeSettings.app.returns('chrome-extension://abcdef');
-
-      var config = configFrom(fakeWindow());
+    it('still reads the config.app setting from the host page', function() {
+      fakeSettings.app.returns('SOME_APP_URL');
 
-      assert.equal(config.app, 'chrome-extension://abcdef');
-      assert.equal(config.annotations, 'SOME_ANNOTATION_ID');
-      assert.isUndefined(config.foo);
+      assert.equal(configFrom(fakeWindow()).app, fakeSettings.app());
     });
 
-    it('ignores the host page config on firefox', function() {
-      fakeSettings.app.returns('moz-extension://abcdef');
+    it('still reads the config.query setting from the host page', function() {
+      fakeSettings.query.returns('SOME_QUERY');
 
-      var config = configFrom(fakeWindow());
+      assert.equal(configFrom(fakeWindow()).query, fakeSettings.query());
+    });
 
-      assert.equal(config.app, 'moz-extension://abcdef');
-      assert.equal(config.annotations, 'SOME_ANNOTATION_ID');
-      assert.isUndefined(config.foo);
+    it('still reads the config.annotations setting from the host page', function() {
+      fakeSettings.annotations.returns('SOME_ANNOTATION_ID');
+
+      assert.equal(configFrom(fakeWindow()).annotations, fakeSettings.annotations());
     });
 
-    it('ignores the host page config on edge', function() {
-      fakeSettings.app.returns('ms-browser-extension://abcdef');
+    it('ignores settings from JSON objects in the host page', function() {
+      fakeSharedSettings.jsonConfigsFrom.returns({foo: 'bar'});
 
-      var config = configFrom(fakeWindow());
+      assert.isUndefined(configFrom(fakeWindow()).foo);
+    });
 
-      assert.equal(config.app, 'ms-browser-extension://abcdef');
-      assert.equal(config.annotations, 'SOME_ANNOTATION_ID');
-      assert.isUndefined(config.foo);
+    it('ignores settings from the hypothesisConfig() function in the host page', function() {
+      fakeSettings.configFuncSettingsFrom.returns({foo: 'bar'});
+
+      assert.isUndefined(configFrom(fakeWindow()).foo);
     });
   });
 

src/annotator/config/test/settings-test.js

@@ -4,7 +4,7 @@ var settings = require('../settings');
 
 var sandbox = sinon.sandbox.create();
 
-describe('annotation.config.settings', function() {
+describe('annotator.config.settings', function() {
 
   afterEach('reset the sandbox', function() {
     sandbox.restore();
@@ -236,4 +236,40 @@ describe('annotation.config.settings', function() {
       });
     });
   });
+
+  describe('#isBrowserExtension', function() {
+    [
+      {
+        url: 'chrome-extension://abcxyz',
+        returns: true,
+      },
+      {
+        url: 'moz-extension://abcxyz',
+        returns: true,
+      },
+      {
+        url: 'ms-browser-extension://abcxyz',
+        returns: true,
+      },
+      {
+        url: 'http://partner.org',
+        returns: false,
+      },
+      {
+        url: 'https://partner.org',
+        returns: false,
+      },
+      // It considers anything not http(s) to be a browser extension.
+      {
+        url: 'ftp://partner.org',
+        returns: true,
+      },
+    ].forEach(function(test) {
+      it('returns ' + test.returns + ' for ' + test.url, function() {
+        assert.equal(
+          settings.isBrowserExtension({app: test.url}),
+          test.returns);
+      });
+    });
+  });
 });