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

minify_html = true causes Major page formatting issues to pages with numbered code blocks. #1942

Closed
Jieiku opened this issue Jul 25, 2022 · 13 comments
Labels
bug done in pr Already done in a PR

Comments

@Jieiku
Copy link
Contributor

Jieiku commented Jul 25, 2022

Bug Report

minify_html = true causes Major page formatting issues to pages with numbered code blocks.

This was first reported to the abridge theme: Jieiku/abridge#84

I implemented this in the after-dark theme and seen the same effect (pay particular attention to the footer at the bottom of the pages)

2022-07-25_13-26-01

with minify_html = false it looks normal:

2022-07-25_14-21-34

Environment

Zola version: 0.16.0

I also tested 0.15.3 with the same result.

Expected Behavior

The page should look the same with or without minify_html = true

Current Behavior

No Errors, just errors in page formatting when using minify_html = true

Step to reproduce

git clone https://github.com/Jieiku/zola-minify-test
cd zola-minify-test
zola serve

open this page: http://127.0.0.1:1111/overview-code-blocks/

Notice the page section headings and page footer are all broken following the numbered code blocks.

To further confirm the issue you can removed the line numbered code blocks, then minify_html = true will no longer cause an issue.

@Keats
Copy link
Collaborator

Keats commented Jul 25, 2022

The non minified HTML output looks sane, it might be a bug in https://github.com/wilsonzlin/minify-html that should be reported there

@Jieiku
Copy link
Contributor Author

Jieiku commented Jul 25, 2022

Thanks, will open an issue report now.

@skewballfox
Copy link

The non minified HTML output looks sane, it might be a bug in https://github.com/wilsonzlin/minify-html that should be reported there

It turns out the generated html was attempting to attempting to close nonexistent spans

@Keats
Copy link
Collaborator

Keats commented Aug 9, 2022

Hm weird I didn't notice those invalid span 🤔

@jhpratt
Copy link

jhpratt commented Sep 22, 2022

So it turns out this is going to be a fair bit more complicated than I had hoped.

<span> tags for marking the code as a given language, inside a function, etc. are opened in the first row of the table and not closed until the last row of the table. This is not allowed, as tags have to be closed in the reverse order they are opened. The only solution that would match the intended behavior is to close all tags for each row and re-open them at the start of the next row. Unfortunately this requires knowing which tags they were, which is not something that appears to be readily available.

@Keats
Copy link
Collaborator

Keats commented Jan 17, 2023

Can someone submit a failing test so we have that at least?

@Jieiku
Copy link
Contributor Author

Jieiku commented Jan 17, 2023

I already did in the opening post....

git clone https://github.com/Jieiku/zola-minify-test
cd zola-minify-test
zola serve

I can try and reduce the size of the test to something more basic though.... (let me know what your looking for)

@Keats
Copy link
Collaborator

Keats commented Jan 17, 2023

Woops sorry I missed the link to the repo in the initial post when I commented.

@skewballfox
Copy link

hey, was poking around the codebase. Where is the code which is causing the bug, or the likely source of the bug.

Also what's the components dir separate from src?

@Jieiku
Copy link
Contributor Author

Jieiku commented Mar 11, 2023

I'de say find the section that deals with numbered code blocks, but I do not know where that is, it sounds like @jhpratt might have investigated it at one point, before it ended up looking like a difficult fix, so maybe they know.

@jhpratt
Copy link

jhpratt commented Mar 11, 2023

I don't recall offhand what file or chunk of code it was that was relevant. My previous comment effectively sums up my recollection. It's not an easy task, unfortunately.

@Keats
Copy link
Collaborator

Keats commented Mar 11, 2023

It will somewhere in https://github.com/getzola/zola/tree/master/components/markdown/src/codeblock

Also what's the components dir separate from src?

This is a Cargo workspace, Zola is split in a few sub crates to speed up compilation.

@TheOnlyMrCat
Copy link
Contributor

TheOnlyMrCat commented Jul 3, 2023

The issue to me seems to be that syntect's HTML ouptut is very inflexible.

The most flexible way (and the method we're currently using) is with line_tokens_to_passed_spans, which requires us to store a &mut ScopeStack parameter for it. We can interact with and inspect the Scopes in the scope stack, but we're missing the scope_to_classes function (used internally by line_tokens_to_passed_spans) that would properly allow us to close-then-reopen the spans.

It is possible to copy the implementation of scope_to_classes from syntect directly into zola's own code, but that's not necessarily good for maintainability.

Incidentally, this does not seem to be an issue for the InlineHighlighter since styled_line_to_highlighted_html always closes its spans.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug done in pr Already done in a PR
Projects
None yet
Development

No branches or pull requests

5 participants