rails: ActionText does not support Trix content attachments
Steps to reproduce
- Create a model that
has_rich_textand a form for it with arich_text_area - 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)
- Observe that the Trix editor displays the embedded attachment, and that the hidden form input’s value contains the HTML for the attachment.
- 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)
Links to this issue
Commits related to this issue
- failed attempt to be able to embed github gists in post using action text. Need to wait until they implement support for static html attachments. See https://github.com/rails/rails/issues/35218 — committed to ajessee/BigDumbWebDev by ajessee 5 years ago
- failed attempt to be able to embed github gists in post using action text. Need to wait until they implement support for static html attachments. See https://github.com/rails/rails/issues/35218 — committed to ajessee/BigDumbWebDev by ajessee 5 years ago
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
sgidcomes from the ActionText::Attachable module’sattachable_sgidmethod.You have to set up something like this:
This gives your videos
video.attachable_sgidwhich you’ll have to send over to the client for inserting with yourTrix.Attachment.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.
<hr>tagsThe
<hr>tag with ActionText is already working.Trix.Attachment. It needs to have thecontentTypeset tovnd.rubyonrails.horizontal-rule.htmlHere’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.The second partial gets rendered when displaying the rich content.
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::ContentAttachmentit’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 buildHTMLAttachmentslikeContentAttachmentbut 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?!?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]
sgid[1]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
iframeto the whitelisted tags might be a security risk. I suppose you might also want to customize theallowed_attributesfield of theWhitelistSanitizer.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_paththat it used so you could override it separate from theto_partial_paththat you might be using for other things. Right now you would have to override theContentHelpermethod to change therendercall.ActionText renders
to_trix_content_attachment_partial_pathfor editing in Trix andto_partial_pathwhen rendering in the ContentHelper @healerjoy. You can override either method to change how those work.Originally,
to_trix_content_attachment_partial_pathwas undefined and you had to implement it yourself, but my PR was merged to default that toto_partial_pathso 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’srender_action_text_attachmentsmethod you can instead check forlocal_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.
@excid3 Ah! This is great insight. Thanks for your reply. Now I understand that the true purpose of
to_trix_content_attachment_partial_pathmethod is to render only when editing with Trix Editor.Use Case I use a model called
Personwhich renders the default partialperson/_person.html.erbusing the defaultto_partial_pathmethod. This partial is already being used to render data of the person.When I use
ActionText::Attachableon this model and override theto_trix_content_attachment_partial_pathmethod 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_pathmethod. As it can be noticed, it is not possible to overrideto_partial_pathin 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
Personmodel without the need to have a separateMentionmodel.Do you feel this is a bug/needed-improvement to allow for a custom partial path? Or the other understanding can be that
ActionText::Attachableshould only be used on a separate model that will work only as attachable. Currently, the separate model approach works properly because theto_partial_pathof the separate model can be used as needed.Looking forward to the Railsconf Couch Edition talk.