From 9ebb69e8ebfdb9beaf8682939932cc70fcb46fa8 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Tue, 3 Dec 2019 17:32:33 +0100 Subject: [PATCH] FIX: Respect `enable_inline_emoji_translation` setting in titles --- .../javascripts/discourse/lib/text.js.es6 | 3 +- .../javascripts/pretty-text/emoji.js.es6 | 73 +++++++++++++++---- .../engines/discourse-markdown/quotes.js.es6 | 4 +- lib/pretty_text.rb | 9 ++- spec/models/topic_spec.rb | 11 +++ test/javascripts/acceptance/topic-test.js.es6 | 21 ++++++ test/javascripts/helpers/site-settings.js | 2 + test/javascripts/lib/emoji-test.js.es6 | 35 +++++++++ 8 files changed, 141 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/text.js.es6 b/app/assets/javascripts/discourse/lib/text.js.es6 index 2767426c09e..7862ce53e01 100644 --- a/app/assets/javascripts/discourse/lib/text.js.es6 +++ b/app/assets/javascripts/discourse/lib/text.js.es6 @@ -68,7 +68,8 @@ function emojiOptions() { return { getURL: Discourse.getURLWithCDN, emojiSet: Discourse.SiteSettings.emoji_set, - enableEmojiShortcuts: Discourse.SiteSettings.enable_emoji_shortcuts + enableEmojiShortcuts: Discourse.SiteSettings.enable_emoji_shortcuts, + inlineEmoji: Discourse.SiteSettings.enable_inline_emoji_translation }; } diff --git a/app/assets/javascripts/pretty-text/emoji.js.es6 b/app/assets/javascripts/pretty-text/emoji.js.es6 index 2089902c3b5..fec9b725c4d 100644 --- a/app/assets/javascripts/pretty-text/emoji.js.es6 +++ b/app/assets/javascripts/pretty-text/emoji.js.es6 @@ -42,10 +42,31 @@ export function buildReplacementsList(emojiReplacements) { .join("|"); } -const unicodeRegexp = new RegExp( - buildReplacementsList(replacements) + "|\\B:[^\\s:]+(?::t\\d)?:?\\B", - "g" -); +let replacementListCache; +const unicodeRegexpCache = {}; + +function replacementList() { + if (replacementListCache === undefined) { + replacementListCache = buildReplacementsList(replacements); + } + + return replacementListCache; +} + +function unicodeRegexp(inlineEmoji) { + if (unicodeRegexpCache[inlineEmoji] === undefined) { + const emojiExpression = inlineEmoji + ? "|:[^\\s:]+(?::t\\d)?:?" + : "|\\B:[^\\s:]+(?::t\\d)?:?\\B"; + + unicodeRegexpCache[inlineEmoji] = new RegExp( + replacementList() + emojiExpression, + "g" + ); + } + + return unicodeRegexpCache[inlineEmoji]; +} // add all default emojis emojis.forEach(code => (emojiHash[code] = true)); @@ -56,12 +77,29 @@ Object.keys(aliases).forEach(name => { aliases[name].forEach(alias => (aliasHash[alias] = name)); }); +function isReplacableInlineEmoji(string, index, inlineEmoji) { + if (inlineEmoji) return true; + + // index depends on regex; when `inlineEmoji` is false, the regex starts + // with a `\B` character, so there's no need to subtract from the index + const beforeEmoji = string.slice(0, index - (inlineEmoji ? 1 : 0)); + + return ( + beforeEmoji.length === 0 || + /(?:\s|[>.,\/#!$%^&*;:{}=\-_`~()])$/.test(beforeEmoji) || + new RegExp(`(?:${replacementList()})$`).test(beforeEmoji) + ); +} + export function performEmojiUnescape(string, opts) { if (!string) { return; } - return string.replace(unicodeRegexp, m => { + const inlineEmoji = opts.inlineEmoji; + const regexp = unicodeRegexp(inlineEmoji); + + return string.replace(regexp, (m, index) => { const isEmoticon = opts.enableEmojiShortcuts && !!translations[m]; const isUnicodeEmoticon = !!replacements[m]; let emojiVal; @@ -78,7 +116,11 @@ export function performEmojiUnescape(string, opts) { ? "emoji emoji-custom" : "emoji"; - return url && (isEmoticon || hasEndingColon || isUnicodeEmoticon) + const isReplacable = + (isEmoticon || hasEndingColon || isUnicodeEmoticon) && + isReplacableInlineEmoji(string, index, inlineEmoji); + + return url && isReplacable ? `${emojiVal}` @@ -89,14 +131,19 @@ export function performEmojiUnescape(string, opts) { } export function performEmojiEscape(string, opts) { - return string.replace(unicodeRegexp, m => { - if (!!translations[m]) { - return opts.emojiShortcuts ? `:${translations[m]}:` : m; - } else if (!!replacements[m]) { - return `:${replacements[m]}:`; - } else { - return m; + const inlineEmoji = opts.inlineEmoji; + const regexp = unicodeRegexp(inlineEmoji); + + return string.replace(regexp, (m, index) => { + if (isReplacableInlineEmoji(string, index, inlineEmoji)) { + if (!!translations[m]) { + return opts.emojiShortcuts ? `:${translations[m]}:` : m; + } else if (!!replacements[m]) { + return `:${replacements[m]}:`; + } } + + return m; }); return string; diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6 index e1c9a31bf08..6c2860577ae 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6 +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6 @@ -120,7 +120,8 @@ const rule = { title = performEmojiUnescape(topicInfo.title, { getURL: options.getURL, emojiSet: options.emojiSet, - enableEmojiShortcuts: options.enableEmojiShortcuts + enableEmojiShortcuts: options.enableEmojiShortcuts, + inlineEmoji: options.inlineEmoji }); } @@ -156,6 +157,7 @@ export function setup(helper) { opts.enableEmoji = siteSettings.enable_emoji; opts.emojiSet = siteSettings.emoji_set; opts.enableEmojiShortcuts = siteSettings.enable_emoji_shortcuts; + opts.inlineEmoji = siteSettings.enable_inline_emoji_translation; }); helper.registerPlugin(md => { diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 761432b26b3..b01117733a1 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -218,6 +218,7 @@ module PrettyText set = SiteSetting.emoji_set.inspect custom = Emoji.custom.map { |e| [e.name, e.url] }.to_h.to_json + protect do v8.eval(<<~JS) __paths = #{paths_json}; @@ -225,7 +226,8 @@ module PrettyText getURL: __getURL, emojiSet: #{set}, customEmoji: #{custom}, - enableEmojiShortcuts: #{SiteSetting.enable_emoji_shortcuts} + enableEmojiShortcuts: #{SiteSetting.enable_emoji_shortcuts}, + inlineEmoji: #{SiteSetting.enable_inline_emoji_translation} }); JS end @@ -238,7 +240,10 @@ module PrettyText protect do v8.eval(<<~JS) - __performEmojiEscape(#{title.inspect}, { emojiShortcuts: #{replace_emoji_shortcuts} }); + __performEmojiEscape(#{title.inspect}, { + emojiShortcuts: #{replace_emoji_shortcuts}, + inlineEmoji: #{SiteSetting.enable_inline_emoji_translation} + }); JS end end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index e3932127954..26e2563cd88 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -308,6 +308,7 @@ describe Topic do let(:topic_emoji) { build_topic_with_title("I 💖 candy alot") } let(:topic_modifier_emoji) { build_topic_with_title("I 👨‍🌾 candy alot") } let(:topic_shortcut_emoji) { build_topic_with_title("I love candy :)") } + let(:topic_inline_emoji) { build_topic_with_title("Hello😊World") } it "escapes script contents" do expect(topic_script.fancy_title).to eq("Topic with <script>alert(‘title’)</script> script in its title") @@ -359,6 +360,16 @@ describe Topic do expect(topic_shortcut_emoji.fancy_title).to eq("I love candy :)") end end + + it "keeps inline emojis if inline emoji setting disabled" do + SiteSetting.enable_inline_emoji_translation = false + expect(topic_inline_emoji.fancy_title).to eq("Hello😊World") + end + + it "expands inline emojis if inline emoji setting enabled" do + SiteSetting.enable_inline_emoji_translation = true + expect(topic_inline_emoji.fancy_title).to eq("Hello:blush:World") + end end context 'fancy title' do diff --git a/test/javascripts/acceptance/topic-test.js.es6 b/test/javascripts/acceptance/topic-test.js.es6 index d856cc9eb1f..0e293e20f74 100644 --- a/test/javascripts/acceptance/topic-test.js.es6 +++ b/test/javascripts/acceptance/topic-test.js.es6 @@ -186,6 +186,27 @@ QUnit.test("Updating the topic title with unicode emojis", async assert => { ); }); +QUnit.test( + "Updating the topic title with unicode emojis without whitespaces", + async assert => { + Discourse.SiteSettings.enable_inline_emoji_translation = true; + await visit("/t/internationalization-localization/280"); + await click("#topic-title .d-icon-pencil-alt"); + + await fillIn("#edit-title", "Test🙂Title"); + + await click("#topic-title .submit-edit"); + + assert.equal( + find(".fancy-title") + .html() + .trim(), + `Testslightly_smiling_faceTitle`, + "it displays the new title with escaped unicode emojis" + ); + } +); + acceptance("Topic featured links", { loggedIn: true, settings: { diff --git a/test/javascripts/helpers/site-settings.js b/test/javascripts/helpers/site-settings.js index 5ca4cbb867b..25568eda89d 100644 --- a/test/javascripts/helpers/site-settings.js +++ b/test/javascripts/helpers/site-settings.js @@ -93,6 +93,8 @@ Discourse.SiteSettingsOriginal = { enable_emoji: true, enable_emoji_shortcuts: true, emoji_set: "emoji_one", + enable_emoji_shortcuts: true, + enable_inline_emoji_translation: false, desktop_category_page_style: "categories_and_latest_topics", enable_mentions: true, enable_personal_messages: true, diff --git a/test/javascripts/lib/emoji-test.js.es6 b/test/javascripts/lib/emoji-test.js.es6 index 69244d06c49..ca06ce07561 100644 --- a/test/javascripts/lib/emoji-test.js.es6 +++ b/test/javascripts/lib/emoji-test.js.es6 @@ -92,6 +92,41 @@ QUnit.test("emojiUnescape", assert => { "no emoticons when emoji shortcuts are disabled", { enable_emoji_shortcuts: false } ); + testUnescape( + "Hello 😊 World", + `Hello blush World`, + "emoji from Unicode emoji" + ); + testUnescape( + "Hello😊World", + "Hello😊World", + "keeps Unicode emoji when inline translation disabled", + { + enable_inline_emoji_translation: false + } + ); + testUnescape( + "Hello😊World", + `HelloblushWorld`, + "emoji from Unicode emoji when inline translation enabled", + { + enable_inline_emoji_translation: true + } + ); + testUnescape( + "hi:smile:", + "hi:smile:", + "no emojis when inline translation disabled", + { + enable_inline_emoji_translation: false + } + ); + testUnescape( + "hi:smile:", + `hismile`, + "emoji when inline translation enabled", + { enable_inline_emoji_translation: true } + ); }); QUnit.test("Emoji search", assert => {