FIX: Preserve display name in quotes when using the rich text editor (#38078)

When `display_name_on_posts` is enabled and `prioritize_username_in_ux`
is disabled, quoting a post in markdown mode correctly attributes the
quote to the user's full name (e.g. `[quote="Robin Ward, post:1,
topic:2, username:eviltrout"]`). However, the rich text editor lost the
display name during the markdown→ProseMirror→markdown round-trip,
falling back to just the username.

The ProseMirror quote node only stored `username` in its attributes.
When BBCode with a `username:` param was parsed, the display name was
discarded. On serialization, only the username survived.

This commit:
- Adds a `displayName` attribute to the ProseMirror quote node spec
- Emits `data-display-name` on the markdown-it aside token so the
ProseMirror parser can pick it up
- Updates the serializer to output the display name and `username:`
param when present
- Fixes a latent issue where `displayName` remained as an array instead
of being joined into a string when the name contains no commas

https://meta.discourse.org/t/397049/12

frontend/discourse-markdown-it/src/features/quotes.js

@@ -42,12 +42,9 @@ const rule = {
         if (split[i].startsWith("username:")) {
           // return users name by selecting all values from the first index to the post
           // this protects us from when a user has a `,` in their name
-          displayName = split.slice(0, split.indexOf(`post:${postNumber}`));
-
-          // preserve `,` in a users name if they exist
-          if (displayName.length > 1) {
-            displayName = displayName.join(", ");
-          }
+          displayName = split
+            .slice(0, split.indexOf(`post:${postNumber}`))
+            .join(", ");
 
           // strip key of 'username:' and return username
           username = split[i].slice(9);
@@ -94,6 +91,10 @@ const rule = {
       token.attrs.push(["data-username", username]);
     }
 
+    if (displayName && displayName !== username) {
+      token.attrs.push(["data-display-name", displayName]);
+    }
+
     if (postNumber) {
       token.attrs.push(["data-post", postNumber]);
     }

frontend/discourse/app/static/prosemirror/extensions/quote.js

@@ -14,6 +14,7 @@ const extension = {
       defining: true,
       attrs: {
         username: { default: null },
+        displayName: { default: null },
         postNumber: { default: null },
         topicId: { default: null },
         full: { default: null },
@@ -25,6 +26,7 @@ const extension = {
           getAttrs(dom) {
             return {
               username: dom.getAttribute("data-username"),
+              displayName: dom.getAttribute("data-display-name"),
               postNumber: dom.getAttribute("data-post"),
               topicId: dom.getAttribute("data-topic"),
               full: dom.getAttribute("data-full"),
@@ -33,17 +35,24 @@ const extension = {
         },
       ],
       toDOM(node) {
-        const { username, postNumber, topicId, full } = node.attrs;
+        const { username, displayName, postNumber, topicId, full } = node.attrs;
         const attrs = { class: "quote" };
         attrs["data-username"] = username;
         attrs["data-post"] = postNumber;
         attrs["data-topic"] = topicId;
         attrs["data-full"] = full ? "true" : "false";
+        if (displayName) {
+          attrs["data-display-name"] = displayName;
+        }
 
         const domSpec = ["aside", attrs];
 
         if (username) {
-          domSpec.push(["div", { class: "title" }, `${username}:`]);
+          domSpec.push([
+            "div",
+            { class: "title" },
+            `${displayName || username}:`,
+          ]);
         }
 
         domSpec.push(["blockquote", 0]);
@@ -67,6 +76,7 @@ const extension = {
       if (token.tag === "aside") {
         state.openNode(state.schema.nodes.quote, {
           username: token.attrGet("data-username"),
+          displayName: token.attrGet("data-display-name"),
           postNumber: token.attrGet("data-post"),
           topicId: token.attrGet("data-topic"),
           full: token.attrGet("data-full"),
@@ -94,12 +104,14 @@ const extension = {
 
   serializeNode: {
     quote(state, node) {
-      const postNumber = node.attrs.postNumber
-        ? `, post:${node.attrs.postNumber}`
-        : "";
-      const topicId = node.attrs.topicId ? `, topic:${node.attrs.topicId}` : "";
-      const quoteValue = node.attrs.username
-        ? `="${node.attrs.username}${postNumber}${topicId}"`
+      const { username, displayName, postNumber, topicId } = node.attrs;
+      const postNumberParam = postNumber ? `, post:${postNumber}` : "";
+      const topicIdParam = topicId ? `, topic:${topicId}` : "";
+      const usernameParam =
+        displayName && username ? `, username:${username}` : "";
+      const name = displayName || username;
+      const quoteValue = name
+        ? `="${name}${postNumberParam}${topicIdParam}${usernameParam}"`
         : "";
 
       state.write(`[quote${quoteValue}]\n`);

frontend/discourse/tests/integration/components/prosemirror-editor/quote-test.js

@@ -28,6 +28,16 @@ module(
         `<aside class="quote" data-username="User" data-post="123" data-topic="456" data-full="false"><div class="title">User:</div><blockquote><p>Full quote example.</p></blockquote></aside>`,
         `[quote="User, post:123, topic:456"]\nFull quote example.\n\n[/quote]\n\n`,
       ],
+      "quote with display name and username": [
+        `[quote="Full Name, post:123, topic:456, username:user123"]\nQuoted with display name.\n\n[/quote]`,
+        `<aside class="quote" data-username="user123" data-post="123" data-topic="456" data-full="false" data-display-name="Full Name"><div class="title">Full Name:</div><blockquote><p>Quoted with display name.</p></blockquote></aside>`,
+        `[quote="Full Name, post:123, topic:456, username:user123"]\nQuoted with display name.\n\n[/quote]\n\n`,
+      ],
+      "quote with display name containing comma": [
+        `[quote="Last, First, post:123, topic:456, username:user123"]\nComma name.\n\n[/quote]`,
+        `<aside class="quote" data-username="user123" data-post="123" data-topic="456" data-full="false" data-display-name="Last, First"><div class="title">Last, First:</div><blockquote><p>Comma name.</p></blockquote></aside>`,
+        `[quote="Last, First, post:123, topic:456, username:user123"]\nComma name.\n\n[/quote]\n\n`,
+      ],
     }).forEach(([name, [markdown, html, expectedMarkdown]]) => {
       test(name, async function (assert) {
         this.siteSettings.rich_editor = true;

lib/quote_rewriter.rb

@@ -62,6 +62,10 @@ class QuoteRewriter
       .each do |aside|
         next unless div = aside.at_css("div.title")
 
+        if aside["data-display-name"] == old_display_name
+          aside["data-display-name"] = new_display_name
+        end
+
         div.children.each do |child|
           if child.text?
             content = child.content

spec/jobs/change_display_name_spec.rb

@@ -42,7 +42,7 @@ RSpec.describe Jobs::ChangeDisplayName do
       it "rewrites the cooked quote display name" do
         expect { described_class.new.execute(args) }.to change { post.reload.cooked }.to(
           match_html(<<~HTML.strip),
-          <aside class="quote no-group" data-username="#{username}" data-post="1" data-topic="#{quoted_post.topic.id}">
+          <aside class="quote no-group" data-username="#{username}" data-display-name="#{new_display_name}" data-post="1" data-topic="#{quoted_post.topic.id}">
           <div class="title">
           <div class="quote-controls"></div>
           <img alt="" width="24" height="24" src="#{avatar_url}" class="avatar"> #{new_display_name}:</div>

spec/lib/pretty_text_spec.rb

@@ -31,7 +31,7 @@ RSpec.describe PrettyText do
       it "correctly extracts usernames from the new quote format" do
         topic = Fabricate(:topic, title: "this is a test topic :slight_smile:")
         expected = <<~HTML
-          <aside class="quote no-group" data-username="codinghorror" data-post="2" data-topic="#{topic.id}">
+          <aside class="quote no-group" data-username="codinghorror" data-display-name="Jeff" data-post="2" data-topic="#{topic.id}">
           <div class="title">
           <div class="quote-controls"></div>
           <a href="http://test.localhost/t/this-is-a-test-topic/#{topic.id}/2">This is a test topic <img width="20" height="20" src="/images/emoji/twitter/slight_smile.png?v=#{Emoji::EMOJI_VERSION}" title="slight_smile" loading="lazy" alt="slight_smile" class="emoji"></a></div>

spec/lib/quote_rewriter_spec.rb

@@ -116,7 +116,7 @@ RSpec.describe QuoteRewriter do
         expect(
           quote_rewriter.rewrite_cooked_display_name(doc, "Jeff", "Mr. Atwood").to_html,
         ).to match_html(<<~HTML.strip)
-          <aside class="quote no-group" data-username="codinghorror" data-post="1" data-topic="#{quoted_post.topic.id}">
+          <aside class="quote no-group" data-username="codinghorror" data-display-name="Mr. Atwood" data-post="1" data-topic="#{quoted_post.topic.id}">
           <div class="title">
           <div class="quote-controls"></div>
           <img alt="" width="24" height="24" src="#{avatar_url}" class="avatar"> Mr. Atwood:</div>

spec/services/username_changer_spec.rb

@@ -517,7 +517,7 @@ RSpec.describe UsernameChanger do
 
           expect(post.cooked).to match_html(<<~HTML)
             <p>Lorem ipsum</p>
-            <aside class="quote no-group" data-username="bar" data-post="1" data-topic="#{quoted_post.topic.id}">
+            <aside class="quote no-group" data-username="bar" data-display-name="Foo Bar" data-post="1" data-topic="#{quoted_post.topic.id}">
             <div class="title">
             <div class="quote-controls"></div>
             <img alt="" width="24" height="24" src="//test.localhost/letter_avatar_proxy/v4/letter/b/b77776/48.png" class="avatar"> Foo Bar:</div>