Rebuild every attachment before editing a message

The editor keeps an attachment's content as it finds it, so a hand-written
embed with a url but no href reached the editor with its markup unvalidated,
and a mention saved under Trix's editor carried the generic octet-stream
content type the editor doesn't permit and was dropped on save. Rebuilding
each attachment from its attachable renders the hardened preview partial
and restores the mention content type.

app/helpers/rich_text_helper.rb

@@ -1,6 +1,4 @@
 module RichTextHelper
-  LEGACY_EMBED_SELECTOR = "action-text-attachment[content-type='#{ActionText::Attachment::OpengraphEmbed::OPENGRAPH_EMBED_CONTENT_TYPE}'][href]"
-
   def rich_text_data_actions
     # submitByKeyboard runs in the capture phase so it can submit on Enter
     # before the editor turns the keystroke into a newline
@@ -12,14 +10,22 @@ module RichTextHelper
       "remote-filtering": true, "empty-results": "No matches"
   end
 
-  # Trix-era opengraph embeds carry their details as node attributes, which
-  # the editor doesn't round-trip. Rendering them into the content attribute
-  # lets the editor preserve them like any embed it created itself.
+  # The editor keeps an attachment's content as it finds it, so every
+  # attachment is rebuilt from its attachable before editing: a Trix-era
+  # embed carries its details as node attributes the editor doesn't
+  # round-trip, a mention edited under Trix carries the generic content type
+  # the editor doesn't permit, and a hand-written embed carries whatever
+  # markup the author put there.
   def editable_body(message)
     fragment = ActionText::Fragment.wrap(message.body.body_before_type_cast)
 
-    transformed = fragment.replace(LEGACY_EMBED_SELECTOR) do |node|
-      node.tap { |n| n["content"] = render_action_text_attachment(ActionText::Attachment.from_node(n)) }
+    transformed = fragment.replace(ActionText::Attachment.tag_name) do |node|
+      attachment = ActionText::Attachment.from_node(node)
+
+      node.tap do |n|
+        n["content-type"] = attachment.attachable.attachable_content_type
+        n["content"] = render_action_text_attachment(attachment)
+      end
     end
 
     ActionText::RichText.new(body: transformed.to_html)

app/models/user/mentionable.rb

@@ -1,6 +1,12 @@
 module User::Mentionable
   include ActionText::Attachable
 
+  MENTION_CONTENT_TYPE = "application/vnd.campfire.mention"
+
+  def attachable_content_type
+    MENTION_CONTENT_TYPE
+  end
+
   def to_attachable_partial_path
     "users/mention"
   end

test/helpers/rich_text_helper_test.rb

@@ -15,7 +15,37 @@ class RichTextHelperTest < ActionView::TestCase
     assert_equal "https://example.com/image.png", content.at_css(".og-embed__image img")["src"]
   end
 
-  test "editable_body leaves bodies without legacy embeds unchanged" do
+  test "editable_body rebuilds a hand-written embed from its validated details" do
+    content = <<~HTML.squish
+      <actiontext-opengraph-embed data-controller="pwn" data-action="click->pwn#run">
+        <div class="og-embed"><div class="og-embed__title"><a href="/rooms/1">Free cookies</a></div>
+        <div class="og-embed__image"><img src="/rooms/1/avatar" data-action="load->pwn#run"></div></div>
+      </actiontext-opengraph-embed>
+    HTML
+    body = %(<p><action-text-attachment content-type="application/vnd.actiontext.opengraph-embed" url="https://example.com/image.png" content="#{ERB::Util.html_escape(content)}"></action-text-attachment></p>)
+    message = Message.create! room: rooms(:pets), body: body, client_message_id: "0019", creator: users(:jason)
+
+    node = editable_body(message).body.fragment.find_all("action-text-attachment").first
+    rebuilt = Nokogiri::HTML.fragment(node["content"])
+
+    assert_equal "Free cookies", rebuilt.at_css(".og-embed__title").text.strip
+    assert_no_match /rooms\/1/, node["content"]
+    assert_no_match /data-/, node["content"]
+    assert_nil rebuilt.at_css("a")
+    assert_nil rebuilt.at_css("img")
+  end
+
+  test "editable_body restores the content type of a mention edited under Trix" do
+    body = %(<div>Hey <action-text-attachment sgid="#{users(:david).attachable_sgid}" content-type="application/octet-stream"></action-text-attachment></div>)
+    message = Message.create! room: rooms(:pets), body: body, client_message_id: "0020", creator: users(:jason)
+
+    node = editable_body(message).body.fragment.find_all("action-text-attachment").first
+
+    assert_equal "application/vnd.campfire.mention", node["content-type"]
+    assert_match "David", node["content"]
+  end
+
+  test "editable_body leaves bodies without attachments unchanged" do
     message = Message.create! room: rooms(:pets), body: "<p>Plain text</p>", client_message_id: "0018", creator: users(:jason)
 
     assert_equal message.body.body.to_html, editable_body(message).body.to_html

test/system/composer_test.rb

@@ -86,6 +86,34 @@ class ComposerTest < ApplicationSystemTestCase
     assert_equal [ users(:jason) ], message.reload.mentionees
   end
 
+  test "editing a message whose mention was saved under Trix keeps the mention" do
+    body = %(<div>Hey <action-text-attachment sgid="#{users(:jason).attachable_sgid}" content-type="application/octet-stream"></action-text-attachment></div>)
+    message = Message.create! room: rooms(:designers), body: body, client_message_id: "trix-edited", creator: users(:jz)
+
+    join_room rooms(:designers)
+
+    within_message message do
+      reveal_message_actions
+      find(".message__edit-btn").click
+      assert_edit_editor_text "Jason"
+      click_on "Save changes"
+    end
+
+    assert_selector last_message_selector(".mention"), text: "Jason"
+    assert_equal [ users(:jason) ], message.reload.mentionees
+  end
+
+  test "pasting a table keeps its text" do
+    paste_in_composer "Name Points\nJason 10", html: "<table><tr><th>Name</th><th>Points</th></tr><tr><td>Jason</td><td>10</td></tr></table>"
+
+    assert_selector "#composer lexxy-editor table"
+
+    click_send_button
+
+    assert_message_text /Name Points\s*Jason 10/
+    assert_no_selector last_message_selector("table")
+  end
+
   test "replying quotes the original message with attribution" do
     within_message messages(:third) do
       reveal_message_actions

test/test_helpers/rich_text_editor_helper.rb

@@ -31,13 +31,14 @@ module RichTextEditorHelper
     composer_editor.send_keys :tab
   end
 
-  def paste_in_composer(text)
+  def paste_in_composer(text, html: nil)
     composer_editor.click
 
-    page.execute_script(<<~JS, text)
+    page.execute_script(<<~JS, text, html)
       const content = document.querySelector("#composer lexxy-editor .lexxy-editor__content")
       const event = new ClipboardEvent("paste", { bubbles: true, cancelable: true, clipboardData: new DataTransfer() })
       event.clipboardData.setData("text/plain", arguments[0])
+      if (arguments[1]) event.clipboardData.setData("text/html", arguments[1])
       content.dispatchEvent(event)
     JS
   end