Commit 3a9d395a authored by Kyle Keating's avatar Kyle Keating Committed by Kyle Keating

Remove isBrowserExtension logic from settings.js

nb. This is now handled inside of getConfig()
parent b208c1e9
...@@ -2,7 +2,6 @@ import { parseJsonConfig } from '../../boot/parse-json-config'; ...@@ -2,7 +2,6 @@ import { parseJsonConfig } from '../../boot/parse-json-config';
import { toBoolean } from '../../shared/type-coercions'; import { toBoolean } from '../../shared/type-coercions';
import configFuncSettingsFrom from './config-func-settings-from'; import configFuncSettingsFrom from './config-func-settings-from';
import { isBrowserExtension } from './is-browser-extension';
import { urlFromLinkTag } from './url-from-link-tag'; import { urlFromLinkTag } from './url-from-link-tag';
/** /**
...@@ -130,17 +129,8 @@ export default function settingsFrom(window_) { ...@@ -130,17 +129,8 @@ export default function settingsFrom(window_) {
} }
function hostPageSetting(name, options = {}) { function hostPageSetting(name, options = {}) {
const allowInBrowserExt = options.allowInBrowserExt || false;
const hasDefaultValue = typeof options.defaultValue !== 'undefined'; const hasDefaultValue = typeof options.defaultValue !== 'undefined';
// TODO: Remove this logic
if (
!allowInBrowserExt &&
isBrowserExtension(urlFromLinkTag(window_, 'sidebar', 'html'))
) {
return hasDefaultValue ? options.defaultValue : null;
}
if (configFuncSettings.hasOwnProperty(name)) { if (configFuncSettings.hasOwnProperty(name)) {
return configFuncSettings[name]; return configFuncSettings[name];
} }
......
...@@ -3,21 +3,16 @@ import { $imports } from '../settings'; ...@@ -3,21 +3,16 @@ import { $imports } from '../settings';
describe('annotator/config/settingsFrom', () => { describe('annotator/config/settingsFrom', () => {
let fakeConfigFuncSettingsFrom; let fakeConfigFuncSettingsFrom;
let fakeIsBrowserExtension;
let fakeParseJsonConfig; let fakeParseJsonConfig;
let fakeUrlFromLinkTag; let fakeUrlFromLinkTag;
beforeEach(() => { beforeEach(() => {
fakeConfigFuncSettingsFrom = sinon.stub().returns({}); fakeConfigFuncSettingsFrom = sinon.stub().returns({});
fakeIsBrowserExtension = sinon.stub().returns(false);
fakeUrlFromLinkTag = sinon.stub().returns('http://example.com/app.html'); fakeUrlFromLinkTag = sinon.stub().returns('http://example.com/app.html');
fakeParseJsonConfig = sinon.stub().returns({}); fakeParseJsonConfig = sinon.stub().returns({});
$imports.$mock({ $imports.$mock({
'./config-func-settings-from': fakeConfigFuncSettingsFrom, './config-func-settings-from': fakeConfigFuncSettingsFrom,
'./is-browser-extension': {
isBrowserExtension: fakeIsBrowserExtension,
},
'./url-from-link-tag': { './url-from-link-tag': {
urlFromLinkTag: fakeUrlFromLinkTag, urlFromLinkTag: fakeUrlFromLinkTag,
}, },
...@@ -354,23 +349,6 @@ describe('annotator/config/settingsFrom', () => { ...@@ -354,23 +349,6 @@ describe('annotator/config/settingsFrom', () => {
it("defaults to 'always' if there's no showHighlights setting in the host page", () => { it("defaults to 'always' if there's no showHighlights setting in the host page", () => {
assert.equal(settingsFrom(fakeWindow()).showHighlights, 'always'); assert.equal(settingsFrom(fakeWindow()).showHighlights, 'always');
}); });
context('when the client is in a browser extension', () => {
beforeEach('configure a browser extension client', () => {
fakeIsBrowserExtension.returns(true);
});
it("doesn't read the setting from the host page, defaults to 'always'", () => {
fakeParseJsonConfig.returns({
showHighlights: 'never',
});
fakeConfigFuncSettingsFrom.returns({
showHighlights: 'never',
});
assert.equal(settingsFrom(fakeWindow()).showHighlights, 'always');
});
});
}); });
describe('#hostPageSetting', () => { describe('#hostPageSetting', () => {
...@@ -378,7 +356,6 @@ describe('annotator/config/settingsFrom', () => { ...@@ -378,7 +356,6 @@ describe('annotator/config/settingsFrom', () => {
{ {
when: 'the client is embedded in a web page', when: 'the client is embedded in a web page',
specify: 'it returns setting values from window.hypothesisConfig()', specify: 'it returns setting values from window.hypothesisConfig()',
isBrowserExtension: false,
configFuncSettings: { foo: 'configFuncValue' }, configFuncSettings: { foo: 'configFuncValue' },
jsonSettings: { foo: 'ignored' }, // hypothesisConfig() overrides js-hypothesis-config jsonSettings: { foo: 'ignored' }, // hypothesisConfig() overrides js-hypothesis-config
expected: 'configFuncValue', expected: 'configFuncValue',
...@@ -395,7 +372,6 @@ describe('annotator/config/settingsFrom', () => { ...@@ -395,7 +372,6 @@ describe('annotator/config/settingsFrom', () => {
{ {
when: 'the client is embedded in a web page', when: 'the client is embedded in a web page',
specify: 'it returns setting values from js-hypothesis-config objects', specify: 'it returns setting values from js-hypothesis-config objects',
isBrowserExtension: false,
configFuncSettings: {}, configFuncSettings: {},
jsonSettings: { foo: 'jsonValue' }, jsonSettings: { foo: 'jsonValue' },
expected: 'jsonValue', expected: 'jsonValue',
...@@ -404,7 +380,6 @@ describe('annotator/config/settingsFrom', () => { ...@@ -404,7 +380,6 @@ describe('annotator/config/settingsFrom', () => {
when: 'the client is embedded in a web page', when: 'the client is embedded in a web page',
specify: specify:
'hypothesisConfig() settings override js-hypothesis-config ones', 'hypothesisConfig() settings override js-hypothesis-config ones',
isBrowserExtension: false,
configFuncSettings: { foo: 'configFuncValue' }, configFuncSettings: { foo: 'configFuncValue' },
jsonSettings: { foo: 'jsonValue' }, jsonSettings: { foo: 'jsonValue' },
expected: 'configFuncValue', expected: 'configFuncValue',
...@@ -413,7 +388,6 @@ describe('annotator/config/settingsFrom', () => { ...@@ -413,7 +388,6 @@ describe('annotator/config/settingsFrom', () => {
when: 'the client is embedded in a web page', when: 'the client is embedded in a web page',
specify: specify:
'even a null from hypothesisConfig() overrides js-hypothesis-config', 'even a null from hypothesisConfig() overrides js-hypothesis-config',
isBrowserExtension: false,
configFuncSettings: { foo: null }, configFuncSettings: { foo: null },
jsonSettings: { foo: 'jsonValue' }, jsonSettings: { foo: 'jsonValue' },
expected: null, expected: null,
...@@ -422,42 +396,13 @@ describe('annotator/config/settingsFrom', () => { ...@@ -422,42 +396,13 @@ describe('annotator/config/settingsFrom', () => {
when: 'the client is embedded in a web page', when: 'the client is embedded in a web page',
specify: specify:
'even an undefined from hypothesisConfig() overrides js-hypothesis-config', 'even an undefined from hypothesisConfig() overrides js-hypothesis-config',
isBrowserExtension: false,
configFuncSettings: { foo: undefined }, configFuncSettings: { foo: undefined },
jsonSettings: { foo: 'jsonValue' }, jsonSettings: { foo: 'jsonValue' },
expected: undefined, expected: undefined,
}, },
{
when: 'the client is in a browser extension',
specify: 'it always returns null',
isBrowserExtension: true,
configFuncSettings: { foo: 'configFuncValue' },
jsonSettings: { foo: 'jsonValue' },
expected: null,
},
{
when: 'the client is in a browser extension and allowInBrowserExt: true is given',
specify: 'it returns settings from window.hypothesisConfig()',
isBrowserExtension: true,
allowInBrowserExt: true,
configFuncSettings: { foo: 'configFuncValue' },
jsonSettings: {},
expected: 'configFuncValue',
},
{
when: 'the client is in a browser extension and allowInBrowserExt: true is given',
specify: 'it returns settings from js-hypothesis-configs',
isBrowserExtension: true,
allowInBrowserExt: true,
configFuncSettings: {},
jsonSettings: { foo: 'jsonValue' },
expected: 'jsonValue',
},
{ {
when: 'no default value is provided', when: 'no default value is provided',
specify: 'it returns null', specify: 'it returns null',
isBrowserExtension: false,
allowInBrowserExt: false,
configFuncSettings: {}, configFuncSettings: {},
jsonSettings: {}, jsonSettings: {},
defaultValue: undefined, defaultValue: undefined,
...@@ -466,8 +411,6 @@ describe('annotator/config/settingsFrom', () => { ...@@ -466,8 +411,6 @@ describe('annotator/config/settingsFrom', () => {
{ {
when: 'a default value is provided', when: 'a default value is provided',
specify: 'it returns that default value', specify: 'it returns that default value',
isBrowserExtension: false,
allowInBrowserExt: false,
configFuncSettings: {}, configFuncSettings: {},
jsonSettings: {}, jsonSettings: {},
defaultValue: 'test value', defaultValue: 'test value',
...@@ -476,33 +419,19 @@ describe('annotator/config/settingsFrom', () => { ...@@ -476,33 +419,19 @@ describe('annotator/config/settingsFrom', () => {
{ {
when: 'a default value is provided but it is overridden', when: 'a default value is provided but it is overridden',
specify: 'it returns the overridden value', specify: 'it returns the overridden value',
isBrowserExtension: false,
allowInBrowserExt: false,
configFuncSettings: { foo: 'not the default value' }, configFuncSettings: { foo: 'not the default value' },
jsonSettings: {}, jsonSettings: {},
defaultValue: 'the default value', defaultValue: 'the default value',
expected: 'not the default value', expected: 'not the default value',
}, },
{
when: 'the client is in a browser extension and a default value is provided',
specify: 'it returns the default value',
isBrowserExtension: true,
allowInBrowserExt: false,
configFuncSettings: { foo: 'ignore me' },
jsonSettings: { foo: 'also ignore me' },
defaultValue: 'the default value',
expected: 'the default value',
},
].forEach(function (test) { ].forEach(function (test) {
context(test.when, () => { context(test.when, () => {
specify(test.specify, () => { specify(test.specify, () => {
fakeIsBrowserExtension.returns(test.isBrowserExtension);
fakeConfigFuncSettingsFrom.returns(test.configFuncSettings); fakeConfigFuncSettingsFrom.returns(test.configFuncSettings);
fakeParseJsonConfig.returns(test.jsonSettings); fakeParseJsonConfig.returns(test.jsonSettings);
const settings = settingsFrom(fakeWindow()); const settings = settingsFrom(fakeWindow());
const setting = settings.hostPageSetting('foo', { const setting = settings.hostPageSetting('foo', {
allowInBrowserExt: test.allowInBrowserExt || false,
defaultValue: test.defaultValue || null, defaultValue: test.defaultValue || null,
}); });
......
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