Prism: <pre> line breaks get deleted when calling prism.highlightElement(code)

[SetTrend]

I set up my HTML using the following snippet:

<div>
 <pre><code class=" language-js" contenteditable="true"></code></pre>
</div>

When I call prism.highlightElement(code); from the following TypeScript snippet:

import * as prism from "prismjs";

const code: HTMLElement = document.getElementsByTagName("code")[0];

prism.highlightElement(code);

... all line breaks in the <code> element get deleted when calling prism.highlightElement(code).

The result is that all code is getting displayed in a single line then:

before

after

I believe this is not supposed to happen?

[RunDevelopment]

I assume that you wrote the highlighted code yourself (because of contenteditable="true").

The problem here is that Prism uses a literal string line break (so \n or \r\n).
But that is not what the browser will insert when you press Enter. It will insert a new <br> which Prism will ignore (just like all other HTML elements).

The best solution, in that case, is to use keyboard events to handle Enter yourself.
A working example of this is dabblet. The editor code can be found here.

I believe this is not supposed to happen?

If my assumption is correct, this __is__ supposed to happen.
Otherwise, please provide the full example producing the bug.

[SetTrend]

It will insert a new <br> which Prism will ignore (just like all other HTML elements).

Nope. The <code> element lies within a <pre> element (as suggested by the Prism homepage), so \r\n and \n are both interpreted as newlines by the browser. I debugged my session and I can confirm that \n characters are being inserted.

I will upload a sample in short time.

[SetTrend]

Is there a public CDN available for Prism I can link to in my sample page?

[RunDevelopment]

https://cdnjs.com/libraries/prism will work I guess.

[RunDevelopment]

Prism will also add the language-js CSS class to the <pre> element.
Could it be that this changes the white-space property to something which is not pre, pre-line, or pre-wrap?

I will upload a sample in short time.

Thank you.

[SetTrend]

Nope. Checked.

[SetTrend]

I created a test page now (the following ZIP file contains the HTML file):

PrismJS Test.zip

You were right: The browser adds <br> tags instead of using \n.

So I needed to replace every occurrence of <br> with \n:

let code = document.getElementsByTagName("code")[0];

code.innerHTML = code.innerHTML.replace(/[<]br[/]?[>]/gi,"\n");

Prism.highlightElement(code);

It was the reason you suggested. Thanks!

---

Though, AFAICS, the reason is based on the fact that prism.js uses var code = element.textContent;. If Prism would use element.innerText instead, the <br> would be converted to \n by the browser itself, automatically:

Wouldn't it make sense to update the code line in prism.js accordingly?

[RunDevelopment]

innerText has a few other properties which make it less useful for Prism.
It would also change the current behavior of Prism, so for some people out there, this might be a breaking change.

Also, you can use hooks to make Prism use innerText.

Prism.hooks.add('before-sanity-check', function (env) {
    env.code = env.element.innerText;
});

[SetTrend]

From the description you mentioned I can only read the inteded behaviour and a performance issue.

In contrast to the documents' statement, innerText has become a standard meanwhile:

[RunDevelopment]

From the description you mentioned I can only read the inteded behaviour and a performance issue.

Performance aside: Is __breaking change__ not enough reason?

Also, I don't think that it's a good idea for the text Prism highlights to change depending on whether the element is being rendered or not.

What do you think @mAAdhaTTah ?

[SetTrend]

Please pardon me for being ignorant. Would you mind elaborating on the issues you see?

I'm just trying to being constructive.

I tested three different scenarios now:

1. <pre><code> </code></pre>, as suggested by Prism documentation.
2. <code></code>
3. <pre></pre>

From my perspective I don't see any negative consequences when applying #1766. On the contrary.

<pre> is supposed to retain newline characters, so it doesn't make sense to remove them (in the original code, see merge request).

[RunDevelopment]

<pre> is supposed to retain newline characters

It does but only _characters_.

From my perspective I don't see any negative consequences

My main problem is that textContent and innerText behave slightly differently. Most users of Prism probably wouldn't even notice the change because the <code> element Prism highlights isn't supposed to contain markup.
( except when it does)

Would you mind elaborating on the issues you see?

innerText is intended to return the text the user gets when selecting and copying the visible text of an element.
So it makes sense that this is what _you_ want. You're writing a text editor, so it makes sense that Prism should highlight what the user sees.

My problem is that this not how Prism behaves now. Prism doesn't highlight the text the user sees, but the text which is there (sounds dumb, I know...).
As an example: innerText doesn't return the text of elements which have display: none, because they are not visible. Now, how should Prism behave?
I say display: none shouldn't matter because markup is stripped.
Btw.: Same problem for display: block where extra \ns will be inserted around the block.

Now, the next problem: innerText is only really defined for elements which are visible. As a consequence, its behavior changes depending on whether the element, from which you want the innerText, is visible.
Example: Let's say we highlight a code element. With innerText we might get _different_ results depending on whether the element is visible (_right now_) or not. (Happy debugging...)

(Well and then again: Most users don't care, so why not do them a favor and use what's faster.)

Summary:
My main issue is that the behavior of Prism changes. So we risk breaking the code of a lot of Prism users.
Also, there is this more general question of how Prism __should__ behave. Should it highlight the visible text or all text?

---

Again: I can see why you want to use innerText in your specific use case but you don't need to change all of Prism for that. You can use the features Prism already provides to implement this.

[SetTrend]

I'm not sure whether these edge cases really matter. However, of course, I refrain from my pull request.

Just to finish the discussion, regardless of the result:

• I wouldn't expect nodes deliberately designated to be display: none to still appear. I personally would consider this behaviour unexpected, or even not desired, if I happened to stumble over it.

• I would expect nodes deliberately designated to be display: block to be displayed with line-breaks. Otherwise I wouldn't expect the web page designer to have applied that attribute to those nodes in the first place.

• A few minutes ago I applied display: none to the <code style="display: none"> element. The result: innerText was the same as textContent: Both omit child elements when the element is hidden.

  I comprehend that old (pre-standard) IE implementations behave differently. For these edge cases element.innerText || element.textContent should have sufficed.
  
  

I would have considered the innerText behaviour - including its line-break behaviour - much more what I'd expect from the converted result as the textContent behaviour.

[SetTrend]

The hook appears to be incorrect. The correct hook name is: before-highlight

Prism.hooks.add('before-highlight', function (env)
{
  env.code = env.element.innerText;
});

[RunDevelopment]

The hook appears to be incorrect. The correct hook name is: before-highlight

I specifically wanted before-sanity-check because it is executed before (almost) all other hooks and (more importantly) before the checks of highlightElement.

Did it not work?

[regularlabs]

This works too:

Prism.hooks.add('before-sanity-check', function (env)
{
  env.element.innerHTML = env.element.innerHTML.replace(/<br>/g, '\n');
  env.code = env.element.textContent;
});