gjtorikian / html-pipeline

HTML processing filters and utilities
MIT License
2.26k stars 380 forks source link

ActionView::Template::Error with version 3.0.0 #387

Closed jeffcovey closed 6 months ago

jeffcovey commented 6 months ago

If we upgrade to html-pipeline 3.0.0, we get:

Minitest::UnexpectedError: ActionView::Template::Error: uninitialized constant ApplicationHelper::HTML

It's being thrown by HTML::Pipeline.new in our application helper:

  def markdown(text, length = nil)
    text ||= ""
    text = truncate(text, length:, escape: false, separator: " ") if length

    @pipeline ||=
      HTML::Pipeline.new(
        [HTML::Pipeline::MarkdownFilter, HTML::Pipeline::SanitizationFilter]
      )

    result = @pipeline.call(text)
    result[:output].to_s.html_safe
  end

We're running Ruby 3.2.2 and Rails 7.1.2. Any suggestions for debugging this?

gjtorikian commented 6 months ago

Hmm, is there a namespace conflict I wonder? Try prefixing the modules with ::, like ::HTML::Pipeline, to turn it into a bare namespace.

jeffcovey commented 6 months ago

That didn't help; we still get the error.

gjtorikian commented 6 months ago

Missed it the first time; the module name changed to HTMLPipeline, not HTML::Pipeline.

Please read the migration guide to avoid any other surprises: https://github.com/gjtorikian/html-pipeline/blob/443c00881d9dcca509a8bc5697f01f69c5e8cdc8/UPGRADING.md#upgrade-guide

Feel free to reopen if this didn't fix the issue (I like to keep my repos clean so close rather aggressively.)

jeffcovey commented 4 months ago

We're trying this now:

@pipeline ||= HTMLPipeline.new(
      text_filters: [], # array of instantiated (`.new`ed) `HTMLPipeline::TextFilter`
      convert_filter: HTMLPipeline::ConvertFilter::MarkdownFilter.new, # a filter that runs to turn text into HTML
      sanitization_config: {}, # an allowlist of elements/attributes/protocols to keep
      node_filters: [] # array of instantiated (`.new`ed) `HTMLPipeline::NodeFilter`
    )

And we get:

Minitest::UnexpectedError: ActionView::Template::Error: uninitialized constant HTMLPipeline::ConvertFilter::MarkdownFilter

We tried ::HTMLPipeline::ConvertFilter::MarkdownFilter.new, as you suggested, but it didn't help. We're using html-pipeline 3.0.3 and commonmarker 1.0.4. What else should we try?

gjtorikian commented 4 months ago

It’s possible you didn’t require the specific filters, eg:

require "html_pipeline"
require "html_pipeline/convert_filter/markdown_filter"
require "html_pipeline/node_filter/image_max_width_filter"
require "html_pipeline/node_filter/mention_filter"
jeffcovey commented 4 months ago

Thanks! We indeed missed that. It doesn't seem like it was necessary before; should it go in the upgrade guide?

We're now finding that we get Markdown conversion when we call HTMLPipeline::ConvertFilter::MarkdownFilter directly, but not when we include it in our HTMLPipeline call. Below is a fuller version of what we're doing, if it helps to see where we're going wrong. result[:output] is just giving us a plain string with no markup. The workaround is working, but we're not putting it into production without the sanitization.

module ApplicationHelper
  require "html_pipeline"
  require "html_pipeline/convert_filter/markdown_filter"

  def markdown(text, length: nil)
    text = truncate_text(text, length) if length
    result = pipeline.call(text)
    result[:output].to_s.html_safe

    # FIXME: remove when HTMLPipeline is working
    filter = HTMLPipeline::ConvertFilter::MarkdownFilter.new
    filter.call(text).to_s.html_safe
  end

  private

  def pipeline
    @pipeline ||=
      HTMLPipeline.new(
        convert_filter: HTMLPipeline::ConvertFilter::MarkdownFilter.new
      )
  end

  def truncate_text(text, length)
    truncate(text, length:, escape: false, separator: " ")
  end
end
gjtorikian commented 4 months ago

I don't know what it could be. I copied your test case line for line, and the results are what I would expect:

class MarkdownIntegrationTest < Minitest::Test
  def markdown(text, length: nil)
    text = truncate_text(text, length) if length
    result = pipeline.call(text)
    result[:output].to_s
  end

  def pipeline
    HTMLPipeline.new(
      convert_filter: HTMLPipeline::ConvertFilter::MarkdownFilter.new,
    )
  end

  def truncate_text(text, length)
    truncate(text, length:, escape: false, separator: " ")
  end

  def test_it_works
    assert_equal(
      "<p>hello world sanitize me</p>",
      markdown("hello world <faketag>sanitize me</faketag>"),
    )
  end
end

Can you give a more specific example with the types of strings inputted, and what's actually outputted by the pipeline?

jeffcovey commented 1 month ago

I'm sorry we never got back to you. Somewhere along the way, a recent version started working for us, so whatever the problem was, it seems to be gone. Thanks again for all your work. Take care!