From d32557ea326735c33a420dee7cc3de856c683dc7 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Wed, 13 Mar 2019 13:02:56 +0100 Subject: [PATCH] Revert "FIX: Better emoji escaping for topic title" This reverts commit 35426b5ad6b00c9047b9b7ba17d42d4f30bdb488. --- app/assets/javascripts/pretty-text-bundle.js | 1 - .../javascripts/pretty-text/emoji.js.es6 | 175 ------------------ .../javascripts/pretty-text/emoji.js.es6.erb | 127 +++++++++++++ .../pretty-text/emoji/data.js.es6.erb | 1 - .../pretty-text/emoji/version.js.es6.erb | 2 - app/models/emoji.rb | 8 +- lib/pretty_text.rb | 12 +- lib/pretty_text/shims.js | 1 - spec/models/topic_spec.rb | 5 - .../acceptance/emoji-picker-test.js.es6 | 2 +- test/javascripts/acceptance/emoji-test.js.es6 | 2 +- test/javascripts/acceptance/topic-test.js.es6 | 19 +- test/javascripts/lib/emoji-test.js.es6 | 3 +- test/javascripts/lib/pretty-text-test.js.es6 | 2 +- test/javascripts/models/topic-test.js.es6 | 2 +- 15 files changed, 141 insertions(+), 221 deletions(-) delete mode 100644 app/assets/javascripts/pretty-text/emoji.js.es6 create mode 100644 app/assets/javascripts/pretty-text/emoji.js.es6.erb delete mode 100644 app/assets/javascripts/pretty-text/emoji/version.js.es6.erb diff --git a/app/assets/javascripts/pretty-text-bundle.js b/app/assets/javascripts/pretty-text-bundle.js index ed5331b6435..41324ba31e5 100644 --- a/app/assets/javascripts/pretty-text-bundle.js +++ b/app/assets/javascripts/pretty-text-bundle.js @@ -2,7 +2,6 @@ //= require ./pretty-text/guid //= require ./pretty-text/censored-words //= require ./pretty-text/emoji/data -//= require ./pretty-text/emoji/version //= require ./pretty-text/emoji //= require ./pretty-text/engines/discourse-markdown-it //= require xss.min diff --git a/app/assets/javascripts/pretty-text/emoji.js.es6 b/app/assets/javascripts/pretty-text/emoji.js.es6 deleted file mode 100644 index d2c8503d692..00000000000 --- a/app/assets/javascripts/pretty-text/emoji.js.es6 +++ /dev/null @@ -1,175 +0,0 @@ -import { - emojis, - aliases, - searchAliases, - translations, - tonableEmojis, - replacements -} from "pretty-text/emoji/data"; -import { IMAGE_VERSION } from "pretty-text/emoji/version"; - -const extendedEmoji = {}; - -export function registerEmoji(code, url) { - code = code.toLowerCase(); - extendedEmoji[code] = url; -} - -export function extendedEmojiList() { - return extendedEmoji; -} - -const emojiHash = {}; - -// add all default emojis -emojis.forEach(code => (emojiHash[code] = true)); - -// and their aliases -const aliasHash = {}; -Object.keys(aliases).forEach(name => { - aliases[name].forEach(alias => (aliasHash[alias] = name)); -}); - -export function performEmojiUnescape(string, opts) { - if (!string) { - return; - } - - return string.replace(/[\u1000-\uFFFF]+|\B:[^\s:]+(?::t\d)?:?\B/g, m => { - const isEmoticon = !!translations[m]; - const isUnicodeEmoticon = !!replacements[m]; - const emojiVal = isEmoticon - ? translations[m] - : isUnicodeEmoticon - ? replacements[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 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; -} - -export function isCustomEmoji(code, opts) { - code = code.toLowerCase(); - if (extendedEmoji.hasOwnProperty(code)) return true; - if (opts && opts.customEmoji && opts.customEmoji.hasOwnProperty(code)) - return true; - return false; -} - -export function buildEmojiUrl(code, opts) { - let url; - code = String(code).toLowerCase(); - if (extendedEmoji.hasOwnProperty(code)) { - url = extendedEmoji[code]; - } - - if (opts && opts.customEmoji && opts.customEmoji[code]) { - url = opts.customEmoji[code]; - } - - const noToneMatch = code.match(/([^:]+):?/); - if ( - noToneMatch && - !url && - (emojiHash.hasOwnProperty(noToneMatch[1]) || - aliasHash.hasOwnProperty(noToneMatch[1])) - ) { - url = opts.getURL( - `/images/emoji/${opts.emojiSet}/${code.replace(/:t/, "/")}.png` - ); - } - - if (url) { - url = url + "?v=" + IMAGE_VERSION; - } - - return url; -} - -export function emojiExists(code) { - code = code.toLowerCase(); - return !!( - extendedEmoji.hasOwnProperty(code) || - emojiHash.hasOwnProperty(code) || - aliasHash.hasOwnProperty(code) - ); -} - -let toSearch; -export function emojiSearch(term, options) { - const maxResults = (options && options["maxResults"]) || -1; - if (maxResults === 0) { - return []; - } - - toSearch = - toSearch || - _.union(_.keys(emojiHash), _.keys(extendedEmoji), _.keys(aliasHash)).sort(); - - const results = []; - - function addResult(t) { - const val = aliasHash[t] || t; - if (results.indexOf(val) === -1) { - results.push(val); - } - } - - // if term matches from beginning - for (let i = 0; i < toSearch.length; i++) { - const item = toSearch[i]; - if (item.indexOf(term) === 0) addResult(item); - } - - if (searchAliases[term]) { - results.push.apply(results, searchAliases[term]); - } - - for (let i = 0; i < toSearch.length; i++) { - const item = toSearch[i]; - if (item.indexOf(term) > 0) addResult(item); - } - - if (maxResults === -1) { - return results; - } else { - return results.slice(0, maxResults); - } -} - -export function isSkinTonableEmoji(term) { - const match = _.compact(term.split(":"))[0]; - if (match) { - return tonableEmojis.indexOf(match) !== -1; - } - return false; -} diff --git a/app/assets/javascripts/pretty-text/emoji.js.es6.erb b/app/assets/javascripts/pretty-text/emoji.js.es6.erb new file mode 100644 index 00000000000..09ab4ce5ed6 --- /dev/null +++ b/app/assets/javascripts/pretty-text/emoji.js.es6.erb @@ -0,0 +1,127 @@ +import { emojis, aliases, searchAliases, translations, tonableEmojis } from 'pretty-text/emoji/data'; + +// bump up this number to expire all emojis +export const IMAGE_VERSION = "<%= Emoji::EMOJI_VERSION %>"; + +const extendedEmoji = {}; + +export function registerEmoji(code, url) { + code = code.toLowerCase(); + extendedEmoji[code] = url; +} + +export function extendedEmojiList() { + return extendedEmoji; +} + +const emojiHash = {}; + +// add all default emojis +emojis.forEach(code => emojiHash[code] = true); + +// and their aliases +const aliasHash = {}; +Object.keys(aliases).forEach(name => { + aliases[name].forEach(alias => aliasHash[alias] = name); +}); + +export function performEmojiUnescape(string, opts) { + if (!string) { 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 url && (isEmoticon || hasEndingColon) ? + `${emojiVal}` : m; + }); + } + + return string; +} + +export function isCustomEmoji(code, opts) { + code = code.toLowerCase(); + if (extendedEmoji.hasOwnProperty(code)) return true; + if (opts && opts.customEmoji && opts.customEmoji.hasOwnProperty(code)) return true; + return false; +} + +export function buildEmojiUrl(code, opts) { + let url; + code = String(code).toLowerCase(); + if (extendedEmoji.hasOwnProperty(code)) { + url = extendedEmoji[code]; + } + + if (opts && opts.customEmoji && opts.customEmoji[code]) { + url = opts.customEmoji[code]; + } + + const noToneMatch = code.match(/([^:]+):?/); + if (noToneMatch && !url && (emojiHash.hasOwnProperty(noToneMatch[1]) || aliasHash.hasOwnProperty(noToneMatch[1]))) { + url = opts.getURL(`/images/emoji/${opts.emojiSet}/${code.replace(/:t/, '/')}.png`); + } + + if (url) { + url = url + "?v=" + IMAGE_VERSION; + } + + return url; +} + +export function emojiExists(code) { + code = code.toLowerCase(); + return !!(extendedEmoji.hasOwnProperty(code) || emojiHash.hasOwnProperty(code) || aliasHash.hasOwnProperty(code)); +}; + +let toSearch; +export function emojiSearch(term, options) { + const maxResults = (options && options["maxResults"]) || -1; + if (maxResults === 0) { return []; } + + toSearch = toSearch || _.union(_.keys(emojiHash), _.keys(extendedEmoji), _.keys(aliasHash)).sort(); + + const results = []; + + function addResult(t) { + const val = aliasHash[t] || t; + if (results.indexOf(val) === -1) { + results.push(val); + } + } + + // if term matches from beginning + for (let i=0; i 0) addResult(item); + } + + if (maxResults === -1) { + return results; + } else { + return results.slice(0, maxResults); + } +}; + +export function isSkinTonableEmoji(term) { + const match = _.compact(term.split(":"))[0]; + if (match) { + return tonableEmojis.indexOf(match) !== -1; + } + return false; +} 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/assets/javascripts/pretty-text/emoji/version.js.es6.erb b/app/assets/javascripts/pretty-text/emoji/version.js.es6.erb deleted file mode 100644 index b846ca59d55..00000000000 --- a/app/assets/javascripts/pretty-text/emoji/version.js.es6.erb +++ /dev/null @@ -1,2 +0,0 @@ -// bump up this number to expire all emojis -export const IMAGE_VERSION = "<%= Emoji::EMOJI_VERSION %>"; diff --git a/app/models/emoji.rb b/app/models/emoji.rb index ebe58ea311e..c20d26ef061 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 634f403993a..d0ecd98a432 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/emoji-picker-test.js.es6 b/test/javascripts/acceptance/emoji-picker-test.js.es6 index 9122cd8ea5b..3c2ebe7be4b 100644 --- a/test/javascripts/acceptance/emoji-picker-test.js.es6 +++ b/test/javascripts/acceptance/emoji-picker-test.js.es6 @@ -1,5 +1,5 @@ import { acceptance } from "helpers/qunit-helpers"; -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { IMAGE_VERSION as v } from "pretty-text/emoji"; import { resetCache } from "discourse/components/emoji-picker"; acceptance("EmojiPicker", { diff --git a/test/javascripts/acceptance/emoji-test.js.es6 b/test/javascripts/acceptance/emoji-test.js.es6 index 39dc37696e6..48ac7f9bcc4 100644 --- a/test/javascripts/acceptance/emoji-test.js.es6 +++ b/test/javascripts/acceptance/emoji-test.js.es6 @@ -1,4 +1,4 @@ -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { IMAGE_VERSION as v } from "pretty-text/emoji"; import { acceptance } from "helpers/qunit-helpers"; acceptance("Emoji", { loggedIn: true }); diff --git a/test/javascripts/acceptance/topic-test.js.es6 b/test/javascripts/acceptance/topic-test.js.es6 index 1c371c41d1c..c38e5743452 100644 --- a/test/javascripts/acceptance/topic-test.js.es6 +++ b/test/javascripts/acceptance/topic-test.js.es6 @@ -1,5 +1,5 @@ import { acceptance } from "helpers/qunit-helpers"; -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { IMAGE_VERSION as v } from "pretty-text/emoji"; acceptance("Topic", { loggedIn: true, @@ -121,23 +121,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: { diff --git a/test/javascripts/lib/emoji-test.js.es6 b/test/javascripts/lib/emoji-test.js.es6 index c3b8ccd5630..47665a8c8ae 100644 --- a/test/javascripts/lib/emoji-test.js.es6 +++ b/test/javascripts/lib/emoji-test.js.es6 @@ -1,5 +1,4 @@ -import { emojiSearch } from "pretty-text/emoji"; -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { emojiSearch, IMAGE_VERSION as v } from "pretty-text/emoji"; import { emojiUnescape } from "discourse/lib/text"; QUnit.module("lib:emoji"); diff --git a/test/javascripts/lib/pretty-text-test.js.es6 b/test/javascripts/lib/pretty-text-test.js.es6 index 9495122a87c..162ef49691f 100644 --- a/test/javascripts/lib/pretty-text-test.js.es6 +++ b/test/javascripts/lib/pretty-text-test.js.es6 @@ -1,7 +1,7 @@ import Quote from "discourse/lib/quote"; import Post from "discourse/models/post"; import { default as PrettyText, buildOptions } from "pretty-text/pretty-text"; -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { IMAGE_VERSION as v } from "pretty-text/emoji"; import { INLINE_ONEBOX_LOADING_CSS_CLASS } from "pretty-text/inline-oneboxer"; QUnit.module("lib:pretty-text"); diff --git a/test/javascripts/models/topic-test.js.es6 b/test/javascripts/models/topic-test.js.es6 index e385e1e92f7..ca6bd1f7b53 100644 --- a/test/javascripts/models/topic-test.js.es6 +++ b/test/javascripts/models/topic-test.js.es6 @@ -1,4 +1,4 @@ -import { IMAGE_VERSION as v } from "pretty-text/emoji/version"; +import { IMAGE_VERSION as v } from "pretty-text/emoji"; QUnit.module("model:topic");