From 6fb49e74a1036d2de3ca3dce62463981b8683e4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Tue, 19 Mar 2019 12:02:47 +0100 Subject: [PATCH] Revert "UX: Better emoji escaping for topic title (#7176)" (#7201) This reverts commit 0d9bc0aaa6dde5a49b9315124a6343a8ffb41a16. --- .../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, 25 insertions(+), 77 deletions(-) diff --git a/app/assets/javascripts/pretty-text/emoji.js.es6 b/app/assets/javascripts/pretty-text/emoji.js.es6 index d3ef7edd75b..5b48a649e4e 100644 --- a/app/assets/javascripts/pretty-text/emoji.js.es6 +++ b/app/assets/javascripts/pretty-text/emoji.js.es6 @@ -3,8 +3,7 @@ import { aliases, searchAliases, translations, - tonableEmojis, - replacements + tonableEmojis } from "pretty-text/emoji/data"; import { IMAGE_VERSION } from "pretty-text/emoji/version"; @@ -35,48 +34,25 @@ export function performEmojiUnescape(string, opts) { return; } - 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"; + // 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 url && (isEmoticon || hasEndingColon || isUnicodeEmoticon) - ? `${emojiVal}` - : m; - }); - - return string; -} - -export function performEmojiEscape(string) { - if (!string) { - return; + return url && (isEmoticon || hasEndingColon) + ? `${emojiVal}` + : m; + }); } - 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 58251707909..32a9b986bdc 100644 --- a/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb +++ b/app/assets/javascripts/pretty-text/emoji/data.js.es6.erb @@ -3,4 +3,3 @@ 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 db3261aeda6..495d56bcbd8 100644 --- a/app/models/emoji.rb +++ b/app/models/emoji.rb @@ -157,7 +157,13 @@ class Emoji end def self.unicode_unescape(string) - PrettyText.escape_emoji(string) + string.each_char.map do |c| + if str = unicode_replacements[c] + ":#{str}:" + else + c + end + end.join end def self.gsub_emoji_to_unicode(str) diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 1943417ef25..d4d8ce56f87 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? && title + return title unless SiteSetting.enable_emoji? set = SiteSetting.emoji_set.inspect custom = Emoji.custom.map { |e| [e.name, e.url] }.to_h.to_json @@ -239,16 +239,6 @@ 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 929dafd13ab..649c3e4b39c 100644 --- a/lib/pretty_text/shims.js +++ b/lib/pretty_text/shims.js @@ -1,7 +1,6 @@ __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 65a9be83bbe..c28ce6f0557 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -279,7 +279,6 @@ 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") @@ -289,10 +288,6 @@ 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 6ba79ef8116..73447b5b0e2 100644 --- a/test/javascripts/acceptance/topic-test.js.es6 +++ b/test/javascripts/acceptance/topic-test.js.es6 @@ -184,23 +184,6 @@ 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: {