Merge pull request #435 from hypothesis/move-isBrowserExtension-into-own-file

(config 1/n): Move isBrowserExtension() into own file

src/annotator/config/index.js

@@ -2,6 +2,7 @@
 
 var settings = require('./settings');
 var sharedSettings = require('../../shared/settings');
+var isBrowserExtension = require('./is-browser-extension');
 
 /**
  * Reads the Hypothesis configuration from the environment.
@@ -24,7 +25,7 @@ function configFrom(window_) {
     annotations: settings.annotations(window_.location.href),
   };
 
-  if (settings.isBrowserExtension(config)) {
+  if (isBrowserExtension(config.app)) {
     return config;
   }
 

src/annotator/config/is-browser-extension.js

+'use strict';
+
+/**
+ * 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(app) {
+  return !(app.startsWith('http://') || app.startsWith('https://'));
+}
+
+module.exports = isBrowserExtension;

src/annotator/config/settings.js

@@ -102,22 +102,9 @@ 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

@@ -5,9 +5,11 @@ var util = require('../../../shared/test/util');
 
 var fakeSharedSettings = {};
 var fakeSettings = {};
+var fakeIsBrowserExtension = sinon.stub();
 
 var configFrom = proxyquire('../index', util.noCallThru({
   './settings': fakeSettings,
+  './is-browser-extension': fakeIsBrowserExtension,
   '../../shared/settings': fakeSharedSettings,
 }));
 
@@ -28,7 +30,11 @@ describe('annotator.config.index', function() {
     fakeSettings.annotations = sinon.stub().returns(null);
     fakeSettings.query = sinon.stub().returns(null);
     fakeSettings.configFuncSettingsFrom = sinon.stub().returns({});
-    fakeSettings.isBrowserExtension = sinon.stub().returns(false);
+  });
+
+  beforeEach('reset fakeIsBrowserExtension()', function() {
+    fakeIsBrowserExtension.reset();
+    fakeIsBrowserExtension.returns(false);
   });
 
   it('gets the config.app setting', function() {
@@ -190,7 +196,7 @@ describe('annotator.config.index', function() {
 
   context('when the client is injected by the browser extension', function() {
     beforeEach('configure a browser extension client', function() {
-      fakeSettings.isBrowserExtension.returns(true);
+      fakeIsBrowserExtension.returns(true);
     });
 
     it('still reads the config.app setting from the host page', function() {

src/annotator/config/test/is-browser-extension-test.js

+'use strict';
+
+var isBrowserExtension = require('../is-browser-extension');
+
+describe('annotator.config.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(isBrowserExtension(test.url), test.returns);
+    });
+  });
+});

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

@@ -236,40 +236,4 @@ describe('annotator.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);
-      });
-    });
-  });
 });