From 6acad0549d3896ec3774b3c1fedb3c2ee65c0d6b Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Sat, 26 Sep 2026 09:51:05 +0200 Subject: [PATCH] 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 | 20 +++++++----- app/models/user/mentionable.rb | 6 ++++ test/helpers/rich_text_helper_test.rb | 32 +++++++++++++++++++- test/system/composer_test.rb | 28 +++++++++++++++++ test/test_helpers/rich_text_editor_helper.rb | 5 +-- 5 files changed, 81 insertions(+), 10 deletions(-) diff --git a/app/helpers/rich_text_helper.rb b/app/helpers/rich_text_helper.rb index 67c6d99..306c3db 100644 --- a/app/helpers/rich_text_helper.rb +++ b/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) diff --git a/app/models/user/mentionable.rb b/app/models/user/mentionable.rb index bb763f2..b028284 100644 --- a/app/models/user/mentionable.rb +++ b/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 diff --git a/test/helpers/rich_text_helper_test.rb b/test/helpers/rich_text_helper_test.rb index bc72d33..21bce8b 100644 --- a/test/helpers/rich_text_helper_test.rb +++ b/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 + +
Free cookies
+
+ + HTML + body = %(

) + 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 = %(
Hey
) + 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: "

Plain text

", client_message_id: "0018", creator: users(:jason) assert_equal message.body.body.to_html, editable_body(message).body.to_html diff --git a/test/system/composer_test.rb b/test/system/composer_test.rb index 315d899..764cbea 100644 --- a/test/system/composer_test.rb +++ b/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 = %(
Hey
) + 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: "
NamePoints
Jason10
" + + 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 diff --git a/test/test_helpers/rich_text_editor_helper.rb b/test/test_helpers/rich_text_editor_helper.rb index 439ab1d..f905547 100644 --- a/test/test_helpers/rich_text_editor_helper.rb +++ b/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