markdown: Remove !avatar() and !gravatar() syntax.

This particular commit has been a long time coming. For reference,
!avatar(email) was an undocumented syntax that simply rendered an
inline 50px avatar for a user in a message, essentially allowing
you to create a user pill like:

`!avatar(alice@example.com) Alice: hey!`

---

Reimplementation

If we decide to reimplement this or a similar feature in the future,
we could use something like `<avatar:userid>` syntax which is more
in line with creating links in markdown. Even then, it would not be
a good idea to add this instead of supporting inline images directly.

Since any usecases of such a syntax are in automation, we do not need
to make it userfriendly and something like the following is a better
implementation that doesn't need a custom syntax:

`![avatar for Alice](/avatar/1234?s=50) Alice: hey!`

---

History

We initially added this syntax back in 2012 and it was 'deprecated'
from the get go. Here's what the original commit had to say about
the new syntax:

> We'll use this internally for the commit bot.  We might eventually
> disable it for external users.

We eventually did start using this for our github integrations in 2013
but since then, those integrations have been neglected in favor of
our GitHub webhooks which do not use this syntax.

When we copied `!gravatar` to add the `!avatar` syntax, we also noted
that we want to deprecate the `!gravatar` syntax entirely - in 2013!

Since then, we haven't advertised either of these syntaxes anywhere
in our docs, and the only two places where this syntax remains is
our game bots that could easily do without these, and the git commit
integration that we have deprecated anyway.

We do not have any evidence of someone asking about this syntax on
chat.zulip.org when developing an integration and rightfully so- only
the people who work on Zulip (and specifically, markdown) are likely
to stumble upon it and try it out.

This is also the only peice of code due to which we had to look up
emails -> userid mapping in our backend markdown. By removing this,
we entirely remove the backend markdown's dependency on user emails
to render messages.

---

Relevant commits:

- Oct 2012, Initial commit        c31462c278
- Nov 2013, Update commit bot     968c393826
- Nov 2013, Add avatar syntax     761c0a0266
- Sep 2017, Avoid email use       c3032a7fe8
- Apr 2019, Remove from webhook   674fcfcce1

frontend_tests/node_tests/markdown.js

@@ -214,8 +214,6 @@ run_test('markdown_detection', () => {
         "User Mention @**leo with some name**",
         "Group Mention @*hamletcharacters*",
         "Stream #**Verona**",
-        "This contains !gravatar(leo@zulip.com)",
-        "And an avatar !avatar(leo@zulip.com) is here",
     ];
 
     const markup = [
@@ -353,10 +351,6 @@ run_test('marked', () => {
          expected: '<p>A pattern written as #1234is not a realm filter.</p>'},
         {input: 'This is a realm filter with ZGROUP_123:45 groups',
          expected: '<p>This is a realm filter with <a href="https://zone_45.zulip.net/ticket/123" title="https://zone_45.zulip.net/ticket/123">ZGROUP_123:45</a> groups</p>'},
-        {input: 'This is an !avatar(cordelia@zulip.com) of Cordelia Lear',
-         expected: '<p>This is an <img alt="cordelia@zulip.com" class="message_body_gravatar" src="/avatar/cordelia@zulip.com?s=30" title="cordelia@zulip.com"> of Cordelia Lear</p>'},
-        {input: 'This is a !gravatar(cordelia@zulip.com) of Cordelia Lear',
-         expected: '<p>This is a <img alt="cordelia@zulip.com" class="message_body_gravatar" src="/avatar/cordelia@zulip.com?s=30" title="cordelia@zulip.com"> of Cordelia Lear</p>'},
         {input: 'Test *italic*',
          expected: '<p>Test <em>italic</em></p>'},
         {input: 'T\n#**Denmark**',
@@ -396,8 +390,6 @@ run_test('marked', () => {
          expected: '<p>@**&lt;h1&gt;The Rogue One&lt;/h1&gt;**</p>'},
         {input: '#**<h1>The Rogue One</h1>**',
          expected: '<p>#**&lt;h1&gt;The Rogue One&lt;/h1&gt;**</p>'},
-        {input: '!avatar(<h1>The Rogue One</h1>)',
-         expected: '<p><img alt="&lt;h1&gt;The Rogue One&lt;/h1&gt;" class="message_body_gravatar" src="/avatar/&lt;h1&gt;The Rogue One&lt;/h1&gt;?s=30" title="&lt;h1&gt;The Rogue One&lt;/h1&gt;"></p>'},
         {input: ':<h1>The Rogue One</h1>:',
          expected: '<p>:&lt;h1&gt;The Rogue One&lt;/h1&gt;:</p>'},
         {input: '@**O\'Connell**',

static/js/markdown.js

@@ -283,12 +283,6 @@ function handleEmoji(emoji_name) {
     return alt_text;
 }
 
-function handleAvatar(email) {
-    return '<img alt="' + email + '"' +
-           ' class="message_body_gravatar" src="/avatar/' + email + '?s=30"' +
-           ' title="' + email + '">';
-}
-
 function handleTimestamp(time) {
     let timeobject;
     if (isNaN(time)) {
@@ -519,7 +513,6 @@ exports.initialize = function (realm_filters, helper_config) {
         smartypants: false,
         zulip: true,
         emojiHandler: handleEmoji,
-        avatarHandler: handleAvatar,
         unicodeEmojiHandler: handleUnicodeEmoji,
         streamHandler: handleStream,
         streamTopicHandler: handleStreamTopic,

static/styles/rendered_markdown.scss

@@ -134,13 +134,6 @@
         background-color: hsla(102, 85%, 57%, 0.5);
     }
 
-    .message_body_gravatar {
-        width: 20px;
-        height: 20px;
-        margin: 2px 2px 2px 0px;
-        vertical-align: text-bottom;
-    }
-
     /* Timestamps */
     time {
         background: hsl(0, 0%, 93%);

static/third/marked/lib/marked.js

@@ -482,9 +482,7 @@ var inline = {
   usermention: noop,
   groupmention: noop,
   stream: noop,
-  avatar: noop,
   tex: noop,
-  gravatar: noop,
   timestamp: noop,
   realm_filters: [],
   text: /^[\s\S]+?(?=[\\<!\[_*`$]| {2,}\n|$)/
@@ -550,8 +548,6 @@ inline.zulip = merge({}, inline.breaks, {
   groupmention: /^@\*([^\*]+)\*/, // Match multi-word string between @* *
   stream_topic: /^#\*\*([^\*>]+)>([^\*]+)\*\*/,
   stream: /^#\*\*([^\*]+)\*\*/,
-  avatar: /^!avatar\(([^)]+)\)/,
-  gravatar: /^!gravatar\(([^)]+)\)/,
   tex: /^(\$\$([^\n_$](\\\$|[^\n$])*)\$\$(?!\$))\B/,
   timestamp: /^<time:([^>]+)>/,
   realm_filters: [],
@@ -821,17 +817,10 @@ InlineLexer.prototype.output = function(src) {
       continue;
     }
 
-    // user avatar
+    // timestamp
-    if (cap = this.rules.avatar.exec(src)) {
+    if (cap = this.rules.timestamp.exec(src)) {
       src = src.substring(cap[0].length);
-      out += this.userAvatar(cap[1]);
+      out += this.timestamp(cap[1]);
-      continue;
-    }
-
-    // user gravatar
-    if (cap = this.rules.gravatar.exec(src)) {
-      src = src.substring(cap[0].length);
-      out += this.userGravatar(cap[1]);
       continue;
     }
 
@@ -895,20 +884,6 @@ InlineLexer.prototype.tex = function (tex, fullmatch) {
   return this.options.texHandler(tex, fullmatch);
 };
 
-InlineLexer.prototype.userAvatar = function (email) {
-  email = escape(email);
-  if (typeof this.options.avatarHandler !== 'function')
-    return '!avatar(' + email + ')';
-  return this.options.avatarHandler(email);
-};
-
-InlineLexer.prototype.userGravatar = function (email) {
-  email = escape(email);
-  if (typeof this.options.avatarHandler !== 'function')
-    return '!gravatar(' + email + ')';
-  return this.options.avatarHandler(email);
-};
-
 InlineLexer.prototype.timestamp = function (time) {
   if (typeof this.options.timestampHandler !== 'function')
     return '&lt;time:' + time + '&gt;';
@@ -1537,8 +1512,6 @@ marked.defaults = {
   gfm: true,
   emoji: false,
   unicodeemoji: false,
-  avatar: false,
-  gravatar: false,
   timestamp: true,
   tables: true,
   breaks: false,

templates/zerver/api/changelog.md

@@ -10,6 +10,11 @@ below features are supported.
 
 ## Changes in Zulip 3.0
 
+**Feature level 24**
+
+* The `!avatar()` and `!gravatar()` markdown syntax, which was never
+  documented and rarely used, was removed.
+
 **Feature level 23**
 
 * `GET/PUT/POST /users/me/pointer`: Eliminated.  We eliminated

templates/zerver/api/incoming-webhooks-overview.md

@@ -109,7 +109,7 @@ below are for a webhook named `MyWebHook`.
 
 * Consider using our Zulip markup to make the output from your
   integration especially attractive or useful (e.g.  emoji, markdown
-  emphasis, @-mentions, or `!avatar(email)`).
+  emphasis or @-mentions).
 
 * Use topics effectively to ensure sequential messages about the same
   thing are threaded together; this makes for much better consumption

version.py

@@ -29,7 +29,7 @@ DESKTOP_WARNING_VERSION = "5.2.0"
 #
 # Changes should be accompanied by documentation explaining what the
 # new level means in templates/zerver/api/changelog.md.
-API_FEATURE_LEVEL = 23
+API_FEATURE_LEVEL = 24
 
 # Bump the minor PROVISION_VERSION to indicate that folks should provision
 # only when going from an old version of the code to a newer version. Bump

zerver/lib/markdown/__init__.py

@@ -103,8 +103,6 @@ version = 1
 _T = TypeVar('_T')
 ElementStringNone = Union[Element, Optional[str]]
 
-AVATAR_REGEX = r'!avatar\((?P<email>[^)]*)\)'
-GRAVATAR_REGEX = r'!gravatar\((?P<email>[^)]*)\)'
 EMOJI_REGEX = r'(?P<syntax>:[\w\-\+]+:)'
 
 def verbose_compile(pattern: str) -> Any:
@@ -1198,35 +1196,6 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor):
                     if title:
                         found_url.family.child.text = title
 
-class Avatar(markdown.inlinepatterns.Pattern):
-    def handleMatch(self, match: Match[str]) -> Optional[Element]:
-        img = Element('img')
-        email_address = match.group('email')
-        email = email_address.strip().lower()
-        profile_id = None
-
-        db_data = self.md.zulip_db_data
-        if db_data is not None:
-            user_dict = db_data['email_info'].get(email)
-            if user_dict is not None:
-                profile_id = user_dict['id']
-
-        img.set('class', 'message_body_gravatar')
-        img.set('src', f'/avatar/{profile_id or email}?s=30')
-        img.set('title', email)
-        img.set('alt', email)
-        return img
-
-def possible_avatar_emails(content: str) -> Set[str]:
-    emails = set()
-    for REGEX in [AVATAR_REGEX, GRAVATAR_REGEX]:
-        matches = re.findall(REGEX, content)
-        for email in matches:
-            if email:
-                emails.add(email)
-
-    return emails
-
 class Timestamp(markdown.inlinepatterns.Pattern):
     def handleMatch(self, match: Match[str]) -> Optional[Element]:
         time_input_string = match.group('time')
@@ -1940,10 +1909,7 @@ class Markdown(markdown.Markdown):
         reg.register(Tex(r'\B(?<!\$)\$\$(?P<body>[^\n_$](\\\$|[^$\n])*)\$\$(?!\$)\B'), 'tex', 90)
         reg.register(StreamTopicPattern(get_compiled_stream_topic_link_regex(), self), 'topic', 87)
         reg.register(StreamPattern(get_compiled_stream_link_regex(), self), 'stream', 85)
-        reg.register(Avatar(AVATAR_REGEX, self), 'avatar', 80)
         reg.register(Timestamp(r'<time:(?P<time>[^>]*?)>'), 'timestamp', 75)
-        # Note that !gravatar syntax should be deprecated long term.
-        reg.register(Avatar(GRAVATAR_REGEX, self), 'gravatar', 70)
         reg.register(UserGroupMentionPattern(mention.user_group_mentions, self), 'usergroupmention', 65)
         reg.register(LinkInlineProcessor(markdown.inlinepatterns.LINK_RE, self), 'link', 60)
         reg.register(AutoLink(get_web_link_regex(), self), 'autolink', 55)
@@ -2107,30 +2073,6 @@ _privacy_re = re.compile('\\w', flags=re.UNICODE)
 def privacy_clean_markdown(content: str) -> str:
     return repr(_privacy_re.sub('x', content))
 
-def get_email_info(realm_id: int, emails: Set[str]) -> Dict[str, FullNameInfo]:
-    if not emails:
-        return dict()
-
-    q_list = {
-        Q(email__iexact=email.strip().lower())
-        for email in emails
-    }
-
-    rows = UserProfile.objects.filter(
-        realm_id=realm_id,
-    ).filter(
-        functools.reduce(lambda a, b: a | b, q_list),
-    ).values(
-        'id',
-        'email',
-    )
-
-    dct = {
-        row['email'].strip().lower(): row
-        for row in rows
-    }
-    return dct
-
 def get_possible_mentions_info(realm_id: int, mention_texts: Set[str]) -> List[FullNameInfo]:
     if not mention_texts:
         return list()
@@ -2318,7 +2260,7 @@ def do_convert(content: str,
     if message_realm is not None:
 
         # Here we fetch the data structures needed to render
-        # mentions/avatars/stream mentions from the database, but only
+        # mentions/stream mentions from the database, but only
         # if there is syntax in the message that might use them, since
         # the fetches are somewhat expensive and these types of syntax
         # are uncommon enough that it's a useful optimization.
@@ -2326,9 +2268,6 @@ def do_convert(content: str,
         if mention_data is None:
             mention_data = MentionData(message_realm.id, content)
 
-        emails = possible_avatar_emails(content)
-        email_info = get_email_info(message_realm.id, emails)
-
         stream_names = possible_linked_stream_names(content)
         stream_name_info = get_stream_name_info(message_realm, stream_names)
 
@@ -2339,7 +2278,6 @@ def do_convert(content: str,
 
         _md_engine.zulip_db_data = {
             'realm_alert_words_automaton': realm_alert_words_automaton,
-            'email_info': email_info,
             'mention_data': mention_data,
             'active_realm_emoji': active_realm_emoji,
             'realm_uri': message_realm.uri,

zerver/tests/fixtures/markdown_test_cases.json

@@ -722,30 +722,6 @@
       "expected_output": "<p>gonna take a break for a bit--all yours if you want to play around too. what I did:</p>\n<ul>\n<li>install cmake</li>\n<li>git clone zulip desktop</li>\n<li>run <code>cmake-gui ..</code> in <code>c:\\zulip\\zulip-desktop\\msvcbuild</code></li>\n<li>hit configure/generate until it generated the msvc project (had to make a fix to some cmake files)</li>\n<li>opened vs2013 </li>\n<li>tried to build</li>\n</ul>",
       "text_content": "gonna take a break for a bit--all yours if you want to play around too. what I did:\n\ninstall cmake\ngit clone zulip desktop\nrun cmake-gui .. in c:\\zulip\\zulip-desktop\\msvcbuild\nhit configure\/generate until it generated the msvc project (had to make a fix to some cmake files)\nopened vs2013 \ntried to build\n"
     },
-    {
-      "name": "avatar",
-      "input": "!avatar(username@example.com)",
-      "expected_output": "<p><img alt=\"username@example.com\" class=\"message_body_gravatar\" src=\"/avatar/username@example.com?s=30\" title=\"username@example.com\"></p>",
-      "text_content": "username@example.com"
-    },
-    {
-      "name": "gravatar",
-      "input": "!gravatar(username@example.com)",
-      "expected_output": "<p><img alt=\"username@example.com\" class=\"message_body_gravatar\" src=\"/avatar/username@example.com?s=30\" title=\"username@example.com\"></p>",
-      "text_content": "username@example.com"
-    },
-    {
-      "name": "avatar_escaped",
-      "input": "`!avatar(username@example.com)`",
-      "expected_output": "<p><code>!avatar(username@example.com)</code></p>",
-      "text_content": "!avatar(username@example.com)"
-    },
-    {
-      "name": "gravatar_escaped",
-      "input": "`!gravatar(username@example.com)`",
-      "expected_output": "<p><code>!gravatar(username@example.com)</code></p>",
-      "text_content": "!gravatar(username@example.com)"
-    },
     {
       "name": "timestamp_backend_markdown_only",
       "input": "<time:Jun 5th 2017, 10:30PM>",

zerver/tests/test_markdown.py

@@ -30,7 +30,6 @@ from zerver.lib.markdown import (
     image_preview_enabled,
     markdown_convert,
     maybe_update_markdown_engines,
-    possible_avatar_emails,
     possible_linked_stream_names,
     topic_links,
     url_embed_preview_enabled,
@@ -2166,39 +2165,3 @@ class MarkdownErrorTests(ZulipTestCase):
 
         result = processor.run(markdown_input)
         self.assertEqual(result, expected)
-
-class MarkdownAvatarTestCase(ZulipTestCase):
-    def test_possible_avatar_emails(self) -> None:
-        content = '''
-            hello !avatar(foo@example.com) my email is ignore@ignore.com
-            !gravatar(bar@yo.tv)
-
-            smushing!avatar(hamlet@example.org) is allowed
-        '''
-        self.assertEqual(
-            possible_avatar_emails(content),
-            {'foo@example.com', 'bar@yo.tv', 'hamlet@example.org'},
-        )
-
-    def test_avatar_with_id(self) -> None:
-        sender_user_profile = self.example_user('othello')
-        message = Message(sender=sender_user_profile, sending_client=get_client("test"))
-
-        user_profile = self.example_user('hamlet')
-        msg = f'!avatar({user_profile.email})'
-        converted = markdown_convert(msg, message=message)
-        values = {'email': user_profile.email, 'id': user_profile.id}
-        self.assertEqual(
-            converted,
-            '<p><img alt="{email}" class="message_body_gravatar" src="/avatar/{id}?s=30" title="{email}"></p>'.format(**values))
-
-    def test_avatar_of_unregistered_user(self) -> None:
-        sender_user_profile = self.example_user('othello')
-        message = Message(sender=sender_user_profile, sending_client=get_client("test"))
-
-        email = 'fakeuser@example.com'
-        msg = f'!avatar({email})'
-        converted = markdown_convert(msg, message=message)
-        self.assertEqual(
-            converted,
-            '<p><img alt="{0}" class="message_body_gravatar" src="/avatar/{0}?s=30" title="{0}"></p>'.format(email))

zproject/urls.py

@@ -621,7 +621,8 @@ urls += [
     path('thumbnail', rest_dispatch,
          {'GET': ('zerver.views.thumbnail.backend_serve_thumbnail',
                   {'override_api_url_scheme'})}),
-    # Avatars have the same constraint due to `!avatar` syntax.
+    # Avatars have the same constraint because their URLs are included
+    # in API data structures used by both the mobile and web clients.
     re_path(r'^avatar/(?P<email_or_id>[\S]+)/(?P<medium>[\S]+)?$',
             rest_dispatch,
             {'GET': ('zerver.views.users.avatar',