Commit dc1801f4 authored by Sean Hammond's avatar Sean Hammond Committed by Robert Knight

Read annotations from the host page in settings.annotations()

Add code to the `settings.annotations()` method so that, instead of
reading the setting from the URL only, it reads the annotations setting
from the `js-hypothesis-config` scripts in the host page or, failing
that, tries to read it from the URL.

In `index.js`, after `settings.annotations()` is called, there are two
`Object.assign()` calls that overwrite values already in the `config`
objects with any returned from `js-hypothesis-config` scripts or the
`window.hypothesisConfig()` function in the host page. So a
`config.annotations` setting from `settings.annotations()` already gets
overwritten by a `annotations` setting in a `js-hypothesis-config`
script or `hypothesisConfig()` function.

This commit makes that overwriting explicit, encapsulates it in the
`annotations()` method, and unit tests it.

Note that the two `Object.assign()`s are still present in `index.js` so
the `config.annotations` setting returned by the `annotations()` method
still gets overwritten by them, but this now has no effect as the
`annotations()` method will already have set `config.annotations` to the
same value that it will be overwritten with.

In a future commit the `Object.assign()`s will be removed and there
won't be any overwriting.

Note that **this will change the behaviour of the code** once the
`Object.assign()`s have been removed: the `annotations()` method reads
the annotations setting from `js-hypothesis-config` scripts but does not
read it from `window.hypothesisConfig()` any longer. I don't believe
it's necessary to read `annotations` from `hypothesisConfig()`.
parent 30160267
...@@ -46,13 +46,19 @@ function settingsFrom(window_) { ...@@ -46,13 +46,19 @@ function settingsFrom(window_) {
* @return {string|null} - The extracted ID, or null. * @return {string|null} - The extracted ID, or null.
*/ */
function annotations() { function annotations() {
// Annotation IDs are url-safe-base64 identifiers
// See https://tools.ietf.org/html/rfc4648#page-7 /** Return the annotations from the URL, or null. */
var annotFragmentMatch = window_.location.href.match(/#annotations:([A-Za-z0-9_-]+)$/); function annotationsFromURL() {
if (annotFragmentMatch) { // Annotation IDs are url-safe-base64 identifiers
return annotFragmentMatch[1]; // See https://tools.ietf.org/html/rfc4648#page-7
var annotFragmentMatch = window_.location.href.match(/#annotations:([A-Za-z0-9_-]+)$/);
if (annotFragmentMatch) {
return annotFragmentMatch[1];
}
return null;
} }
return null;
return jsonConfigs.annotations || annotationsFromURL();
} }
/** /**
......
...@@ -120,6 +120,24 @@ describe('annotator.config.settingsFrom', function() { ...@@ -120,6 +120,24 @@ describe('annotator.config.settingsFrom', function() {
} }
describe('#annotations', function() { describe('#annotations', function() {
context('when the host page has a js-hypothesis-config with an annotations setting', function() {
beforeEach('add a js-hypothesis-config annotations setting', function() {
fakeSharedSettings.jsonConfigsFrom.returns({annotations: 'annotationsFromJSON'});
});
it('returns the annotations from the js-hypothesis-config script', function() {
assert.equal(settingsFrom(fakeWindow()).annotations, 'annotationsFromJSON');
});
context("when there's also an annotations in the URL fragment", function() {
specify('js-hypothesis-config annotations override URL ones', function() {
var window_ = fakeWindow('http://localhost:3000#annotations:annotationsFromURL');
assert.equal(settingsFrom(window_).annotations, 'annotationsFromJSON');
});
});
});
[ [
{ {
describe: "when there's a valid #annotations:<ID> fragment", describe: "when there's a valid #annotations:<ID> fragment",
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment