Merge pull request #425 from hypothesis/simplify-reading-query-and-annotations

Simplify reading query and annotations

src/annotator/config/index.js

@@ -11,30 +11,23 @@ var sharedSettings = require('../../shared/settings');
 function configFrom(window_) {
   var config = {
     app: settings.app(window_.document),
+
+    // Extract the default annotation ID or query from the URL.
+    //
+    // The Chrome extension or proxy may already have provided this config via
+    // a tag injected into the DOM, which avoids the problem where the page's
+    // JS rewrites the URL before Hypothesis loads.
+    //
+    // In environments where the config has not been injected into the DOM,
+    // we try to retrieve it from the URL here.
+    query: settings.query(window_.location.href),
+    annotations: settings.annotations(window_.location.href),
   };
 
   var chromeExt = 'chrome-extension://';
   var mozExt = 'moz-extension://';
   var edgeExt = 'ms-browser-extension://';
 
-  // Extract the default annotation ID or query from the URL.
-  //
-  // The Chrome extension or proxy may already have provided this config
-  // via a tag injected into the DOM, which avoids the problem where the page's
-  // JS rewrites the URL before Hypothesis loads.
-  //
-  // In environments where the config has not been injected into the DOM,
-  // we try to retrieve it from the URL here.
-  var query = settings.query(window_.location.href);
-  if (query) {
-    config.query = query;
-  } else {
-    var annotations = settings.annotations(window_.location.href);
-    if (annotations) {
-      config.annotations = annotations;
-    }
-  }
-
   // If the client is injected by the browser extension, ignore
   // the rest of the host page config.
   if (config.app.indexOf(chromeExt) === 0 ||

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

@@ -30,8 +30,8 @@ describe('annotator.config', function() {
 
   beforeEach('reset fakeSettings', function() {
     fakeSettings.app = sinon.stub().returns('IFRAME_URL');
-    fakeSettings.annotations = sinon.stub();
-    fakeSettings.query = sinon.stub();
+    fakeSettings.annotations = sinon.stub().returns(null);
+    fakeSettings.query = sinon.stub().returns(null);
     fakeSettings.configFuncSettingsFrom = sinon.stub().returns({});
   });
 
@@ -178,8 +178,7 @@ describe('annotator.config', function() {
     configFrom(fakeWindow());
 
     assert.calledOnce(fakeSettings.annotations);
-    assert.calledWithExactly(
-      fakeSettings.annotations, 'LOCATION_HREF');
+    assert.calledWithExactly(fakeSettings.annotations, 'LOCATION_HREF');
   });
 
   context("when there's a direct-linked annotation ID", function() {
@@ -193,24 +192,31 @@ describe('annotator.config', function() {
   });
 
   context("when there's no direct-linked annotation ID", function() {
-    it("doesn't add any .annotations setting to the config", function() {
-      assert.isFalse(configFrom(fakeWindow()).hasOwnProperty('annotations'));
+    it('sets config.annotations to null', function() {
+      assert.isNull(configFrom(fakeWindow()).annotations);
     });
+  });
 
-    context("when there's no annotations query", function() {
-      it("doesn't add any .query setting to the config", function() {
-        assert.isFalse(configFrom(fakeWindow()).hasOwnProperty('query'));
-      });
+  it("extracts the query from the parent page's URL", function() {
+    configFrom(fakeWindow());
+
+    assert.calledOnce(fakeSettings.query);
+    assert.calledWithExactly(fakeSettings.query, 'LOCATION_HREF');
+  });
+
+  context("when there's no annotations query", function() {
+    it('sets config.query to null', function() {
+      assert.isNull(configFrom(fakeWindow()).query);
     });
+  });
 
-    context("when there's an annotations query", function() {
-      beforeEach(function() {
-        fakeSettings.query.returns('QUERY');
-      });
+  context("when there's an annotations query", function() {
+    beforeEach(function() {
+      fakeSettings.query.returns('QUERY');
+    });
 
-      it('adds the query to the config', function() {
-        assert.equal(configFrom(fakeWindow()).query, 'QUERY');
-      });
+    it('adds the query to the config', function() {
+      assert.equal(configFrom(fakeWindow()).query, 'QUERY');
     });
   });