HTML headings with line breaks -> no heading and inconsistency between preview and rendered post

rahim123 · April 8, 2023, 4:05pm

Hi there, I have some posts imported from another forum software with line breaks in headings. These are shown correctly in the post preview, but they are not rendered as headings in the final baked post. I’m seeing this issue on my forum updated yesterday and also here on Discourse Meta:

Heading test with line breaks

Normal paragraph.

Heading with no line breaks

It’s leaving the <h2> blank and is wrapping the text after that in <p> :

<h2 dir="ltr">
</h2><p dir="ltr">Heading test with line breaks</p>

I’m 99% sure that this is a recent regression, as I specifically checked headings in imported posts from the same user during the migration process, and the user and I both remember that they were rendered correctly.

sam · April 11, 2023, 1:16am

Hmm given the ltr stuff there @Osama could this be related to the CSS flipper we recently added?

Osama · April 12, 2023, 1:56am

I don’t think so because the final/cooked HTML of the post is wrong and the CSS flipper (whether the old or the new one) isn’t involved at all in the post cooking process.

^ the text inside the <p> should be inside the <h2> and there should be no <p> at all (I think).

selase · May 8, 2023, 6:15pm

This seems to happen during cooking, specifically markdown parsing.

github.com

discourse/discourse/blob/81aff58d09e46c01670e40e5a4e1471ea0cd3486/lib/pretty_text.rb#L302-L315


      
          sanitized = markdown(working_text, options)
          
          
doc = Nokogiri::HTML5.fragment(sanitized)
          
          
add_nofollow = !options[:omit_nofollow] && SiteSetting.add_rel_nofollow_to_user_content
          add_rel_attributes_to_user_content(doc, add_nofollow)
          strip_hidden_unicode_bidirectional_characters(doc)
          sanitize_hotlinked_media(doc)
          
          
add_mentions(doc, user_id: opts[:user_id]) if SiteSetting.enable_mentions
          
          
scrubber = Loofah::Scrubber.new { |node| node.remove if node.name == "script" }
          loofah_fragment = Loofah.fragment(doc.to_html)
          loofah_fragment.scrub!(scrubber).to_html

Passing a header with linebreaks to PrettyText.markdown returns with the content of header wrapped in a p tag.

And then during scrubbing, the paragraph get’s extracted out out of the header tag resulting in the following:

I suppose this happens since a paragraph in a header tag isn’t spec compliant?

I’m still reviewing the markdown parser code to figure out why the content is being wrapped in p tag in the first place.

sam · May 9, 2023, 12:02am

Oh this is very related to:

I am OK to wait a few more weeks for Loofah to release

Will shoot out a Tweet to Mike to mention this popped up.

rahim123 · May 9, 2023, 4:28am

Hi there, thanks for looking into this. HTML doesn’t care about line breaks, so technically this is correct:

<h2>

Heading test with line breaks

</h2>

It simply seems to be a problem of the Markdown parser running after/inside the HTML parser, which is generating <p> tags for the line returns.