diff --git a/frontend_tests/node_tests/composebox_typeahead.js b/frontend_tests/node_tests/composebox_typeahead.js index 7af5b77699..d811f60c2a 100644 --- a/frontend_tests/node_tests/composebox_typeahead.js +++ b/frontend_tests/node_tests/composebox_typeahead.js @@ -44,7 +44,10 @@ set_global('$', global.make_zjquery()); set_global('page_params', {}); set_global('channel', {}); -set_global('emoji', {emojis: emoji_list}); +set_global('emoji', { + active_realm_emojis: {}, + emojis: emoji_list, +}); set_global('pygments_data', {langs: {python: 0, javscript: 1, html: 2, css: 3}, }); @@ -832,14 +835,13 @@ global.people.add(deactivated_user); (function test_content_highlighter() { var fake_this = { completing: 'emoji' }; - var item = { emoji_name: 'person shrugging', emoji_url: '¯\_(ツ)_/¯' }; + var emoji = { emoji_name: 'person shrugging', emoji_url: '¯\_(ツ)_/¯' }; var th_render_typeahead_item_called = false; - typeahead_helper.render_typeahead_item = function (item) { - assert.equal(item.primary, 'person shrugging'); - assert.equal(item.img_src, '¯\_(ツ)_/¯'); + typeahead_helper.render_emoji = function (item) { + assert.deepEqual(item, emoji); th_render_typeahead_item_called = true; }; - ct.content_highlighter.call(fake_this, item); + ct.content_highlighter.call(fake_this, emoji); fake_this = { completing: 'mention' }; var th_render_person_called = false; diff --git a/frontend_tests/node_tests/markdown.js b/frontend_tests/node_tests/markdown.js index 7181ecff3a..b5f9db6930 100644 --- a/frontend_tests/node_tests/markdown.js +++ b/frontend_tests/node_tests/markdown.js @@ -229,9 +229,9 @@ var bugdown_data = JSON.parse(fs.readFileSync(path.join(__dirname, '../../zerver {input: 'mmm...:burrito:s', expected: '

mmm...:burrito:s

'}, {input: 'This is an :poop: message', - expected: '

This is an :poop: message

'}, + expected: '

This is an :poop: message

'}, {input: "\ud83d\udca9", - expected: '

:poop:

'}, + expected: '

:poop:

'}, {input: '\u{1f937}', expected: '

\u{1f937}

' }, // Test only those realm filters which don't return True for diff --git a/frontend_tests/node_tests/reactions.js b/frontend_tests/node_tests/reactions.js index bf24c59d20..2bfa3c8b55 100644 --- a/frontend_tests/node_tests/reactions.js +++ b/frontend_tests/node_tests/reactions.js @@ -38,6 +38,12 @@ set_global('page_params', {user_id: 5}); set_global('channel', {}); set_global('templates', {}); +set_global('emoji_codes', { + name_to_codepoint: { + alien: '1f47d', + smile: '1f604', + }, +}); set_global('emoji_picker', { hide_emoji_popover: function () {}, }); diff --git a/frontend_tests/node_tests/templates.js b/frontend_tests/node_tests/templates.js index c7ee2a9a41..d24df43cea 100644 --- a/frontend_tests/node_tests/templates.js +++ b/frontend_tests/node_tests/templates.js @@ -1176,6 +1176,7 @@ function render(template_name, args) { primary: 'primary-text', secondary: 'secondary-text', img_src: 'https://zulip.org', + is_emoji: true, has_image: true, has_secondary: true, }; diff --git a/frontend_tests/node_tests/typeahead_helper.js b/frontend_tests/node_tests/typeahead_helper.js index 2e403754af..1f716704a0 100644 --- a/frontend_tests/node_tests/typeahead_helper.js +++ b/frontend_tests/node_tests/typeahead_helper.js @@ -441,6 +441,57 @@ _.each(matches, function (person) { assert(rendered); }()); +(function test_render_emoji() { + // Test render_emoji with normal emoji. + var rendered = false; + var emoji = { + emoji_name: 'thumbs_up', + codepoint: '1f44d', + }; + set_global('emoji', { + active_realm_emojis: { + realm_emoji: 'TBD', + }, + }); + + global.templates.render = function (template_name, args) { + assert.equal(template_name, 'typeahead_list_item'); + assert.deepEqual(args, { + primary: 'thumbs up', + codepoint: '1f44d', + is_emoji: true, + has_image: false, + has_secondary: false, + }); + rendered = true; + return 'typeahead-item-stub'; + }; + assert.equal(th.render_emoji(emoji), 'typeahead-item-stub'); + assert(rendered); + + // Test render_emoji with normal emoji. + rendered = false; + emoji = { + emoji_name: 'realm_emoji', + emoji_url: 'TBD', + }; + + global.templates.render = function (template_name, args) { + assert.equal(template_name, 'typeahead_list_item'); + assert.deepEqual(args, { + primary: 'realm emoji', + img_src: 'TBD', + is_emoji: true, + has_image: true, + has_secondary: false, + }); + rendered = true; + return 'typeahead-item-stub'; + }; + assert.equal(th.render_emoji(emoji), 'typeahead-item-stub'); + assert(rendered); +}()); + (function test_sort_emojis() { var emoji_list = [ { emoji_name: '+1' }, diff --git a/static/js/composebox_typeahead.js b/static/js/composebox_typeahead.js index 7be6d8e1cc..05e8337b51 100644 --- a/static/js/composebox_typeahead.js +++ b/static/js/composebox_typeahead.js @@ -314,10 +314,7 @@ exports.compose_content_begins_typeahead = function (query) { exports.content_highlighter = function (item) { if (this.completing === 'emoji') { - return typeahead_helper.render_typeahead_item({ - primary: item.emoji_name.split("_").join(" "), - img_src: item.emoji_url, - }); + return typeahead_helper.render_emoji(item); } else if (this.completing === 'mention') { return typeahead_helper.render_person(item); } else if (this.completing === 'stream') { diff --git a/static/js/emoji.js b/static/js/emoji.js index d77ce76d60..fdfeede0bf 100644 --- a/static/js/emoji.js +++ b/static/js/emoji.js @@ -71,8 +71,7 @@ exports.initialize = function initialize() { _.each(emoji_codes.names, function (value) { var base_name = emoji_codes.name_to_codepoint[value]; default_emojis.push({emoji_name: value, - codepoint: emoji_codes.name_to_codepoint[value], - emoji_url: "/static/generated/emoji/images/emoji/unicode/" + base_name + ".png"}); + codepoint: emoji_codes.name_to_codepoint[value]}); if (exports.default_emoji_aliases.hasOwnProperty(base_name)) { exports.default_emoji_aliases[base_name].push(value); @@ -82,8 +81,7 @@ exports.initialize = function initialize() { }); _.each(emoji_codes.codepoints, function (value) { default_unicode_emojis.push({emoji_name: value, - codepoint: value, - emoji_url: "/static/generated/emoji/images/emoji/unicode/" + value + ".png"}); + codepoint: value}); }); exports.update_emojis(page_params.realm_emoji); diff --git a/static/js/markdown.js b/static/js/markdown.js index 8207f09217..9da7e84dfc 100644 --- a/static/js/markdown.js +++ b/static/js/markdown.js @@ -117,35 +117,33 @@ function escape(html, encode) { } function handleUnicodeEmoji(unicode_emoji) { - var hex_value = unicode_emoji.codePointAt(0).toString(16); - if (emoji.emojis_by_unicode.hasOwnProperty(hex_value)) { - var emoji_url = emoji.emojis_by_unicode[hex_value]; - var emoji_name = emoji_codes.codepoint_to_name[hex_value]; + var codepoint = unicode_emoji.codePointAt(0).toString(16); + if (emoji_codes.codepoint_to_name.hasOwnProperty(codepoint)) { + var emoji_name = emoji_codes.codepoint_to_name[codepoint]; var alt_text = ':' + emoji_name + ':'; var title = emoji_name.split("_").join(" "); - return '' + alt_text + ''; + return '' + alt_text + + ''; } return unicode_emoji; } function handleEmoji(emoji_name) { - var input_emoji = ':' + emoji_name + ':'; + var alt_text = ':' + emoji_name + ':'; var title = emoji_name.split("_").join(" "); - var emoji_url; if (emoji.active_realm_emojis.hasOwnProperty(emoji_name)) { - emoji_url = emoji.active_realm_emojis[emoji_name].emoji_url; - return '' + input_emoji + ''; - } else if (emoji.emojis_by_name.hasOwnProperty(emoji_name)) { - emoji_url = emoji.emojis_by_name[emoji_name]; - return '' + input_emoji + ''; + } else if (emoji_codes.name_to_codepoint.hasOwnProperty(emoji_name)) { + var codepoint = emoji_codes.name_to_codepoint[emoji_name]; + return '' + alt_text + + ''; } - return input_emoji; + return alt_text; } function handleAvatar(email) { diff --git a/static/js/reactions.js b/static/js/reactions.js index af880c1c3c..7586f5b966 100644 --- a/static/js/reactions.js +++ b/static/js/reactions.js @@ -14,7 +14,7 @@ exports.open_reactions_popover = function () { }; function send_reaction_ajax(message_id, emoji_name, operation) { - if (!emoji.emojis_by_name[emoji_name] && !emoji.active_realm_emojis[emoji_name]) { + if (!emoji_codes.name_to_codepoint[emoji_name] && !emoji.active_realm_emojis[emoji_name]) { // Emoji doesn't exist return; } diff --git a/static/js/typeahead_helper.js b/static/js/typeahead_helper.js index 238b414039..73a15f2269 100644 --- a/static/js/typeahead_helper.js +++ b/static/js/typeahead_helper.js @@ -114,6 +114,19 @@ exports.render_stream = function (stream) { return html; }; +exports.render_emoji = function (item) { + var args = { + is_emoji: true, + primary: item.emoji_name.split("_").join(" "), + }; + if (emoji.active_realm_emojis.hasOwnProperty(item.emoji_name)) { + args.img_src = item.emoji_url; + } else { + args.codepoint = item.codepoint; + } + return exports.render_typeahead_item(args); +}; + exports.sorter = function (query, objs, get_item) { var results = util.prefix_sort(query, objs, get_item); return results.matches.concat(results.rest); diff --git a/static/js/ui.js b/static/js/ui.js index 5aac273b4e..377d861b9b 100644 --- a/static/js/ui.js +++ b/static/js/ui.js @@ -14,7 +14,7 @@ exports.actively_scrolling = function () { exports.replace_emoji_with_text = function (element) { element.find(".emoji").replaceWith(function () { - return $(this).attr("alt"); + return $(this).text(); }); }; diff --git a/static/templates/typeahead_list_item.handlebars b/static/templates/typeahead_list_item.handlebars index 92ea984fb7..139a0f54c9 100644 --- a/static/templates/typeahead_list_item.handlebars +++ b/static/templates/typeahead_list_item.handlebars @@ -1,7 +1,11 @@ -{{#if has_image}} - -   -{{/if~}} +{{#if is_emoji}} + {{#if has_image}} + + {{else}} + + {{/if}} +    +{{/if}} {{~ primary ~}} diff --git a/tools/setup/emoji/build_emoji b/tools/setup/emoji/build_emoji index 5eaac7dd01..3061188f90 100755 --- a/tools/setup/emoji/build_emoji +++ b/tools/setup/emoji/build_emoji @@ -216,12 +216,13 @@ def generate_map_files(cache_path, emoji_map, emoji_data, emoji_catalog, unified if name in emoji_map: patched_css_classes[str(name)] = str(emoji['unified'].lower()) + name_to_codepoint = {name: unified_reactions_data[name] for name in names} codepoint_to_name = generate_codepoint_to_name_map(names, unified_reactions_data) emoji_codes_file.write(EMOJI_CODES_FILE_TEMPLATE % { 'names': names, 'codepoints': sorted([str(code_point) for code_point in set(emoji_map.values())]), - 'name_to_codepoint': {str(key): str(emoji_map[key]) for key in emoji_map}, + 'name_to_codepoint': name_to_codepoint, 'codepoint_to_name': codepoint_to_name, 'emoji_catalog': emoji_catalog, 'patched_css_classes': patched_css_classes @@ -231,7 +232,7 @@ def generate_map_files(cache_path, emoji_map, emoji_data, emoji_catalog, unified NAME_TO_CODEPOINT_PATH = os.path.join(cache_path, 'name_to_codepoint.json') name_to_codepoint_file = open(NAME_TO_CODEPOINT_PATH, 'w') - name_to_codepoint_file.write(ujson.dumps(emoji_map)) + name_to_codepoint_file.write(ujson.dumps(name_to_codepoint)) name_to_codepoint_file.close() CODEPOINT_TO_NAME_PATH = os.path.join(cache_path, 'codepoint_to_name.json') diff --git a/zerver/fixtures/markdown_test_cases.json b/zerver/fixtures/markdown_test_cases.json index b442e493bf..f3d950bad7 100644 --- a/zerver/fixtures/markdown_test_cases.json +++ b/zerver/fixtures/markdown_test_cases.json @@ -243,13 +243,13 @@ { "name": "many_emoji", "input": "test :smile: again :poop:\n:) foo:)bar x::y::z :wasted waste: :fakeemojithisshouldnotrender:", - "expected_output": "

test \":smile:\" again \":poop:\"
\n:) foo:)bar x::y::z :wasted waste: :fakeemojithisshouldnotrender:

", + "expected_output": "

test :smile: again :poop:
\n:) foo:)bar x::y::z :wasted waste: :fakeemojithisshouldnotrender:

", "bugdown_matches_marked": true }, { "name": "random_emoji_1", "input": ":airplane:", - "expected_output": "

\":airplane:\"

", + "expected_output": "

:airplane:

", "bugdown_matches_marked": true }, { @@ -261,19 +261,19 @@ { "name": "random_emoji_2", "input": ":poop:", - "expected_output": "

\":poop:\"

", + "expected_output": "

:poop:

", "bugdown_matches_marked": true }, { "name": "emojis_without_space", "input": ":cat:hello:dog::rabbit:", - "expected_output": "

\":cat:\"hello\":dog:\"\":rabbit:\"

", + "expected_output": "

:cat:hello:dog::rabbit:

", "bugdown_matches_marked": true }, { "name": "emojis_newline", "input": ":cat:\n:dog:", - "expected_output": "

\":cat:\"
\n\":dog:\"

", + "expected_output": "

:cat:
\n:dog:

", "bugdown_matches_marked": true }, { @@ -285,61 +285,61 @@ { "name": "unicode_emoji", "input": "\ud83d\udca9", - "expected_output":"

\":poop:\"<\/p>", + "expected_output":"

:poop:

", "bugdown_matches_marked": true }, { "name": "two_unicode_emoji", "input": "\ud83d\udca9\ud83d\udca9", - "expected_output":"

\":poop:\"\":poop:\"<\/p>", + "expected_output":"

:poop::poop:<\/p>", "bugdown_matches_marked": true }, { "name": "two_unicode_emoji_separated_by_text", "input": "\ud83d\udca9 word \ud83d\udca9", - "expected_output":"

\":poop:\" word \":poop:\"<\/p>", + "expected_output":"

:poop: word :poop:<\/p>", "bugdown_matches_marked": true }, { "name": "miscellaneous_symbols_and_pictographs", "input": "Merry Christmas!!\ud83c\udf84", - "expected_output":"

Merry Christmas!!\":christmas_tree:\"<\/p>", + "expected_output":"

Merry Christmas!!:christmas_tree:<\/p>", "bugdown_matches_marked": true }, { "name": "miscellaneous_and_dingbats_emoji", "input": "\u2693\u2797", - "expected_output":"

\":anchor:\"\":heavy_division_sign:\"<\/p>", + "expected_output":"

:anchor::heavy_division_sign:<\/p>", "bugdown_matches_marked": true }, { "name": "supplemental_symbols_and_pictographs", "input": "I am a robot \ud83e\udd16.", - "expected_output":"

I am a robot \":robot_face:\".<\/p>", + "expected_output":"

I am a robot :robot_face:.<\/p>", "bugdown_matches_marked": true }, { "name": "miscellaneous_symbols_and_arrows", "input": "Black upward arrow \u2b06", - "expected_output":"

Black upward arrow \":arrow_up:\"<\/p>", + "expected_output":"

Black upward arrow :arrow_up:<\/p>", "bugdown_matches_marked": true }, { "name": "unicode_emoji_without_space", "input": "Extra\ud83d\udc7dTerrestrial", - "expected_output":"

Extra\":alien:\"Terrestrial<\/p>", + "expected_output":"

Extra:alien:Terrestrial<\/p>", "bugdown_matches_marked": true }, { "name": "unicode_emojis_new_line", "input": "\ud83d\udc7d\n\ud83d\udc7d", - "expected_output":"

\":alien:\"
\n\":alien:\"<\/p>", + "expected_output":"

:alien:
\n:alien:

", "bugdown_matches_marked": true }, { "name": "emoji_alongside_punctuation", "input": ":smile:, :smile:; :smile:", - "expected_output": "

\":smile:\", \":smile:\"; \":smile:\"

", + "expected_output": "

:smile:, :smile:; :smile:

", "bugdown_matches_marked": true }, { diff --git a/zerver/lib/bugdown/__init__.py b/zerver/lib/bugdown/__init__.py index 10f4056004..5b0dd83737 100644 --- a/zerver/lib/bugdown/__init__.py +++ b/zerver/lib/bugdown/__init__.py @@ -802,16 +802,13 @@ unicode_emoji_regex = u'(?P['\ def make_emoji(codepoint, display_string): # type: (Text, Text) -> Element - src = '/static/generated/emoji/images/emoji/unicode/%s.png' % (codepoint,) # Replace underscore in emoji's title with space title = display_string[1:-1].replace("_", " ") - - elt = markdown.util.etree.Element('img') - elt.set('src', src) - elt.set('class', 'emoji') - elt.set("alt", display_string) - elt.set("title", title) - return elt + span = markdown.util.etree.Element('span') + span.set('class', 'emoji emoji-%s' % (codepoint,)) + span.set('title', title) + span.text = display_string + return span def make_realm_emoji(src, display_string): # type: (Text, Text) -> Element diff --git a/zerver/lib/notifications.py b/zerver/lib/notifications.py index bec150f8af..e336d7ab19 100644 --- a/zerver/lib/notifications.py +++ b/zerver/lib/notifications.py @@ -89,11 +89,10 @@ def relative_to_full_url(base_url, content): content = re.sub( r"", "", content) - # URLs for emoji are of the form - # "static/generated/emoji/images/emoji/snowflake.png". + # Convert the zulip emoji's relative url to absolute one. content = re.sub( - r"(?<=\=['\"])/static/generated/emoji/images/emoji/(?=[^<]+>)", - base_url + r"/static/generated/emoji/images/emoji/", + r"(?<=\=['\"])/static/generated/emoji/images/emoji/unicode/zulip.png(?=[^<]+>)", + base_url + r"/static/generated/emoji/images/emoji/unicode/zulip.png", content) # Realm emoji should use absolute URLs when referenced in missed-message emails. @@ -110,6 +109,18 @@ def relative_to_full_url(base_url, content): return content +def fix_emojis(content, base_url): + # type: (Text, Text) -> Text + # Convert the emoji spans to img tags. + content = re.sub( + r'(\S+)', + r'\3', + content) + content = content.replace(' class="emoji"', ' style="height: 20px;"') + + return content + def build_message_list(user_profile, messages): # type: (UserProfile, List[Message]) -> List[Dict[str, Any]] """ @@ -133,10 +144,6 @@ def build_message_list(user_profile, messages): # with a simple hyperlink. return re.sub(r"\[(\S*)\]\((\S*)\)", r"\2", content) - def fix_emojis(html): - # type: (Text) -> Text - return html.replace(' class="emoji"', ' height="20px" style="position: relative;top: 6px;"') - def build_message_payload(message): # type: (Message) -> Dict[str, Text] plain = message.content @@ -153,7 +160,7 @@ def build_message_list(user_profile, messages): assert message.rendered_content is not None html = message.rendered_content html = relative_to_full_url(user_profile.realm.uri, html) - html = fix_emojis(html) + html = fix_emojis(html, user_profile.realm.uri) return {'plain': plain, 'html': html} diff --git a/zerver/tests/test_bugdown.py b/zerver/tests/test_bugdown.py index e8fc5fc025..a76f22517f 100644 --- a/zerver/tests/test_bugdown.py +++ b/zerver/tests/test_bugdown.py @@ -392,7 +392,7 @@ class BugdownTest(ZulipTestCase): media_tweet_html = ('' 'http://twitter.com/NEVNBoston/status/421654515616849920/photo/1') - emoji_in_tweet_html = """Zulip is :hundred_points:% open-source!""" + emoji_in_tweet_html = """Zulip is :hundred_points:% open-source!""" def make_inline_twitter_preview(url, tweet_html, image_html=''): # type: (Text, Text, Text) -> Text @@ -541,11 +541,11 @@ class BugdownTest(ZulipTestCase): # type: () -> None msg = u'\u2615' # ☕ converted = bugdown_convert(msg) - self.assertEqual(converted, u'

:coffee:

') + self.assertEqual(converted, u'

:coffee:

') msg = u'\u2615\u2615' # ☕☕ converted = bugdown_convert(msg) - self.assertEqual(converted, u'

:coffee::coffee:

') + self.assertEqual(converted, u'

:coffee::coffee:

') def test_same_markup(self): # type: () -> None diff --git a/zerver/tests/test_notifications.py b/zerver/tests/test_notifications.py index f9bd33e705..42c35e1adc 100644 --- a/zerver/tests/test_notifications.py +++ b/zerver/tests/test_notifications.py @@ -13,8 +13,8 @@ from mock import patch, MagicMock from six.moves import range from typing import Any, Dict, List, Text -from zerver.lib.notifications import handle_missedmessage_emails, \ - relative_to_full_url +from zerver.lib.notifications import fix_emojis, \ + handle_missedmessage_emails, relative_to_full_url from zerver.lib.actions import do_update_message from zerver.lib.message import access_message from zerver.lib.test_classes import ZulipTestCase @@ -338,7 +338,7 @@ class TestMissedMessages(ZulipTestCase): msg_id = self.send_message(self.example_email('othello'), self.example_email('hamlet'), Recipient.PERSONAL, 'Extremely personal message with a realm emoji :green_tick:!') - body = ':green_tick:' + body = ':green_tick:' subject = 'Othello, the Moor of Venice sent you a message' self._test_cases(tokens, msg_id, body, subject, send_as_user=False, verify_html_body=True) @@ -403,12 +403,6 @@ class TestMissedMessages(ZulipTestCase): # Specific test cases. - # A path to an emoji image - test_data = 'emoji' - actual_output = relative_to_full_url("http://example.com", test_data) - expected_output = 'emoji' - self.assertEqual(actual_output, expected_output) - # A path similar to our emoji path, but not in a link: test_data = "

Check out the file at: '/static/generated/emoji/images/emoji/'

" actual_output = relative_to_full_url("http://example.com", test_data) @@ -427,3 +421,14 @@ class TestMissedMessages(ZulipTestCase): actual_output = relative_to_full_url("http://example.com", test_data) expected_output = '

Set src="/avatar/username@example.com?s=30"

' self.assertEqual(actual_output, expected_output) + + def test_fix_emoji(self): + # type: () -> None + # An emoji. + test_data = '

See ' + \ + ':cloud_with_lightning_and_rain:.

' + actual_output = fix_emojis(test_data, "http://example.com") + expected_output = '

See :cloud_with_lightning_and_rain:.

' + self.assertEqual(actual_output, expected_output)