From 0d9bc0aaa6dde5a49b9315124a6343a8ffb41a16 Mon Sep 17 00:00:00 2001 From: Tim Lange Date: Tue, 19 Mar 2019 09:33:10 +0100 Subject: [PATCH] UX: Better emoji escaping for topic title (#7176) --- .../javascripts/pretty-text/emoji.js.es6 | 58 +++++++++++++------ .../pretty-text/emoji/data.js.es6.erb | 1 + app/models/emoji.rb | 8 +-- lib/pretty_text.rb | 12 +++- lib/pretty_text/shims.js | 1 + spec/models/topic_spec.rb | 5 ++ test/javascripts/acceptance/topic-test.js.es6 | 17 ++++++ 7 files changed, 77 insertions(+), 25 deletions(-) diff --git a/app/assets/javascripts/pretty-text/emoji.js.es6 b/app/assets/javascripts/pretty-text/emoji.js.es6 index 5b48a649e4e..d3ef7edd75b 100644 --- a/app/assets/javascripts/pretty-text/emoji.js.es6 +++ b/app/assets/javascripts/pretty-text/emoji.js.es6 @@ -3,7 +3,8 @@ import { aliases, searchAliases, translations, - tonableEmojis + tonableEmojis, + replacements } from "pretty-text/emoji/data"; import { IMAGE_VERSION } from "pretty-text/emoji/version"; @@ -34,25 +35,48 @@ export function performEmojiUnescape(string, opts) { return; } - // this can be further improved by supporting matches of emoticons that don't begin with a colon - if (string.indexOf(":") >= 0) { - return string.replace(/\B:[^\s:]+(?::t\d)?:?\B/g, m => { - const isEmoticon = !!translations[m]; - const emojiVal = isEmoticon ? translations[m] : m.slice(1, m.length - 1); - const hasEndingColon = m.lastIndexOf(":") === m.length - 1; - const url = buildEmojiUrl(emojiVal, opts); - const classes = isCustomEmoji(emojiVal, opts) - ? "emoji emoji-custom" - : "emoji"; + return string.replace(/[\u1000-\uFFFF]+|\B:[^\s:]+(?::t\d)?:?\B/g, m => { + const isEmoticon = !!translations[m]; + const isUnicodeEmoticon = !!replacements[m]; + let emojiVal; + if (isEmoticon) { + emojiVal = translations[m]; + } else if (isUnicodeEmoticon) { + emojiVal = replacements[m]; + } else { + emojiVal = m.slice(1, m.length - 1); + } + const hasEndingColon = m.lastIndexOf(":") === m.length - 1; + const url = buildEmojiUrl(emojiVal, opts); + const classes = isCustomEmoji(emojiVal, opts) + ? "emoji emoji-custom" + : "emoji"; - return url && (isEmoticon || hasEndingColon) - ? `${emojiVal}` - : m; - }); + return url && (isEmoticon || hasEndingColon || isUnicodeEmoticon) + ? `${emojiVal}` + : m; + }); + + return string; +} + +export function performEmojiEscape(string) { + if (!string) { + return; } + return string.replace(/[\u1000-\uFFFF]+|\B:[^\s:]+(?::t\d)?:?\B/g, m => { + if (!!translations[m]) { + return ":" + translations[m] + ":"; + } else if (!!replacements[m]) { + return ":" + replacements[m] + ":"; + } else { + return m; + } + }); + return string; } diff --git a/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb b/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb index 32a9b986bdc..58251707909 100644 --- a/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb +++ b/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb @@ -3,3 +3,4 @@ export const tonableEmojis = <%= Emoji.tonable_emojis.flatten.inspect %>; export const aliases = <%= Emoji.aliases.inspect.gsub("=>", ":") %>; export const searchAliases = <%= Emoji.search_aliases.inspect.gsub("=>", ":") %>; export const translations = <%= Emoji.translations.inspect.gsub("=>", ":") %>; +export const replacements = <%= Emoji.unicode_replacements_json %>; diff --git a/app/models/emoji.rb b/app/models/emoji.rb index 495d56bcbd8..db3261aeda6 100644 --- a/app/models/emoji.rb +++ b/app/models/emoji.rb @@ -157,13 +157,7 @@ class Emoji end def self.unicode_unescape(string) - string.each_char.map do |c| - if str = unicode_replacements[c] - ":#{str}:" - else - c - end - end.join + PrettyText.escape_emoji(string) end def self.gsub_emoji_to_unicode(str) diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index d4d8ce56f87..1943417ef25 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -227,7 +227,7 @@ module PrettyText end def self.unescape_emoji(title) - return title unless SiteSetting.enable_emoji? + return title unless SiteSetting.enable_emoji? && title set = SiteSetting.emoji_set.inspect custom = Emoji.custom.map { |e| [e.name, e.url] }.to_h.to_json @@ -239,6 +239,16 @@ module PrettyText end end + def self.escape_emoji(title) + return unless title + + protect do + v8.eval(<<~JS) + __performEmojiEscape(#{title.inspect}); + JS + end + end + def self.cook(text, opts = {}) options = opts.dup diff --git a/lib/pretty_text/shims.js b/lib/pretty_text/shims.js index 649c3e4b39c..929dafd13ab 100644 --- a/lib/pretty_text/shims.js +++ b/lib/pretty_text/shims.js @@ -1,6 +1,7 @@ __PrettyText = require("pretty-text/pretty-text").default; __buildOptions = require("pretty-text/pretty-text").buildOptions; __performEmojiUnescape = require("pretty-text/emoji").performEmojiUnescape; +__performEmojiEscape = require("pretty-text/emoji").performEmojiEscape; __utils = require("discourse/lib/utilities"); diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index c28ce6f0557..65a9be83bbe 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -279,6 +279,7 @@ describe Topic do let(:topic_image) { build_topic_with_title("Topic with image in its title") } let(:topic_script) { build_topic_with_title("Topic with script in its title") } let(:topic_emoji) { build_topic_with_title("I 💖 candy alot") } + let(:topic_modifier_emoji) { build_topic_with_title("I 👨‍🌾 candy alot") } it "escapes script contents" do expect(topic_script.fancy_title).to eq("Topic with <script>alert(‘title’)</script> script in its title") @@ -288,6 +289,10 @@ describe Topic do expect(topic_emoji.fancy_title).to eq("I :sparkling_heart: candy alot") end + it "keeps combined emojis" do + expect(topic_modifier_emoji.fancy_title).to eq("I :man_farmer: candy alot") + end + it "escapes bold contents" do expect(topic_bold.fancy_title).to eq("Topic with <b>bold</b> text in its title") end diff --git a/test/javascripts/acceptance/topic-test.js.es6 b/test/javascripts/acceptance/topic-test.js.es6 index 73447b5b0e2..6ba79ef8116 100644 --- a/test/javascripts/acceptance/topic-test.js.es6 +++ b/test/javascripts/acceptance/topic-test.js.es6 @@ -184,6 +184,23 @@ QUnit.test("Updating the topic title with emojis", async assert => { ); }); +QUnit.test("Updating the topic title with unicode emojis", async assert => { + await visit("/t/internationalization-localization/280"); + await click("#topic-title .d-icon-pencil-alt"); + + await fillIn("#edit-title", "emojis title 👨‍🌾"); + + await click("#topic-title .submit-edit"); + + assert.equal( + find(".fancy-title") + .html() + .trim(), + `emojis title man_farmer`, + "it displays the new title with escaped unicode emojis" + ); +}); + acceptance("Topic featured links", { loggedIn: true, settings: {