Keep the doctype so saved pages are not quirks mode (issue #16) - #78
Merged
Conversation
kage serialises a rendered page as the outerHTML of <html>. A doctype is a sibling of <html> rather than a child, so it was never in that string and every page kage has ever written came out without one. A document with no doctype is quirks mode in every browser. The box model reverts to the pre-CSS2 IE one and line-height, table cell inheritance and vertical-align all change, so the saved copy lays out differently from the original. It also weakens the <meta charset> that ensureCharset guarantees: in standards mode that declaration is authoritative, while in quirks mode a browser is readier to fall back to its locale encoding and mojibake the text. That is the symptom reported in issue #16, and a webview or e-reader with no encoding menu has no way back from it. Read the doctype from the DOM after the render and put it back. It is reproduced exactly rather than replaced with <!DOCTYPE html>, because the string itself selects the rendering mode: HTML 4.01 Transitional is standards mode with its system identifier and quirks mode without it. A page that genuinely had no doctype on the live web still gets none, so it keeps rendering the way its author saw it. The parts come from an untrusted page and land at the top of a file we write, so the source form is assembled in Go and anything that could close the token early or carry markup is dropped. The banner comment now goes after the doctype so the doctype is the first thing in the file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #16.
The report
No URL and no saved file, so this sat unanswered. There is a real bug behind it, and it is not quite the one the title names.
What is actually wrong
browser.Renderserialises a page withpage.HTML(), which is the outerHTML of the<html>element. A doctype is a sibling of<html>, not a child, so it is never in that string.html.Parsethen builds a tree with noDoctypeNodeandhtml.Renderwrites no doctype. Every page kage has ever saved comes out without one.A document with no doctype is quirks mode in every browser, and that produces the reported symptom directly. In standards mode a
<meta charset>is authoritative. In quirks mode a browser is readier to fall back to its locale default, which is windows-1252 in Western locales, so the text mojibakes and the user has to pick the encoding by hand. The reporter's worry about webview is well founded: a webview or an e-reader has no encoding menu, so there is no way back. For a tool whose output is meant to be read in Kiwix on a plane, that matters.There is a second symptom nobody reported. Quirks mode reverts the box model to the pre-CSS2 IE one and changes
line-height, table cell inheritance andvertical-align, so every saved page also lays out differently from the original.The fix
Read
document.doctypeafter the render and put it back at the top of the serialised page.The doctype is reproduced exactly rather than replaced with a flat
<!DOCTYPE html>, because the string itself selects the rendering mode. HTML 4.01 Transitional is standards mode with its system identifier and quirks mode without it, and those old sites are exactly the audience for--mobile-readable. Reproducing it means no page changes rendering mode in either direction: a page that was standards mode live is standards mode offline, and a page that genuinely had no doctype stays quirks so it keeps rendering the way its author saw it.The parts arrive from an untrusted page and land at the very top of a file we write, so the source form is assembled in Go rather than taken from the page's own
XMLSerializer.x/net/htmlquotes the identifiers but does not escape a quote inside one, and it writes the name verbatim, so anything that could close the token early or carry markup is dropped rather than escaped.One thing in
sanitize:insertBannerput thecloned by kagecomment atroot.FirstChild, which with a doctype present now means ahead of it. Modern browsers still read a doctype that follows a comment, but older ones and several offline readers do not, so the banner now goes after it.Verified
TestRenderPreservesDoctypedrives real Chrome over three pages, HTML5, HTML 4.01 Transitional and no doctype, and checks each comes back the way it went in.TestRenderDoctypecovers the assembly and the rejection cases without Chrome.sanitizegets two tests: a doctype survivesStripwith the banner behind it, and sanitize never invents one.End to end on a local site:
go build ./...,go vet ./...andgo test ./...all pass with Chrome present.Not in scope
#16 has a second half that #74 already covers: a page declaring
<meta charset="iso-8859-1">keeps that declaration even though kage writes UTF-8 bytes. The two fixes are independent and go well together, since #74 corrects the meta and this makes the meta authoritative again. I left it out to keep the two reviewable separately.