rails: ActionText does not support Trix content attachments

Steps to reproduce

  1. Create a model that has_rich_text and a form for it with a rich_text_area
  2. Load the form, and in the browser JS console run:
attachment = new Trix.Attachment({content: '<iframe src="https://player.vimeo.com/video/70591644?title=0&byline=0&portrait=0" width="640" height="360" frameborder="0" webkitallowfullscreen mozallowfullscreen allowfullscreen></iframe>'})
editor = document.querySelector('trix-editor').editor
editor.insertAttachment(attachment)
  1. Observe that the Trix editor displays the embedded attachment, and that the hidden form input’s value contains the HTML for the attachment.
  2. Submit the form

Expected behavior

The Trix content attachment is saved with the rest of the rich content

Actual behavior

The content attachment is dropped and not persisted

System configuration

Rails version: 6.0.0.beta1

Ruby version: 2.6.1

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 5
  • Comments: 37 (16 by maintainers)

Commits related to this issue

Most upvoted comments

This isn’t documented anywhere yet on how ActionText stores attachments, but here’s a quick explanation. I’m planning on writing up some more stuff for the docs.

ActionText requires attachments to have a signed global ID (sgid). It minifies the HTML you insert on save, so that it can re-render it with up-to-date content later on. This makes it so you can reference models and always display the current content when those records change.

The sgid comes from the ActionText::Attachable module’s attachable_sgid method.

You have to set up something like this:

class Video < ApplicationRecord
  include ActionText::Attachable
end

This gives your videos video.attachable_sgid which you’ll have to send over to the client for inserting with your Trix.Attachment.

attachment = new Trix.Attachment({
  sgid: video.attachable_sgid,
  content: '<iframe src="https://player.vimeo.com/video/70591644?title=0&byline=0&portrait=0" width="640" height="360" frameborder="0" webkitallowfullscreen mozallowfullscreen allowfullscreen></iframe>'
})

ActionText will load up the model from the global id and then render it with the default partial path when you render the content. In this case it would be videos/_video.html.erb.

Unfortunately, I don’t think there are any good ways of inserting attachments right now that aren’t backed by a GlobalID referenceable model.

If attachments must have an sgid, does this mean that the proposed solution to allow HR tags in trix can’t work with actiontext? basecamp/trix#570 (inserting as a content attachment)?

Horizontal rules may be a bit of an odd case, but it would be really nice to be able to support them.

<hr> tags

The <hr> tag with ActionText is already working.

  1. We need to insert a Trix.Attachment. It needs to have the contentType set to vnd.rubyonrails.horizontal-rule.html
const trixEditorTag = document.querySelector("trix-editor");
const attachment = new Trix.Attachment({
    content: "<hr>",
    contentType: "vnd.rubyonrails.horizontal-rule.html"
  })
  trixEditorTag.editor.insertAttachment(attachment);

Here’s the reason why: https://github.com/rails/rails/blob/67705c8bbfa6f6d0e3be1208741a947143111786/actiontext/lib/action_text/attachables/content_attachment.rb#L8-L18 2. We need to define two partials. The first is to render the <hr> tag in <trix-editor> when loading the content from the database.

views/action_text/attachables/content_attachments/_horizontal_rule.html.erb
<hr>

The second partial gets rendered when displaying the rich content.

views/action_text/attachables/_content_attachment.html.erb
<hr>

Is this something we should write documentation for? (#34978) It could also make sense to update Trix to include a <hr> toolbar-button and add these partials to ActionText so it’s possible to use all features of ActionText? I’d happily help to improve the documentation or code 😃

Custom content Attachments

Since this solution is based on the already implemented Attachables::ContentAttachment it’s possible to use <hr> tags without a database model. As far as I understand to solve this issue we would need a way to build HTMLAttachments like ContentAttachment but configurable by the developer? Is this something ActionText tries to solve, or is this out of scope and too narrow of a use case?

Um. So all my attachments will break when we rotate secret_key_base ?!?

Um. So all my attachments will break when we rotate secret_key_base ?!?

Does anyone have any thoughts or feelings about this? Feels like a potential flaw in this approach?

Just to summarize what’s been said here so far

Currently ActionText supports 2 (3ish) kinds of attachments [0]

  • attachments that are linked to a sgid [1]
  • image attachments [2]

Missing is support for static HTML attachments. Currently the implementation for those is half-finished [3] and only supports horizontal rules [4].

All three types are expected to render out of partials, so without refactoring support for static Trix attachments probably instead means support for attaching static partials.

I think a naive solution could simply add a new content-type to ContentAttachment that specifies the attachment as static and then renders the matching name from action_text/attachables/content_attachments/#{name.underscore}

@micahbf Are you sure, the attachment isn’t persisted to the DB? I’ve run into a similar issue and tracked it down to the HTML sanitizer removing non-whitelisted tags.

I’ve added the missing tag to the whitelist using the following initializer. Bear in mind that adding iframe to the whitelisted tags might be a security risk. I suppose you might also want to customize the allowed_attributes field of the WhitelistSanitizer.

# config/initializers/html_sanitation.rb

Rails::Html::WhiteListSanitizer.allowed_tags << "tagname"

See: https://github.com/rails/rails-html-sanitizer/blob/f82bfd25c02f806e51b5e2ae62334bba4e908ad8/lib/rails/html/sanitizer.rb#L107

@matthee hah! Yes, fixed my code example.

@marckohlbrugge thanks for this useful information. Will keep this in mind when revisiting that part of code.

For anyone’s reference, when I was working on this problem, I proceeded with the approach of using a separate database-less ruby model to support my use case.

I think it would be nice if there was a separate to_content_partial_path that it used so you could override it separate from the to_partial_path that you might be using for other things. Right now you would have to override the ContentHelper method to change the render call.

ActionText renders to_trix_content_attachment_partial_path for editing in Trix and to_partial_path when rendering in the ContentHelper @healerjoy. You can override either method to change how those work.

Originally, to_trix_content_attachment_partial_path was undefined and you had to implement it yourself, but my PR was merged to default that to to_partial_path so it would be the same in editing and rendering.

Sometimes you need these to different partials, like if you’re embedding videos, you’ll want a thumbnail in Trix, but the actual iframe in the rendered version.

My Railsconf Couch Edition talk covers about all this in-depth with examples on YouTube embeds and user mentions, so be on the lookout for that!

@healerjoy @excid3 If you don’t want to override ContentHelper’s render_action_text_attachments method you can instead check for local_assigns.key?(:in_gallery) in your partial. It seems like this is always assigned when rendering through Action Text.

It’s definitely still a hack, but doesn’t require monkey patching and is easy to undo if Rails 6.1 does indeed add support for dedicated partials.

<% if local_assigns.key?(:in_gallery) %>
  <!-- render what an @mention in action text should look like -->
<% else %>
  <!-- your existing people/person partial code -->
<% end %>

@excid3 Ah! This is great insight. Thanks for your reply. Now I understand that the true purpose of to_trix_content_attachment_partial_path method is to render only when editing with Trix Editor.

Use Case I use a model called Person which renders the default partial person/_person.html.erb using the default to_partial_path method. This partial is already being used to render data of the person.

When I use ActionText::Attachable on this model and override the to_trix_content_attachment_partial_path method with a custom partial path, then the editing in Trix Editor works well.

However, the view content is still loaded using the ContentHelper which uses the to_partial_path method. As it can be noticed, it is not possible to override to_partial_path in this case because it is already being used for another purpose.

I am using the above method to build @mentions in Trix Editor using the Person model without the need to have a separate Mention model.

Do you feel this is a bug/needed-improvement to allow for a custom partial path? Or the other understanding can be that ActionText::Attachable should only be used on a separate model that will work only as attachable. Currently, the separate model approach works properly because the to_partial_path of the separate model can be used as needed.

Looking forward to the Railsconf Couch Edition talk.