Skip to content

Conversation

@cderv
Copy link
Collaborator

@cderv cderv commented Apr 4, 2025

fix #12295

If a HTML file does not have DOCTYPE, then the browser are using Quirks mode and not applying standard HTML5 CSS. Hence the problem seen in #12295

@cscheid
Copy link
Collaborator

cscheid commented Apr 4, 2025

Good fix! But do we understand where the regression comes from?

@cderv
Copy link
Collaborator Author

cderv commented Apr 4, 2025

But do we understand where the regression comes from?

I tried to explain this above. Let me try to reformulate. When there is no DOCTYPE in the document, browsers use specific modes that don't use the same browser's default CSS. The link I shared above explains that.

The problem is caused by a difference in the browser's default CSS. I can't point you to the rule exactly.

However, we were wrongly removing the DOCTYPE while doing embedding, so it seems to be the cause of the breakage in Quarto, even thought I did not see the browser rule that changed.

Quirks mode showed other warning in Browser console, so we definitely don't want that.

@cscheid
Copy link
Collaborator

cscheid commented Apr 4, 2025

Sorry. What I meant is that I thought this was a regression given #12295 (comment), and I would like to understand what caused the regression.

@cderv
Copy link
Collaborator Author

cderv commented Apr 4, 2025

Did you see #12295 (comment) ?

The commit that cause the problem is 19cf9ef where esbuild is used before embedding.

Following the change the DOM is parsed

const contents = Deno.readTextFileSync(file);
const dom = await parseHtml(contents);

and inserted in raw block after esbuild run
await bundleModules(dom, workingDir);
const input: string[] = [];
input.push("````````{=html}");
input.push(dom.documentElement!.outerHTML);
input.push("````````");

Before that it was directly the text file content put in raw block (which includes DOCTYPE line)

const contents = Deno.readTextFileSync(file);
const input: string[] = [];
input.push("````````{=html}");
input.push(contents);
input.push("````````");

But parsing the DOM and only use dom.documentElement!.outerHTML looses the DOCTYPE it seems.
So we need to put it back first if any which this PR does.

I feel you may have understood this already, and I missing the point of what you want to understand... 🤔 am I ?

@cscheid
Copy link
Collaborator

cscheid commented Apr 4, 2025

This clarifies it, thank you! It's the roundtrip through the DOM parser specifically, and not esbuild itself.

@cscheid cscheid merged commit efe3a9d into main Apr 4, 2025
49 checks passed
@cscheid cscheid deleted the fix/embed-doctype branch April 4, 2025 18:31
@cderv
Copy link
Collaborator Author

cderv commented Apr 4, 2025

It's the roundtrip through the DOM parser specifically, and not esbuild itself.

Yes exactly ! A very good summary 😄 thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

min-height: 100% for code in Reveal.js when embed-resources: true

3 participants