Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Newlines are lost during formatting #3285

Closed
nrktkt opened this issue Jul 24, 2021 · 15 comments
Closed

Newlines are lost during formatting #3285

nrktkt opened this issue Jul 24, 2021 · 15 comments
Labels
bug help welcome Could use help from community parser

Comments

@nrktkt
Copy link

nrktkt commented Jul 24, 2021

Describe the issue/behavior that seems buggy
Preformatted code is lost after highlighting a block in 11.1.
It seems that in version 10 br elements were inserted, and now they aren't. Although I don't know why that should matter in a pre block.

Sample Code or Instructions to Reproduce

I have a little library that uses highlight to embed source using web components.
https://kag0.github.io/sauce/
You can see on the documentation page, it's embedding snippets from an example html file in the repo https://kag0.github.io/sauce/example.html but they're missing the line breaks.

To prove that they would normally have line breaks:

  1. clone the repo
  2. open example.html and change the script embed from https://kag0.github.io/sauce/sauce.js to the local sauce.js
  3. open sauce.js and comment out line 43 .then(_ => hljs.highlightBlock(code))
  4. open example.html in the browser, observe that line breaks and indentation are correct
  5. uncomment the line and observe that code is highlighted, but line breaks are gone

Expected behavior
line breaks should be preserved

Additional context
rel
#2131
#3242

@nrktkt nrktkt added bug help welcome Could use help from community parser labels Jul 24, 2021
@joshgoebel
Copy link
Member

joshgoebel commented Jul 24, 2021

Not sure what I'm looking at. All the examples look fine to me here in Safari.

  • We literally do NOTHING with linebreaks in pre/code, they are preserved - without being modified. There are exactly as many line breaks before and after highlighting. We only add span tags.
  • We no longer support adding <br>, the useBR option is gone. (parser) Deprecate useBR #2559
  • If you were using that you'll now need to use a plugin OR switch to the recommended <pre><code> markup or use CSS to fix linebreaks with non-standard mark-up.

@joshgoebel
Copy link
Member

joshgoebel commented Jul 24, 2021

You may also want to look at textContent vs innerText... there are very different line handling behavior with the various APIs.

@nrktkt
Copy link
Author

nrktkt commented Jul 24, 2021

All the examples look fine to me here in Safari.

Sorry about that, I had pinned example.html to 10.6. It's now updated to 11.1. If you revisit https://kag0.github.io/sauce/example.html you should see the lines lost.

However, following the reproduction steps I outlined should still have demonstrated the issue.

switch to the recommended <pre><code>

If you inspect the code snippets, you should see them in <pre><code> blocks (and the repro steps will show that they're working in the absence of highlighting). I'll look at the plugin in #2559 in the meantime, but I'm hoping I won't need it. Or if I do, at least to understand why.

@nrktkt
Copy link
Author

nrktkt commented Jul 24, 2021

textContent vs innerText

ah hah! That did indeed make the difference. Thank you!
Could you educate me on why that is? pre doesn't seem to care, but some api called by highlight does?

@joshgoebel
Copy link
Member

some api called by highlight does?

Nothing to do with us, the two DOM APIs are subtly different in their behavior. I'm not aware of why this should have changed from version 10 to version 11 though.

Could you educate me on why that is?

https://stackoverflow.com/questions/35213147/difference-between-textcontent-vs-innertext

@joshgoebel
Copy link
Member

ah hah! That did indeed make the difference.

Glad to help. Closing issue.

@nrktkt
Copy link
Author

nrktkt commented Jul 25, 2021

Nothing to do with us

Could we at least document for folks in the future that if they programmatically populate their blocks, they need to use textContent and not innerText? I think that's certainly non-obvious to me as an inexperienced web developer.

why this should have changed from version 10 to version 11 though

I don't think it did exactly, 10 just added br so it was irrelevant and 11 doesn't. So before they looked the same visually but now <pre><code> blocks populated with innerText are arguably "broken" (based on what you were saying in #2131)

code block should be structured the same BEFORE and AFTER syntax highlighting other than color/italics/bold/etc. IE, highlighting a snippet should NOT alter the visible whitespace... I don't expect whitespace/line breaks to be added, and I don't expect it to be removed

@joshgoebel
Copy link
Member

Could we at least document for folks in the future that if they programmatically populate their blocks, they need to use textContent and not innerText? I think that's certainly non-obvious to me as an inexperienced web developer.

It really depends on what you are trying to do. There are uses for the different APIs, that's why they each exist... knowing how to user the DOM API is kind of outside the scope of using HLJS. Perhaps you could create a wiki page though ? I'm not sure where else we'd document that as I'm not sure it's actually super relevant for many users (and our README is already quite long). I thought of FAQ, but this isn't really a FAQ either (IME)... did you have a specific suggestion?

I don't think it did exactly, 10 just added br

Only if use used the useBR option (perhaps you were?)... otherwise I'm not aware of a behavior difference in this area.

@yn-academia
Copy link

yn-academia commented Apr 21, 2022

Just walked right into this one. I guess the existence of this issue is documentation in itself.

Regarding FAQ: I don't know how often a question needs to arise in order to make it into FAQ, but it seems that multiple people have encountered this, or something related, in #1664

I agree that how to use the DOM API is outside the scope of using HLJS and its documentation.

However, people will continue opening Github issues for this in HLJS, because it's not obvious right away that it's DOM misuse. Having in the FAQ "before you create a github issue about missing highlights due to comments, or broken line breaks, or whitespace issues, or anything else, make sure you know the difference between innerText and textContent, and are using the latter" - that would at least prevent people from opening new Github issues for this project.

@ribeirobreno
Copy link

ribeirobreno commented Mar 25, 2024

You may also want to look at textContent vs innerText... there are very different line handling behavior with the various APIs.

I would love to see this information here:
https://github.com/highlightjs/highlight.js/blob/main/README.md#using-custom-html

This one phrase would have saved me a lot of time.

Edit: My code did use <pre><code> as suggested in the readme and the docs.
Edit 2: The linebreaks were perfectly fine without highlight.js, keeping everything else the same.

@joshgoebel
Copy link
Member

Were you using <br>?

@ribeirobreno
Copy link

Were you using <br>?

No.

@joshgoebel
Copy link
Member

What exactly was the issue with your linebreaks then, do you recall?

@ribeirobreno
Copy link

What exactly was the issue with your linebreaks then, do you recall?

This: https://jsfiddle.net/scbL8p2j/1/

@joshgoebel
Copy link
Member

If you check the browse console you'll see tons of errors/warnings. Perhaps we could make a more specific error for using <br>/innerText...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help welcome Could use help from community parser
Projects
None yet
Development

No branches or pull requests

4 participants