From 4124da2439b9d3f5b867e63962229ff508957d76 Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Tue, 20 Nov 2018 00:01:23 -0800 Subject: [PATCH] fix(emojos): fix emojos on Ubuntu and Chrome on Windows (#661) * fix(emojos): fix emojos on Ubuntu and Chrome on Windows * fixup * start working on unit tests * fixup * add more tests and fix emoji --- .travis.yml | 1 + CONTRIBUTING.md | 7 +- bin/setup-mastodon-in-travis.sh | 2 +- package-lock.json | 79 +++++++++++++++++- package.json | 3 + routes/_utils/emojiRegex.js | 8 ++ routes/_utils/emojifyText.js | 6 ++ routes/_utils/removeEmoji.js | 9 +-- routes/_utils/replaceEmoji.js | 35 ++++++++ scss/global.scss | 9 ++- templates/2xx.html | 2 +- tests/unit/test-emoji.js | 137 ++++++++++++++++++++++++++++++++ 12 files changed, 284 insertions(+), 14 deletions(-) create mode 100644 routes/_utils/emojiRegex.js create mode 100644 routes/_utils/replaceEmoji.js create mode 100644 tests/unit/test-emoji.js diff --git a/.travis.yml b/.travis.yml index 170381a..14d2537 100644 --- a/.travis.yml +++ b/.travis.yml @@ -56,6 +56,7 @@ matrix: include: - env: BROWSER=chrome:headless COMMAND=test-browser-suite0 - env: BROWSER=chrome:headless COMMAND=test-browser-suite1 + - env: COMMAND=test-unit - env: COMMAND=deploy-all-travis allow_failures: - env: COMMAND=deploy-all-travis diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ad01706..fe31756 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -101,4 +101,9 @@ This is also available locally after `npm run build` at `.sapper/client/report.h 6. Commit all changed files 7. Run `rm -fr mastodon/` and `npm run run-mastodon` to confirm everything's working -Check `mastodon.log` if you have any issues. \ No newline at end of file +Check `mastodon.log` if you have any issues. + +## Unit tests + +There are also some unit tests that run in Node using Mocha. You can find them in `tests/unit` and +run them using `npm run test-unit`. \ No newline at end of file diff --git a/bin/setup-mastodon-in-travis.sh b/bin/setup-mastodon-in-travis.sh index c67a5d5..56a7881 100755 --- a/bin/setup-mastodon-in-travis.sh +++ b/bin/setup-mastodon-in-travis.sh @@ -2,7 +2,7 @@ set -e -if [[ "$COMMAND" = deploy-all-travis ]]; then +if [[ "$COMMAND" = deploy-all-travis || "$COMMAND" = test-unit ]]; then exit 0 # no need to setup mastodon in this case fi diff --git a/package-lock.json b/package-lock.json index f8f942e..85b9015 100644 --- a/package-lock.json +++ b/package-lock.json @@ -508,7 +508,7 @@ }, "util": { "version": "0.10.3", - "resolved": "https://registry.npmjs.org/util/-/util-0.10.3.tgz", + "resolved": "http://registry.npmjs.org/util/-/util-0.10.3.tgz", "integrity": "sha1-evsa/lCAUkZInj23/g7TeTNqwPk=", "requires": { "inherits": "2.0.1" @@ -1621,6 +1621,12 @@ "base64-js": "^1.1.2" } }, + "browser-stdout": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/browser-stdout/-/browser-stdout-1.3.1.tgz", + "integrity": "sha512-qhAVI1+Av2X7qelOfAIYwXONood6XlZE/fXaBSmW/T5SzLAmCgzi+eiWE7fUvbHaeNBQH13UftjpXxsfLkMpgw==", + "dev": true + }, "browserify-aes": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/browserify-aes/-/browserify-aes-1.2.0.tgz", @@ -3444,6 +3450,12 @@ "repeating": "^2.0.0" } }, + "diff": { + "version": "3.5.0", + "resolved": "https://registry.npmjs.org/diff/-/diff-3.5.0.tgz", + "integrity": "sha512-A46qtFgd+g7pDZinpnwiRJtxbC1hpgf0uzP3iG89scHk0AUC7A1TGxf5OiiOUv/JMZR8GOt8hL900hV0bOy5xA==", + "dev": true + }, "diffie-hellman": { "version": "5.0.3", "resolved": "https://registry.npmjs.org/diffie-hellman/-/diffie-hellman-5.0.3.tgz", @@ -4991,6 +5003,12 @@ "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.1.11.tgz", "integrity": "sha1-Dovf5NHduIVNZOBOp8AOKgJuVlg=" }, + "growl": { + "version": "1.10.5", + "resolved": "https://registry.npmjs.org/growl/-/growl-1.10.5.tgz", + "integrity": "sha512-qBr4OuELkhPenW6goKVXiv47US3clb3/IbuWF9KNKEijAy9oeHxU9IgzjvJhHkUzhaj7rOUD7+YGWqUjLp5oSA==", + "dev": true + }, "gulp-clone": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/gulp-clone/-/gulp-clone-2.0.1.tgz", @@ -5161,6 +5179,12 @@ "minimalistic-assert": "^1.0.1" } }, + "he": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/he/-/he-1.1.1.tgz", + "integrity": "sha1-k0EP0hsAlzUVH4howvJx80J+I/0=", + "dev": true + }, "helmet": { "version": "3.14.0", "resolved": "https://registry.npmjs.org/helmet/-/helmet-3.14.0.tgz", @@ -6447,6 +6471,59 @@ "minimist": "0.0.8" } }, + "mocha": { + "version": "5.2.0", + "resolved": "https://registry.npmjs.org/mocha/-/mocha-5.2.0.tgz", + "integrity": "sha512-2IUgKDhc3J7Uug+FxMXuqIyYzH7gJjXECKe/w43IGgQHTSj3InJi+yAA7T24L9bQMRKiUEHxEX37G5JpVUGLcQ==", + "dev": true, + "requires": { + "browser-stdout": "1.3.1", + "commander": "2.15.1", + "debug": "3.1.0", + "diff": "3.5.0", + "escape-string-regexp": "1.0.5", + "glob": "7.1.2", + "growl": "1.10.5", + "he": "1.1.1", + "minimatch": "3.0.4", + "mkdirp": "0.5.1", + "supports-color": "5.4.0" + }, + "dependencies": { + "debug": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-3.1.0.tgz", + "integrity": "sha512-OX8XqP7/1a9cqkxYw2yXss15f26NKWBpDXQd0/uK/KPqdQhxbPa994hnzjcE2VqQpDslf55723cKPUOGSmMY3g==", + "dev": true, + "requires": { + "ms": "2.0.0" + } + }, + "glob": { + "version": "7.1.2", + "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.2.tgz", + "integrity": "sha512-MJTUg1kjuLeQCJ+ccE4Vpa6kKVXkPYJ2mOCQyUuKLcLQsdrMCpBPUi8qVE6+YuaJkozeA9NusTAw3hLr8Xe5EQ==", + "dev": true, + "requires": { + "fs.realpath": "^1.0.0", + "inflight": "^1.0.4", + "inherits": "2", + "minimatch": "^3.0.4", + "once": "^1.3.0", + "path-is-absolute": "^1.0.0" + } + }, + "supports-color": { + "version": "5.4.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.4.0.tgz", + "integrity": "sha512-zjaXglF5nnWpsq470jSv6P9DwPvgLkuapYmfDm3JWOm0vkNTVF2tI4UrN2r6jH1qM/uc/WtxYY1hYoA2dOKj5w==", + "dev": true, + "requires": { + "has-flag": "^3.0.0" + } + } + } + }, "moment": { "version": "2.22.2", "resolved": "https://registry.npmjs.org/moment/-/moment-2.22.2.tgz", diff --git a/package.json b/package.json index 9c7ee3d..d8868b5 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "testcafe": "run-s testcafe-suite0 testcafe-suite1", "testcafe-suite0": "cross-env-shell testcafe --hostname localhost --skip-js-errors -c 4 $BROWSER tests/spec/0*", "testcafe-suite1": "cross-env-shell testcafe --hostname localhost --skip-js-errors $BROWSER tests/spec/1*", + "test-unit": "mocha -r esm tests/unit/", "wait-for-mastodon-to-start": "node -r esm bin/wait-for-mastodon-to-start.js", "wait-for-mastodon-data": "node -r esm bin/wait-for-mastodon-data.js", "globalize-css": "node ./bin/globalize-css.js", @@ -104,7 +105,9 @@ "webpack-bundle-analyzer": "^3.0.3" }, "devDependencies": { + "assert": "^1.4.1", "eslint-plugin-html": "^5.0.0", + "mocha": "^5.2.0", "now": "^12.0.0", "standard": "^12.0.1", "testcafe": "^0.23.0" diff --git a/routes/_utils/emojiRegex.js b/routes/_utils/emojiRegex.js new file mode 100644 index 0000000..fb57bf2 --- /dev/null +++ b/routes/_utils/emojiRegex.js @@ -0,0 +1,8 @@ +import emojiRegex from 'emoji-regex/es2015/text' + +let theEmojiRegex + +export function getEmojiRegex () { + theEmojiRegex = theEmojiRegex || emojiRegex() // only init when needed, then cache + return theEmojiRegex +} diff --git a/routes/_utils/emojifyText.js b/routes/_utils/emojifyText.js index 00028b3..8c7581c 100644 --- a/routes/_utils/emojifyText.js +++ b/routes/_utils/emojifyText.js @@ -1,6 +1,11 @@ import { replaceAll } from './strings' +import { replaceEmoji } from './replaceEmoji' export function emojifyText (text, emojis, autoplayGifs) { + // replace native emoji with wrapped spans so we can give them the proper font-family + text = replaceEmoji(text, substring => `${substring}`) + + // replace custom emoji if (emojis) { for (let emoji of emojis) { let urlToUse = autoplayGifs ? emoji.url : emoji.static_url @@ -13,5 +18,6 @@ export function emojifyText (text, emojis, autoplayGifs) { ) } } + return text } diff --git a/routes/_utils/removeEmoji.js b/routes/_utils/removeEmoji.js index 5297144..5619865 100644 --- a/routes/_utils/removeEmoji.js +++ b/routes/_utils/removeEmoji.js @@ -1,7 +1,5 @@ import { replaceAll } from './strings' -import emojiRegex from 'emoji-regex/es2015/text.js' - -let theEmojiRegex +import { replaceEmoji } from './replaceEmoji' export function removeEmoji (text, emojis) { // remove custom emoji @@ -11,7 +9,6 @@ export function removeEmoji (text, emojis) { text = replaceAll(text, shortcodeWithColons, '') } } - // remove regular emoji - theEmojiRegex = theEmojiRegex || emojiRegex() // only init when needed, then cache - return text.replace(theEmojiRegex, '').trim() + // remove native emoji + return replaceEmoji(text, () => '').trim() } diff --git a/routes/_utils/replaceEmoji.js b/routes/_utils/replaceEmoji.js new file mode 100644 index 0000000..a76df3f --- /dev/null +++ b/routes/_utils/replaceEmoji.js @@ -0,0 +1,35 @@ +import { getEmojiRegex } from './emojiRegex' + +// replace emoji in HTML with something else, safely skipping HTML tags +export function replaceEmoji (string, replacer) { + let output = '' + let leftAngleBracketIdx = string.indexOf('<') + let currentIdx = 0 + let emojiRegex = getEmojiRegex() + + function safeReplacer (substring) { + if (substring.match(/^[0-9]+$/)) { // for some reason, emoji-regex matches digits + return substring + } + return replacer(substring) + } + + while (leftAngleBracketIdx !== -1) { + let substring = string.substring(currentIdx, leftAngleBracketIdx) + + output += substring.replace(emojiRegex, safeReplacer) + + let rightAngleBracketIdx = string.indexOf('>', leftAngleBracketIdx + 1) + if (rightAngleBracketIdx === -1) { // broken HTML, abort + output += string.substring(leftAngleBracketIdx, string.length) + return output + } + output += string.substring(leftAngleBracketIdx, rightAngleBracketIdx) + '>' + currentIdx = rightAngleBracketIdx + 1 + leftAngleBracketIdx = string.indexOf('<', currentIdx) + } + + output += string.substring(currentIdx, string.length).replace(emojiRegex, safeReplacer) + + return output +} diff --git a/scss/global.scss b/scss/global.scss index 5c59c5a..42d54c2 100644 --- a/scss/global.scss +++ b/scss/global.scss @@ -1,14 +1,11 @@ body { margin: 0; - font-family: system-ui, -apple-system, BlinkMacSystemFont, Segoe UI, Roboto, Oxygen, Ubuntu, Cantarell, Fira Sans, Droid Sans, Helvetica Neue, sans-serif; + font-family: system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Oxygen-Sans, Ubuntu, Cantarell, "Fira Sans", "Droid Sans", "Helvetica Neue", sans-serif; font-size: 14px; line-height: 1.4; color: var(--body-text-color); background: var(--body-bg); -webkit-tap-highlight-color: transparent; /* fix for blue background on spoiler tap on Chrome for Android */ - //-webkit-overflow-scrolling: touch; - //overflow-y: auto; - //overflow-x: hidden; } .main-content { @@ -214,3 +211,7 @@ textarea { object-fit: contain; vertical-align: middle; } + +.inline-emoji { + font-family: "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Twemoji Mozilla", "Noto Color Emoji", "EmojiOne Color", "Android Emoji", sans-serif; +} \ No newline at end of file diff --git a/templates/2xx.html b/templates/2xx.html index 1aa5d05..3027a2a 100644 --- a/templates/2xx.html +++ b/templates/2xx.html @@ -17,7 +17,7 @@ diff --git a/tests/unit/test-emoji.js b/tests/unit/test-emoji.js new file mode 100644 index 0000000..3d6a35f --- /dev/null +++ b/tests/unit/test-emoji.js @@ -0,0 +1,137 @@ +/* global describe, it */ + +import { replaceEmoji } from '../../routes/_utils/replaceEmoji' +import assert from 'assert' + +const mindBlown = String.fromCodePoint(0x1F92F) +const elephant = String.fromCodePoint(0x1F418) +const womanBowing = [0x1f647, 0x200d, 0x2640, 0xfe0f].map(_ => String.fromCodePoint(_)).join('') + +describe('test-emoji.js', function () { + it('does emoji replacement correctly', function () { + let replacer = _ => `
${_}
` + assert.strictEqual( + replaceEmoji('hello world', replacer), + 'hello world' + ) + assert.strictEqual( + replaceEmoji(`${mindBlown}`, replacer), + `
${mindBlown}
` + ) + assert.strictEqual( + replaceEmoji(`${mindBlown} ${elephant}`, replacer), + `
${mindBlown}
${elephant}
` + ) + assert.strictEqual( + replaceEmoji(`${elephant} woot ${mindBlown}`, replacer), + `
${elephant}
woot
${mindBlown}
` + ) + assert.strictEqual( + replaceEmoji(`woot ${mindBlown}`, replacer), + `woot
${mindBlown}
` + ) + assert.strictEqual( + replaceEmoji(`${mindBlown} woot`, replacer), + `
${mindBlown}
woot` + ) + }) + + it('handles multi-code emoji', function () { + let replacer = _ => `
${_}
` + assert.strictEqual( + replaceEmoji(`hello ${womanBowing}`, replacer), + `hello
${womanBowing}
` + ) + }) + + it('handles emoji mixed with custom emoji', function () { + let replacer = _ => `
${_}
` + assert.strictEqual( + replaceEmoji(`hello ${womanBowing} and :blobpats: and ${elephant}`, replacer), + `hello
${womanBowing}
and :blobpats: and
${elephant}
` + ) + }) + + it('handles sequential emoji', function () { + let replacer = _ => `
${_}
` + assert.strictEqual( + replaceEmoji(`${elephant}${elephant}${womanBowing}${mindBlown}`, replacer), + `
${elephant}
${elephant}
${womanBowing}
${mindBlown}
` + ) + }) + + it('does not replace digits', function () { + let replacer = _ => `
${_}
` + assert.strictEqual( + replaceEmoji(`it's over 9000`, replacer), + `it's over 9000` + ) + }) + + it('does not replace emoji inside HTML tags', function () { + let replacer = _ => `
${_}
` + assert.strictEqual( + replaceEmoji(`check this cool link: link`, replacer), + `check this cool link: link` + ) + assert.strictEqual( + replaceEmoji( + `link and link`, + replacer + ), + `link and link` + ) + assert.strictEqual( + replaceEmoji( + `link and ${mindBlown}`, + replacer + ), + `link and
${mindBlown}
` + ) + assert.strictEqual( + replaceEmoji( + `link and ${mindBlown} and ` + + `link`, + replacer + ), + `link and
${mindBlown}
and ` + + `link` + ) + }) + + it('removes emoji', function () { + let replacer = _ => '' + assert.strictEqual( + replaceEmoji(`woot ${mindBlown}`, replacer), + `woot ` + ) + assert.strictEqual( + replaceEmoji(`woot ${mindBlown} woot`, replacer), + `woot woot` + ) + assert.strictEqual( + replaceEmoji(`woot ${mindBlown}${elephant}`, replacer), + `woot ` + ) + assert.strictEqual( + replaceEmoji(`woot ${mindBlown}${elephant} woot`, replacer), + `woot woot` + ) + }) + + it('can handle a dangling left angle bracket for some reason', function () { + let replacer = _ => `
${_}
` + assert.strictEqual( + replaceEmoji(`woot ${mindBlown} <`, replacer), + `woot
${mindBlown}
<` + ) + assert.strictEqual( + replaceEmoji(`woot ${mindBlown} ${mindBlown}