trix icon indicating copy to clipboard operation
trix copied to clipboard

ID attributes are stripped after saving an edit

Open walterdavis opened this issue 5 years ago • 17 comments

Starting from imported HTML that contains footnotes (anchor links within the document), any attempt to edit the document causes the IDs to be stripped from the page elements that have them. This renders the anchor links (which are not altered in the process) useless.

Because the anchor links from imported Word documents follow a predictable naming scheme, I was able to use JavaScript in the browser to restore their function. But this is not ideal, because the pattern could change in the future, and render my hack obsolete. Trix seems to also treat classnames the same way, stripping them off when it finds them.

Steps to Reproduce
  1. Import valid HTML text into Trix's ActiveRecord column (do not use the front-end editor for this step).
  2. Open that record for editing using the Trix editor, and make any change, to any part of the content.
  3. Save.
  4. Notice that the anchors no longer work, because their targets have lost their IDs.
Details
  • Trix version: 1.2.3
  • Browser name and version: Safari/Firefox/Chrome (evergreen, latest versions)
  • Operating system: Mac OS 10.14.6

walterdavis avatar Apr 08 '20 15:04 walterdavis

This issue has been automatically marked as stale after 90 days of inactivity. It will be closed if no further activity occurs.

stale[bot] avatar Jul 10 '20 01:07 stale[bot]

Bump! This is keeping me from using Trix at all in production. IDs are a critical part of the content.

walterdavis avatar Jul 10 '20 13:07 walterdavis

This issue has been automatically marked as stale after 90 days of inactivity. It will be closed if no further activity occurs.

stale[bot] avatar Oct 10 '20 12:10 stale[bot]

Is there any reason why these properties are being removed from imported HTML? Is this a considered design decision, or just something that you didn't plan out?

walterdavis avatar Oct 10 '20 14:10 walterdavis

This issue has been automatically marked as stale after 90 days of inactivity. It will be closed if no further activity occurs.

stale[bot] avatar Mar 11 '21 14:03 stale[bot]

Is there any reason why these properties are being removed from imported HTML? Is this a considered design decision, or just something that you didn't plan out?

walterdavis avatar Mar 11 '21 14:03 walterdavis

This issue has been automatically marked as stale after 90 days of inactivity. It will be closed if no further activity occurs.

stale[bot] avatar Jun 11 '21 01:06 stale[bot]

Sigh: bumping this again...

Is there any reason why these properties are being removed from imported HTML? Is this a considered design decision, or just something that you didn't plan out?

walterdavis avatar Jun 11 '21 13:06 walterdavis

I also had this issue. I was following a GoRails guide to add user mentions in Actiontext. My _user.html.erb was rendering out the username as a link for the mention in a comment. That id was not being saved after the comment was saved (showed when first rendered but disappears after refresh). I am having the same issue with data turbo false so that links don't cause the comment to disappear with turbo frames.

<span class="text-sapphire bg-gray-200" id="test by jay">
  <% if user.username.present? %>
    <%= link_to user, class: "link", data: { turbo: false } do %>
      <%= "@" + user.username %>
    <% end %>
  <% else %>
    <%= "@" + user.email %>
  <% end %>
</span>

This issue resolved the id disappearing. https://github.com/rails/rails/issues/36725#issuecomment-701332182

It required an initializer on action_text.rb that allows the id to be rendered. So the following in ./config/initializers/action_text.rb fixed the id issue. This might help for you @walterdavis 🤷‍♂️

Rails.application.config.after_initialize do
    ActionText::ContentHelper.allowed_attributes.add 'id'
end

Hunch, but this might also be affected by a csp that rejects inline script tags?

jaykilleen avatar Jun 24 '22 01:06 jaykilleen

Ah cool. I can confirm that adding the below to the Action Text initializer file allowed the , data: { turbo: false } part of my path for to be rendered. Might also be worth adding things like target for the target _top approach to link click refreshing.

  ActionText::ContentHelper.allowed_attributes.add 'data-turbo'

jaykilleen avatar Jun 24 '22 01:06 jaykilleen

For anyone else here trying to solve the Trix links on turbo frames causing content to disappear. It is not the same as this issue, but the problems likely come hand-in-hand.

So, if you go to your "./app/views/layouts/actiontext/content/_content.html.erb" file and wrap your trix content yield div like below, then it will disable turbo links on all actiontext link content (which for me, fixed links entered in by users causing the action text body to just disappear).

<div class="trix-content" data-turbo="false">
  <%= yield -%>
</div>

jaykilleen avatar Jun 24 '22 01:06 jaykilleen

Oh wow, @jaykilleen yes! Thanks! I wonder what else is available in this interface? I have a bunch of things that clients have asked for over the years that I have struggled to implement using AT.

walterdavis avatar Jun 24 '22 12:06 walterdavis

[deleted]

marckohlbrugge avatar Jul 02 '22 19:07 marckohlbrugge

Thank you, @jaykilleen!

I found that another way to resolve the Turbo issue is to intercept the ActionText Content and modify the HTML output before it's rendered. GoRails has a new (paid) video walking through the process for adding a rel attribute to links, but the same process works for "data-turbo" or anything else. The important bit is in the ("action_text.rb" initialiser over here).

This was useful for me because the project I'm working on isn't currently setup to have access to customisable ActionText HTML layout files, and there's a few things I wanted do to all anchor elements in addition to the Turbo fix, like change the target to _blank (which I was previously doing via JavaScript after rendering).

Also, quick note in case it's useful to anyone, as well as allowed_attributes there's a related allowed_tags which you can use to stop the santizer stripping out certain tags from custom ActionText attachments, if you have a need. For example, if you wanted to let users embed custom iframes:

ActionText::ContentHelper.allowed_tags << "iframe"

Spydarlee avatar Jul 27 '22 09:07 Spydarlee

still broken ..

makowskid avatar Dec 04 '23 13:12 makowskid

Setting up a new project with rails 7.1.2 and encountering this issue again (in a way).

Seeing this error when trying to restart the server after copying my initializer file from the last project (as discussed in this issue). If I find a solution I'll post it here as this seems to pop up using google-fu to debug this issue.

~/config/initializers/action_text.rb:2:in `block in <main>': undefined method `add' for nil (NoMethodError)
22:54:47 web.1  |   ActionText::ContentHelper.allowed_attributes.add 'id'

jaykilleen avatar Jan 17 '24 13:01 jaykilleen

Ok so I finally tracked down why this solution stopped working for rails 7.1.x. Google fu led me to this Stack Overflow question and ultimately this pull request where the initial change was made and this pull request where the fix for action_text.rb was provided. So good reading if you want to understand what has changed and why.

I ended up changing my ./config/initializers/action_text.rb to something like what @krsyoung proposed and all is working again.

# config/initializers/action_text.rb
Rails.application.config.after_initialize do
  default_allowed_attributes = Rails::HTML5::Sanitizer.safe_list_sanitizer.allowed_attributes + ActionText::Attachment::ATTRIBUTES.to_set
  custom_allowed_attributes = Set.new(%w[rel style id data-controller data-action data-hovercard-target data-hovercard-card-id-value data-hovercard-url-value])
  ActionText::ContentHelper.allowed_attributes = (default_allowed_attributes + custom_allowed_attributes).freeze

  default_allowed_tags = Rails::HTML5::Sanitizer.safe_list_sanitizer.allowed_tags + Set.new([ActionText::Attachment.tag_name, "figure", "figcaption"])
  custom_allowed_tags = Set.new(%w[id mark hr table tbody thead tr th td])
  ActionText::ContentHelper.allowed_tags = (default_allowed_tags + custom_allowed_tags).freeze
end

jaykilleen avatar Feb 14 '24 22:02 jaykilleen