<br> tags within <pre><code>...</code></pre> tags don't get converted to newlines

[Corion]

When using a contentEditable element, the browser (Firefox) inserts <br> tags when the Enter key is pressed. This results in HTML like the following:

<div><pre><code>First line<br>Second line<br>Third line</code></pre></div>

This is currently converted to

First lineSecond lineThird line

Expected is

    First line
    Second line
    Third line

I've added the following test cases to the index.html test suite:

<div class="case" data-name="pre/code block with newline as &lt;br&gt; tag">
  <div class="input"><div><pre><code>First line<br>Second line<br>Third line</code></pre></div></div>

  <pre class="expected">    First line
    Second line
    Third line</pre>
</div>

<div class="case" data-name="fenced pre/code block with newline as &lt;br&gt; tag" data-options='{"codeBlockStyle": "fenced"}'>
  <div class="input"><div><pre><code>First line<br>Second line<br>Third line</code></pre></div></div>

  <pre class="expected">```
First line
Second line
Third line
```</pre>
</div>

The fix for my Perl port of turndown is at Corion/Text-HTML-Turndown@a28a2a9 , for comparison / inspiration

[pavelhoral]

The issue is pretty apparent that we are using textContent node property which simply ignores any element, including <br>. We probably should switch to innerText. Not sure if that would have any negative side effects though.

[Corion]

I have a satisfactory solution for my Perl code, I'm happy to port this to Javascript and submit a PR if that is wanted.

[pavelhoral]

Your link contains changes for parsing tables, so I am not sure what to look for.

Also I have checked if Domino supports innerText and it does not. So probably the best approach would be to pre-process the element content and replace all <br> with newlines before it gets processed by Turndown (or write custom element processor).

[Corion]

Sorry - the link was wrong - I've fixed the link and have a change. It doesn't exactly preprocess the element content, but iterates over the childNodes. I can't open a PR right now, but will so later if the code looks reasonable to you:

rules.indentedCodeBlock = {
  filter: function (node, options) {
    return (
      options.codeBlockStyle === 'indented' &&
      node.nodeName === 'PRE' &&
      node.firstChild &&
      node.firstChild.nodeName === 'CODE'
    )
  },

  replacement: function (content, node, options) {
    const nodeChildren = Array.prototype.slice.call(node.firstChild.childNodes)
    return (
      '\n\n    ' +
        nodeChildren.map((e) => {
          if (e.nodeType === 3) { return e.textContent.replace(/\n/g, '\n    ') }
          if (e.nodeType === 1 && e.nodeName === 'BR') { return '\n    ' }
          // otherwise ignore the element, could be an HTML comment
        }).join('') +
      '\n\n'
    )
  }
}

(and roughly the same for fencedCodeBlock)

[Corion]

( pull request created )

[pavelhoral]

Merged the modified PR.

[martincizek]

Turndown will be subject of a major refactoring in the near future and we'll see if this can be addressed cleanly and elegantly.

The Turndown's rule of thumb is that we focus on turndowning HTML features that are produced by rendering of the semantic markdown counterpart. And <br>s inside PRE are likely not result of any standard Markdown rendering and actually it is rather a discouraged (though valid) HTML practice.

In any case, we should definitely offer an extension point for this - which is probably not a case for a patch update.

For now, please consider addressing this where the <br> is created. An example for TinyMCE: https://github.com/taqueci/redmine_wysiwyg_editor/blob/main/assets/javascripts/RedmineFormatPlugin.js#L31

Or you can preprocess it like this:

function normalizePreInDom(nodeOrHtml) {
  const node = typeof nodeOrHtml === 'string'
    ? new DOMParser().parseFromString(nodeOrHtml, 'text/html').body
    : nodeOrHtml;

  node.querySelectorAll('pre br').forEach(br => br.replaceWith('\n'));
  node.querySelectorAll('pre').forEach(pre => pre.normalize());
  return node;
}

On the rule level, it can be addressed by cloning the node and doing the BR element -> '\n' substitution there.

P.S. I admit it is not ideal, as this is apparently not a niche case.

[Corion]

I use a bare div element with contentEditable="true" , which also produces those <br> elements. But simply normalizing before sending/parsing the HTML is also an approach, yes.

I'm looking forward to the refactoring, since I've encountered some (extended) cases, where the current approach becomes quite cumbersome to modify.