diff --git a/CHANGELOG.md b/CHANGELOG.md index d5441ac48..d28dd816d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +# Next release + +### Compatibility changes + +* The `favicon` setting is now interpreted as a pathname to a favicon file, not + a URL. Please see the documentation comment in `settings.json.template`. +* The undocumented `faviconPad` and `faviconTimeslider` settings have been + removed. + # 1.8.13 ### Notable fixes diff --git a/settings.json.docker b/settings.json.docker index 228c70d2a..030ab88f1 100644 --- a/settings.json.docker +++ b/settings.json.docker @@ -80,10 +80,12 @@ "title": "${TITLE:Etherpad}", /* - * favicon default name - * alternatively, set up a fully specified Url to your own favicon + * Pathname of the favicon you want to use. If null, the skin's favicon is + * used if one is provided by the skin, otherwise the default Etherpad favicon + * is used. If this is a relative path it is interpreted as relative to the + * Etherpad root directory. */ - "favicon": "${FAVICON:favicon.ico}", + "favicon": "${FAVICON}", /* * Skin name. diff --git a/settings.json.template b/settings.json.template index 8a676b56d..90a89970c 100644 --- a/settings.json.template +++ b/settings.json.template @@ -71,10 +71,12 @@ "title": "Etherpad", /* - * favicon default name - * alternatively, set up a fully specified Url to your own favicon + * Pathname of the favicon you want to use. If null, the skin's favicon is + * used if one is provided by the skin, otherwise the default Etherpad favicon + * is used. If this is a relative path it is interpreted as relative to the + * Etherpad root directory. */ - "favicon": "favicon.ico", + "favicon": null, /* * Skin name. diff --git a/src/node/hooks/express/specialpages.js b/src/node/hooks/express/specialpages.js index 725139c09..66ee0221e 100644 --- a/src/node/hooks/express/specialpages.js +++ b/src/node/hooks/express/specialpages.js @@ -80,6 +80,7 @@ exports.expressCreateServer = (hookName, args, cb) => { args.app.get('/favicon.ico', (req, res, next) => { (async () => { const fns = [ + ...(settings.favicon ? [path.resolve(settings.root, settings.favicon)] : []), path.join(settings.root, 'src', 'static', 'skins', settings.skinName, 'favicon.ico'), path.join(settings.root, 'src', 'static', 'favicon.ico'), ]; diff --git a/src/node/utils/Settings.js b/src/node/utils/Settings.js index 60f4e6442..b31cf16b4 100644 --- a/src/node/utils/Settings.js +++ b/src/node/utils/Settings.js @@ -50,11 +50,12 @@ console.log('All relative paths will be interpreted relative to the identified ' exports.title = 'Etherpad'; /** - * The app favicon fully specified url, visible e.g. in the browser window + * Pathname of the favicon you want to use. If null, the skin's favicon is + * used if one is provided by the skin, otherwise the default Etherpad favicon + * is used. If this is a relative path it is interpreted as relative to the + * Etherpad root directory. */ -exports.favicon = 'favicon.ico'; -exports.faviconPad = `../${exports.favicon}`; -exports.faviconTimeslider = `../../${exports.favicon}`; +exports.favicon = null; /* * Skin name. diff --git a/src/templates/index.html b/src/templates/index.html index 8f20c08da..4d08312f9 100644 --- a/src/templates/index.html +++ b/src/templates/index.html @@ -8,7 +8,7 @@ - + diff --git a/src/templates/pad.html b/src/templates/pad.html index 7bf2346f9..d7c3c0823 100644 --- a/src/templates/pad.html +++ b/src/templates/pad.html @@ -39,7 +39,7 @@ - + <% e.begin_block("styles"); %> diff --git a/src/templates/timeslider.html b/src/templates/timeslider.html index b3fd3d006..fe43668c8 100644 --- a/src/templates/timeslider.html +++ b/src/templates/timeslider.html @@ -33,7 +33,7 @@ - + <% e.begin_block("timesliderStyles"); %> diff --git a/src/tests/backend/specs/favicon-test-custom.png b/src/tests/backend/specs/favicon-test-custom.png new file mode 100644 index 000000000..9c6532c96 Binary files /dev/null and b/src/tests/backend/specs/favicon-test-custom.png differ diff --git a/src/tests/backend/specs/favicon.js b/src/tests/backend/specs/favicon.js index 76140b72f..a5e3095de 100644 --- a/src/tests/backend/specs/favicon.js +++ b/src/tests/backend/specs/favicon.js @@ -12,11 +12,13 @@ describe(__filename, function () { let agent; let backupSettings; let skinDir; + let wantCustomIcon; let wantDefaultIcon; let wantSkinIcon; before(async function () { agent = await common.init(); + wantCustomIcon = await fsp.readFile(path.join(__dirname, 'favicon-test-custom.png')); wantDefaultIcon = await fsp.readFile(path.join(settings.root, 'src', 'static', 'favicon.ico')); wantSkinIcon = await fsp.readFile(path.join(__dirname, 'favicon-test-skin.png')); }); @@ -28,6 +30,7 @@ describe(__filename, function () { }); afterEach(async function () { + delete settings.favicon; delete settings.skinName; Object.assign(settings, backupSettings); try { @@ -38,8 +41,40 @@ describe(__filename, function () { } catch (err) { /* intentionally ignored */ } }); + it('uses custom favicon if set (relative pathname)', async function () { + settings.favicon = + path.relative(settings.root, path.join(__dirname, 'favicon-test-custom.png')); + assert(!path.isAbsolute(settings.favicon)); + const {body: gotIcon} = await agent.get('/favicon.ico') + .accept('png').buffer(true).parse(superagent.parse.image) + .expect(200); + assert(gotIcon.equals(wantCustomIcon)); + }); + + it('uses custom favicon if set (absolute pathname)', async function () { + settings.favicon = path.join(__dirname, 'favicon-test-custom.png'); + assert(path.isAbsolute(settings.favicon)); + const {body: gotIcon} = await agent.get('/favicon.ico') + .accept('png').buffer(true).parse(superagent.parse.image) + .expect(200); + assert(gotIcon.equals(wantCustomIcon)); + }); + + it('falls back if custom favicon is missing', async function () { + // The previous default for settings.favicon was 'favicon.ico', so many users will continue to + // have that in their settings.json for a long time. There is unlikely to be a favicon at + // path.resolve(settings.root, 'favicon.ico'), so this test ensures that 'favicon.ico' won't be + // a problem for those users. + settings.favicon = 'favicon.ico'; + const {body: gotIcon} = await agent.get('/favicon.ico') + .accept('png').buffer(true).parse(superagent.parse.image) + .expect(200); + assert(gotIcon.equals(wantDefaultIcon)); + }); + it('uses skin favicon if present', async function () { await fsp.writeFile(path.join(skinDir, 'favicon.ico'), wantSkinIcon); + settings.favicon = null; const {body: gotIcon} = await agent.get('/favicon.ico') .accept('png').buffer(true).parse(superagent.parse.image) .expect(200); @@ -47,6 +82,7 @@ describe(__filename, function () { }); it('falls back to default favicon', async function () { + settings.favicon = null; const {body: gotIcon} = await agent.get('/favicon.ico') .accept('png').buffer(true).parse(superagent.parse.image) .expect(200);