diff --git a/tests/background.js b/tests/background.js index eaffe6b..00da25e 100644 --- a/tests/background.js +++ b/tests/background.js @@ -303,9 +303,11 @@ async function test(name, body) { await settle(); assert.strictEqual(absentResult, true); // Order is the assertion, not just membership: jszip and readability are - // globals the scripts after them use at load time. + // globals the scripts after them use at load time, and sanitizeHtml.js + // defines the tag lists extractHtml.js reads while it loads. assert.deepStrictEqual(absent.state.executeScripts[0].files, - ['libs/jszip.js', 'libs/readability.js', 'utils.js', 'extractHtml.js', 'saveEbook.js']); + ['libs/jszip.js', 'libs/readability.js', 'utils.js', 'sanitizeHtml.js', + 'extractHtml.js', 'saveEbook.js']); const denied = createHarness({pingResponse: undefined, executeScriptFailure: true}); let deniedResult; @@ -431,10 +433,27 @@ async function test(name, body) { assert.deepStrictEqual(request(h, {type: 'get uuid'}), [{uuid: 'second-id'}]); }); + await test('the book stylesheet persists for a book and is discarded with it', () => { + const h = createHarness(); + assert.deepStrictEqual(request(h, {type: 'get book css'}), [{css: ''}], + 'a book nobody styled has an empty stylesheet, not a missing one'); + request(h, {type: 'set book css', css: 'body{font-family:serif}'}); + assert.deepStrictEqual(request(h, {type: 'get book css'}), + [{css: 'body{font-family:serif}'}]); + request(h, {type: 'remove'}); + assert.strictEqual(h.state.local.bookCss, undefined, + 'the stylesheet belongs to the discarded chapters'); + // the per-site styles are a different thing entirely and outlive any book + request(h, {type: 'set styles', styles: [{title: 'a site'}]}); + request(h, {type: 'remove'}); + assert.deepStrictEqual(h.state.local.styles, [{title: 'a site'}]); + }); + await test('each runtime message sends at most one response', async () => { const cases = [ {type: 'get'}, {type: 'set', pages: []}, {type: 'remove'}, {type: 'get uuid'}, {type: 'get title'}, {type: 'set title', title: 'T'}, + {type: 'get book css'}, {type: 'set book css', css: 'p{}'}, {type: 'get styles'}, {type: 'set styles', styles: []}, {type: 'get current style'}, {type: 'set current style', currentStyle: 2}, {type: 'get include style'}, {type: 'set include style', includeStyle: true}, diff --git a/tests/build-edited-epub.sh b/tests/build-edited-epub.sh new file mode 100755 index 0000000..a5bdaac --- /dev/null +++ b/tests/build-edited-epub.sh @@ -0,0 +1,48 @@ +#!/bin/bash +# Builds a book out of chapters that were edited in the chapter editor, so that +# EPUBCheck sees content an edit produced. +# +# The editing happens in a real DOM - contentEditable, element removal and the +# serialization back out of it are all browser behaviour, and a shim would be +# testing the shim - so the chapters come from the headless run of +# dom/chapter-editing.html, which writes them into the page as JSON. This script +# lifts them back out of the dumped dom and hands them to the same builder the +# extension ships. +# +# The whole risk of the editing feature is emitting XHTML a reading system +# cannot parse. This is the step that would notice. +set -u + +cd "$(dirname "$0")" || exit 1 + +CHROME="${CHROME:-/Applications/Google Chrome.app/Contents/MacOS/Google Chrome}" +if [ ! -x "$CHROME" ]; then + CHROME="$(command -v google-chrome || command -v chromium || true)" +fi +if [ -z "$CHROME" ] || [ ! -x "$CHROME" ]; then + echo "SKIP: no Chrome found. Set CHROME=/path/to/chrome" >&2 + exit 0 +fi + +mkdir -p out + +dom=$("$CHROME" --headless --disable-gpu --no-sandbox \ + --allow-file-access-from-files --virtual-time-budget=8000 \ + --dump-dom "file://$PWD/dom/chapter-editing.html" 2>/dev/null) + +# One line, written by exportSavedChapters(). The dump is html, so the three +# characters the serializer escapes have to be put back before it is json again. +json=$(echo "$dom" \ + | grep -o '
.*
' \ + | sed -e 's/^
//' -e 's|
$||' \ + -e 's/<//g' -e 's/&/\&/g') + +if [ -z "$json" ]; then + echo "FAILED: the editing fixture produced no saved chapters" + exit 1 +fi + +printf '%s' "$json" > out/edited-chapters.json + +node build-epub.js out/edited.epub --chapters out/edited-chapters.json || exit 1 +./run-epubcheck.sh out/edited.epub diff --git a/tests/build-epub.js b/tests/build-epub.js index abe5650..8fe9fc7 100644 --- a/tests/build-epub.js +++ b/tests/build-epub.js @@ -2,13 +2,18 @@ // saveEbook.js, so what EPUBCheck validates is the file the extension ships - // not a reimplementation of it. // -// node build-epub.js [out.epub] [--single] +// node build-epub.js [out.epub] [--single] [--chapters file.json] // // --single builds a one-chapter book. That is the dominant real case ("save this // page"), and the package metadata takes different branches for it: dc:date and // dc:description come from the article itself, which would be a guess to assert // for a book assembled from several pages. // +// --chapters builds from a chapter list on disk instead of the fixture below. +// build-edited-epub.sh writes one out of the chapter editor - chapters that were +// edited in a live DOM and serialized back - so that EPUBCheck sees the content +// an edit produces and not only the content extraction produces. +// // saveEbook.js expects a browser: the handful of globals it touches are stubbed // below, and downloadBlob() is swapped for a write to disk. @@ -19,7 +24,11 @@ const vm = require('vm'); const EXT = path.join(__dirname, '..', 'web-extension'); const args = process.argv.slice(2); const single = args.indexOf('--single') > -1; -const outFile = args.filter((a) => a !== '--single')[0] || +const chaptersFlag = args.indexOf('--chapters'); +const chaptersFile = chaptersFlag > -1 ? args[chaptersFlag + 1] : null; +// the first argument that is not a flag, and not the file name belonging to one +const outFile = args.filter((a, i) => a !== '--single' && a !== '--chapters' && + !(chaptersFlag > -1 && i === chaptersFlag + 1))[0] || path.join(__dirname, 'out', 'fixture.epub'); // 1x1 png @@ -45,6 +54,16 @@ const chapters = [ }, styleFileName: 'style0.css', styleFileContent: '.a1 {color:rgb(0, 0, 0);font-size:12px;}', + // Css the user wrote for this one chapter, alongside the book-wide + // stylesheet passed to buildEbook() below. Both are here so that + // EPUBCheck sees them: user-authored css is deliberately not held to the + // allowlist extraction uses, and this is what says that a book carrying + // some is still a valid book. + customCss: '.a1 {text-align: justify;}\n' + + // and that what does escape a stylesheet is taken out before + // it can contradict a manifest that claims no remote resources + '.a1 {background-image: url("https://cdn.example.com/paper.png");}\n' + + '@import url("https://cdn.example.com/more.css");', // img-3 is the chapter a version that could not type a webp left in // storage: an image with no resolvable media type, which cannot be // declared in the manifest. It and the below pointing at it must @@ -239,11 +258,24 @@ sandbox.downloadBlob = function (blob, fileName) { built = {blob: blob, fileName: fileName}; }; -sandbox.ebookTitle = 'Fixture Book'; -vm.runInContext('_buildEbook(FIXTURE)', - Object.assign(sandbox, {FIXTURE: single ? chapters.slice(0, 1) : chapters})); +let toBuild = chapters; +if (chaptersFile) { + toBuild = JSON.parse(fs.readFileSync(chaptersFile, 'utf8')); + if (!Array.isArray(toBuild) || toBuild.length === 0) { + console.error('FAILED: ' + chaptersFile + ' holds no chapters'); + process.exit(1); + } + console.log('building from ' + chaptersFile + ' (' + toBuild.length + ' chapters)'); +} -// _buildEbook finishes asynchronously in generateAsync +vm.runInContext('buildEbook(FIXTURE, {title: "Fixture Book", css: BOOK_CSS})', + Object.assign(sandbox, { + FIXTURE: single ? toBuild.slice(0, 1) : toBuild, + BOOK_CSS: 'body {\n font-family: serif;\n line-height: 1.5;\n}\n' + + 'blockquote {\n font-style: italic;\n}\n' + })); + +// buildEbook finishes asynchronously in generateAsync const deadline = Date.now() + 30000; (function wait() { if (built) { @@ -255,7 +287,7 @@ const deadline = Date.now() + 30000; return; } if (Date.now() > deadline) { - console.error('FAILED: _buildEbook never produced a file'); + console.error('FAILED: buildEbook never produced a file'); process.exit(1); } setTimeout(wait, 50); diff --git a/tests/dom/alt-text.html b/tests/dom/alt-text.html index eac6807..44a9998 100644 --- a/tests/dom/alt-text.html +++ b/tests/dom/alt-text.html @@ -8,6 +8,7 @@ window.chrome = { }; + x'})); + report('css cannot break out of the style element it is inlined into', + hostile.querySelectorAll('#escaped').length === 0); + + // ---- the panel it is rendered in ------------------------------------------ + var previewButton = document.getElementById('li0').getElementsByTagName('button')[0]; + var panel = document.getElementById('chapterEditor-previewPanel'); + report('the preview panel starts closed', panel.style.display === 'none'); + + // an unsaved rename is what the chapter heading has to show + document.getElementById('text0').value = 'Renamed One'; + previewButton.click(); + + var frame = document.getElementById('chapterEditor-previewFrame'); + report('previewing renders a frame instead of alerting', !!frame); + report('the panel is open', panel.style.display === 'block'); + // allow-scripts is the flag that matters and it is not there: the content is + // script-free by construction and the sandbox is what keeps it that way if it + // stops being. allow-same-origin grants the content nothing - nothing in it + // can run - and is what lets the editor reach into the document. + report('the frame is sandboxed, and not for scripts', + !!frame && frame.getAttribute('sandbox') === 'allow-same-origin', + frame ? '"' + frame.getAttribute('sandbox') + '"' : 'no frame'); + report('the frame is fed by srcdoc, not a url', + !!frame && !frame.getAttribute('src') && frame.srcdoc.indexOf('') === 0); + report('the preview shows the rename that has not been saved yet', + !!frame && frame.srcdoc.indexOf('

Renamed One

') > -1); + report('the images are decoded into this frame', + !!frame && frame.srcdoc.indexOf('data:image/png;base64,') > -1); + + // one chapter at a time: the frame holding the previous chapter's decoded + // images goes before the next one is built + document.getElementById('li1').getElementsByTagName('button')[0].click(); + var frames = document.getElementsByTagName('iframe'); + report('only one chapter is rendered at a time', frames.length === 1, + frames.length + ' frames'); + report('the second chapter replaced the first', + frames.length === 1 && frames[0].srcdoc.indexOf('two') > -1); + + // escape belongs to the preview while it is open - it used to close the tab + var escapeKey = new KeyboardEvent('keydown', {keyCode: 27, which: 27}); + document.onkeydown(escapeKey); + report('escape closes the preview', + document.getElementById('chapterEditor-previewPanel').style.display === 'none'); + report('escape did not close the tab behind it', closedTab === false); + report('the closed panel holds no frame', document.getElementsByTagName('iframe').length === 0); + + // ...and once it is closed, escape means what it meant before + document.onkeydown(escapeKey); + report('escape closes the editor once the preview is gone', closedTab === true); + + // ---- and it renders --------------------------------------------------------- + // + // Everything above reads the document as a string. This one loads it, which is + // the only way to know that the data urls decode and the inlined css applies - + // the whole point of the preview. Not sandboxed, only because a sandboxed + // frame has an opaque origin and nothing here could read into it; the frame + // the editor builds is the sandboxed one, and it is checked above. + var probe = document.createElement('iframe'); + probe.srcdoc = buildChapterPreviewDocument({ + title: 'Rendered', + url: 'r.xhtml', + content: '

text

kept', + images: [{filename: 'img-1.png', data: PNG_1X1}], + styleFileName: 'style0.css', + styleFileContent: '.sae-one{color:rgb(1, 2, 3)}' + }, {bookCss: 'p{margin:7px}'}); + probe.onload = function () { + var inner = probe.contentDocument; + var image = inner.querySelector('img'); + var paragraph = inner.querySelector('.sae-one'); + report('the image decodes in the frame', + image.complete && image.naturalWidth === 1, + 'naturalWidth=' + image.naturalWidth); + report('the chapter stylesheet applies', + inner.defaultView.getComputedStyle(paragraph).color === 'rgb(1, 2, 3)', + inner.defaultView.getComputedStyle(paragraph).color); + report('the book stylesheet applies too', + inner.defaultView.getComputedStyle(paragraph).marginTop === '7px', + inner.defaultView.getComputedStyle(paragraph).marginTop); + document.title = 'DONE'; + }; + document.body.appendChild(probe); +}, 300); + + + diff --git a/tests/dom/computed-css.html b/tests/dom/computed-css.html index 90962ba..6b43b0e 100644 --- a/tests/dom/computed-css.html +++ b/tests/dom/computed-css.html @@ -13,6 +13,7 @@ window.chrome = { }; + ' + + '' + + '' + + body + + ''; + + return {html: html, chapter: chapter, hasTitleHeading: outline.hasTitleHeading}; +} + +// The document on its own, for callers that only render it. +function buildChapterPreviewDocument(page, options) { + return buildChapterPreview(page, options).html; +} diff --git a/web-extension/chapters.html b/web-extension/chapters.html index 505e787..1f03ee5 100644 --- a/web-extension/chapters.html +++ b/web-extension/chapters.html @@ -11,7 +11,16 @@ the page able to read everything it did. It is an extension page now. --> + + + + + + diff --git a/web-extension/extractHtml.js b/web-extension/extractHtml.js index 37800d5..424ae5b 100644 --- a/web-extension/extractHtml.js +++ b/web-extension/extractHtml.js @@ -13,82 +13,11 @@ var extractedImages = []; // in every list row - would otherwise download, store and index it once per // , at full size each time. var imageFileNames = new Map(); -var MATHML_NS = 'http://www.w3.org/1998/Math/MathML'; -// MathML Core is what every WebKit and Chromium reading system implements - -// Apple Books, Thorium, Kobo, anything built on either engine - and it dropped a -// large part of what MathML 3 allowed. Everything Core removed is deliberately -// missing from this list and rewritten by normalizeMathMl() instead, because -// passing one of them through means a reader draws nothing where the element -// was: -// -// mfenced expanded into the mrow of fences the MathML 3 spec defines -// it to be equivalent to. A Core reader lays out an mfenced's -// children with no brackets at all, so "f(x)" comes out as "fx" - -// and older MathJax output is full of it. -// mlabeledtr renamed to , keeping the label as its first cell. -// Unwrapping it would put s straight into the . -// menclose unwrapped: the box or strikethrough it draws is lost either -// way, and its content is the part worth keeping. -// mstack, msgroup, mlongdiv -// unwrapped. These lay digits out in a grid for long division; -// without the layout the digits are all that is left, but they -// are at least still in reading order. -// mscarries, mscarry -// dropped with their content - a carried digit annotates a -// position in that grid, and outside it it is a stray number in -// the middle of another number. -// -// mprescripts and none are the other direction: both are required by -// mmultiscripts and neither was ever allowed, so a prescripted symbol lost the -// marker saying where its prescripts begin. -var mathMLTags = [ - 'math', 'maction', 'merror', 'mfrac', 'mglyph', 'mi', 'mmultiscripts', 'mn', 'mo', - 'mover', 'mpadded', 'mphantom', 'mprescripts', 'mroot', 'mrow', 'ms', 'mspace', - 'msqrt', 'mstyle', 'msub', 'msup', 'msubsup', 'mtable', 'mtd', 'mtext', 'mtr', - 'munder', 'munderover', 'none', 'semantics' -]; -// blockquote was missing rather than excluded, and it is not a tag whose absence -// shows up as a gap: it is not in strippedContentTags either, so a quotation was -// unwrapped - the tag went, the words stayed - and ran into the paragraph before -// it with nothing to say where the quote started or ended. -var allowedTags = [ - 'address', 'article', 'aside', 'blockquote', 'footer', 'header', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', - 'hgroup', 'nav', 'section', 'dd', 'div', 'dl', 'dt', 'figcaption', 'figure', 'hr', 'li', - 'main', 'ol', 'p', 'pre', 'ul', 'a', 'abbr', 'b', 'bdi', 'bdo', 'br', 'cite', 'code', 'data', - 'dfn', 'em', 'i', 'img', 'kbd', 'mark', 'q', 'rb', 'rp', 'rt', 'rtc', 'ruby', 's', 'samp', 'small', 'span', - 'strong', 'sub', 'sup', 'time', 'u', 'var', 'wbr', 'del', 'ins', 'caption', 'col', 'colgroup', - 'table', 'tbody', 'td', 'tfoot', 'th', 'thead', 'tr' - // TODO ? - // ,'form', 'button' +// The tag allowlists, the attribute filters and parseHTML() itself live in +// sanitizeHtml.js, which this file requires: the chapter editor writes content +// through the same sanitizer, and it cannot load a content script. - // TODO svg support ? - // , 'svg', 'g', 'path', 'line', 'circle', 'text' -].concat(mathMLTags); -// const svgTags = ['svg', 'g', 'path', 'line', 'circle', 'text'] -// Allowed tags that have no content and must be written self-closed, otherwise -// the generated page is not well formed XML -var voidTags = ['br', 'col', 'hr', 'img', 'wbr']; -// Tags that are dropped together with everything inside them. Any other tag -// that is not in allowedTags is unwrapped instead - the tag goes, its text stays. -var strippedContentTags = [ - 'script', 'style', 'noscript', 'template', 'iframe', 'object', 'embed', 'applet', - 'head', 'title', 'meta', 'link', 'base', - 'textarea', 'select', 'option', 'optgroup', 'input', - 'audio', 'video', 'canvas', 'svg', 'map', 'area', - // keeps its presentation MathML and loses its annotations. An - // annotation is an alternative encoding of the same formula - almost always - // the TeX it was typeset from - and a conforming reader renders only the - // first child of , so it is never meant to be seen. But - // was in neither list, so it was unwrapped instead and its raw - // TeX printed as visible text right beside the rendered formula. That is the - // single most visible MathML bug on a KaTeX page. - // - // The TeX itself is not thrown away: normalizeMathMl() lifts it into the - // first, where it is a fallback rather than a duplicate. - 'annotation', 'annotation-xml', - 'mscarries', 'mscarry' -]; // Extraction runs in three phases, and the split matters: // // read - walk the LIVE dom, measure and compute everything that only exists @@ -770,167 +699,6 @@ function applyReadState(cloneRoot, state) { normalizeMathMl(cloneRoot); } -// MathML carries its meaning in its attributes at least as much as in its tags. -// display="block" alone is the difference between a centred display equation and -// something jammed into the middle of a sentence; stretchy and largeop size the -// brackets and the integral signs; columnalign is what makes a matrix line up; -// linethickness="0" is how a binomial coefficient is written. Only alttext used -// to survive, so all of that was lost on every formula. -// -// The vocabulary is allowed wholesale rather than enumerated, which is the -// opposite of how the HTML attributes above are handled and deliberately so: -// MathML's is a closed vocabulary defined by a spec, an attribute this extension -// has not heard of is far more likely to be one MathML has than one an attacker -// invented, and a list of thirty names would go out of date silently. What is -// excluded is what is dangerous, or what would break the file: -// -// on* event handlers -// href, src MathML's own link and image attributes. href can hold a -// javascript: url, and src would point at a file that is not -// in the archive. -// style, class the chapter stylesheet is generated from computed styles, -// never copied from the page -// id, data-* an html id is carried over - it is what a link inside the -// page points at - but nothing ever links into a formula, and -// an id here would only be one more name the ids minted while -// the chapter is written have to avoid colliding with -// anything with a colon, or that is not a plain lowercase name. An xmlns: or -// xlink: attribute needs a namespace declaration the chapter documents do not -// make, and an undeclared prefix does not make one formula wrong, it makes the -// whole xhtml file unparseable. -var deniedMathMLAttributes = ['href', 'src', 'style', 'class', 'id', 'xmlns']; - -function isAllowedMathMLAttribute(name) { - return /^[a-z][a-z0-9-]*$/.test(name) && - name.indexOf('on') !== 0 && - name.indexOf('data-') !== 0 && - deniedMathMLAttributes.indexOf(name) < 0; -} - -// HTML attributes are enumerated, where MathML's are allowed wholesale. The -// vocabularies differ in the way that matters: HTML's is huge, mostly about -// behaviour rather than meaning, and full of names that either do nothing in a -// reader (onclick, target, srcset) or actively break the file (style and class, -// which the generated stylesheet owns). -// -// What is here is the subset that changes what the chapter says. Everything on -// this list was previously dropped, and dropping it was not a loss of polish: -// -// colspan, rowspan a merged cell renders as an unmerged one, so every row -// after it is short and the columns stop lining up. This -// is a wrong table, not a less navigable one. -// span same, for a column group -// start, reversed a list that should read 7, 8, 9 renders 1, 2, 3 -// value same, for one item in it -// scope the only cheap way to say which cells a header heads -// lang a chapter is tagged with one language; a quotation in -// another is what a nested lang is for, and a speech -// synthesiser reads it in the wrong voice without it -// dir bdo is on the allowed tag list and does nothing at all -// without dir, so an RTL run inside an LTR paragraph came -// out reversed -// datetime, value what
    and
  1. are the two that may be negative or zero -function attrInteger(value) { - var normalized = value.trim(); - return /^-?[0-9]{1,9}$/.test(normalized) ? String(parseInt(normalized, 10)) : ''; -} - -function attrText(value) { - return value.replace(/\s+/g, ' ').trim(); -} - -// datetime is a format, not free text, and epubcheck validates it: letting a -// page's junk through would turn a tag that carries a date into an error in the -// file. The subset here - a year, a month, a date, any of them with a time and -// an offset, or a bare time - is what pages actually write. The exotic -// remainder of the html grammar (weeks, durations, yearless dates) is dropped -// rather than guessed at, which costs the attribute and keeps the element. -var DATETIME_REGEX = new RegExp( - '^(' + - '[0-9]{4}(-[0-9]{2}(-[0-9]{2})?)?' + - '([T ][0-9]{2}:[0-9]{2}(:[0-9]{2}(\\.[0-9]{1,3})?)?' + - '(Z|[+-][0-9]{2}:?[0-9]{2})?)?' + - '|' + - '[0-9]{2}:[0-9]{2}(:[0-9]{2}(\\.[0-9]{1,3})?)?' + - ')$'); - -function attrDateTime(value) { - var normalized = value.trim(); - return DATETIME_REGEX.test(normalized) ? normalized : ''; -} - -// A url attribute that is not the one being followed for content -
    and point at a source, and nothing in the archive depends on -// them resolving, but they are still urls a reader may open. -function attrUrl(value) { - var normalized = value.trim(); - if (normalized === '' || !isSafeLinkUrl(normalized)) { - return ''; - } - return getHref(normalized); -} - -// Valid on every element, so they are checked before the per-tag table -var globalHtmlAttributes = { - lang: normalizeLanguageTag, - dir: attrEnum(['ltr', 'rtl', 'auto']) -}; - -var htmlAttributes = { - a: {}, - abbr: {title: attrText}, - blockquote: {cite: attrUrl}, - q: {cite: attrUrl}, - del: {cite: attrUrl, datetime: attrDateTime}, - ins: {cite: attrUrl, datetime: attrDateTime}, - time: {datetime: attrDateTime}, - data: {value: attrText}, - // A boolean attribute is true because it is there, whatever it says - and - // XHTML has no bare form, so it is written back out in the long one - ol: {start: attrInteger, reversed: function() { return 'reversed'; }}, - li: {value: attrInteger}, - col: {span: attrCount(1)}, - colgroup: {span: attrCount(1)}, - // rowspan="0" is legal and means "to the end of the section"; colspan="0" is - // not, which is why the two have different floors - td: {colspan: attrCount(1), rowspan: attrCount(0)}, - th: {colspan: attrCount(1), rowspan: attrCount(0), scope: attrEnum(['row', 'col', 'rowgroup', 'colgroup'])} -}; - // Ids already written into this chapter. An id is only addressable if it is // unique in the file it lives in, and a page that repeats one - which is invalid // html but common - would otherwise repeat it here too. @@ -940,56 +708,6 @@ var htmlAttributes = { // not hand out an id the first already used. var usedElementIds = new Set(); -// walkHtmlFragment hands over values the HTML parser has already decoded, so -// they only need escaping - decoding here as well would turn a literal -// "&lt;" on the page into a "<" in the epub -function attrValue(value) { - return escapeXMLChars(value == null ? '' : String(value)); -} - -// The id, the globals and whatever the tag itself allows. Attributes the caller -// has already written - src, alt, href, class and the img dimensions - are not -// in any of these tables and cannot be duplicated from here. -function extraHtmlAttributes(tag, attrs) { - let result = ''; - let perTag = htmlAttributes[tag] || {}; - // A page is free to write the same attribute twice. The parser hands both - // over, and writing both would make the chapter unparseable. - let written = new Set(); - - for (let i = 0; i < attrs.length; i++) { - let name = String(attrs[i].name).toLowerCase(); - let rawValue = attrs[i].value == null ? '' : String(attrs[i].value); - - if (written.has(name)) { - continue; - } - - if (name === 'id') { - if (!isUsableId(rawValue) || usedElementIds.has(rawValue)) { - continue; - } - usedElementIds.add(rawValue); - written.add(name); - result += ' id="' + attrValue(rawValue) + '"'; - continue; - } - - let filter = globalHtmlAttributes[name] || perTag[name]; - if (!filter) { - continue; - } - let value = filter(rawValue); - if (value === '') { - continue; - } - written.add(name); - result += ' ' + name + '="' + attrValue(value) + '"'; - } - - return result; -} - // Where an ends up. A link the page made to a position inside itself // stays a link to a position inside the chapter, and everything else is // resolved against the page so that it still works away from it. @@ -1014,6 +732,20 @@ function linkHref(rawHref) { return getHref(value); } +// Where a url-valued attribute that is not an ends up -
    , . Nothing in the archive depends on these resolving and none +// of them addresses a position in the chapter, so unlike linkHref() a fragment +// is absolutised with everything else. What still matters is the scheme: a +// javascript: cite is inert in most readers, but not all, and epubcheck rejects +// it outright. +function citeUrl(rawValue) { + let value = rawValue == null ? '' : String(rawValue).trim(); + if (value === '' || !isSafeLinkUrl(value)) { + return ''; + } + return getHref(value); +} + // The fragment of an href that addresses this same document, or null if the href // goes somewhere else. Both spellings count: the bare "#note-3", and the fully // written out form a template engine emits, which resolves to this page. @@ -1040,184 +772,24 @@ function sameDocumentFragment(href) { } } -// Appends to allImages / extractedImages - the caller owns resetting them, so -// that a multi-range selection accumulates the images of every range instead of -// each range discarding the ones collected before it. -function parseHTML(rawContentString) { - let results = ''; - let lastFragment = ''; - // Tags written to the output that still need a closing tag. Kept here rather - // than trusting the input to be balanced: a page can close tags it never - // opened, or leave tags open, and the epub still has to be valid XML. - let openTags = []; - // Tags whose content is being dropped, innermost last - let skippedTags = []; - - let isVoidTag = (tag) => voidTags.indexOf(tag) > -1; - - let closeTagsDownTo = (index) => { - for (let i = openTags.length - 1; i >= index; i--) { - results += ''; - } - openTags.length = index; - }; - - try { - walkHtmlFragment(rawContentString, { - start: function(tag, attrs, unary) { - if (skippedTags.length > 0) { - // Inside dropped content - track nesting so that the matching - // end tag, and only it, ends the skip - if (!unary && !isVoidTag(tag)) { - skippedTags.push(tag); - } - return; - } - - if (allowedTags.indexOf(tag) < 0) { - if (strippedContentTags.indexOf(tag) > -1 && !unary && !isVoidTag(tag)) { - skippedTags.push(tag); - } - return; - } - - let tmpAttrsTxt = ''; - - if (tag === 'img') { - let tmpSrc = '' - // null and '' are different answers: null is a source that - // never said anything about this image, '' is one that - // deliberately marked it decorative. Collapsing them - which - // is what writing alt="" on everything did - turns every - // undescribed image into a false claim that it needs no - // description, and the accessibility metadata is computed - // from exactly this distinction. - let tmpAlt = null; - for (let i = 0; i < attrs.length; i++) { - if (attrs[i].name === 'src') { - tmpSrc = getImageSrc(attrs[i].value) - tmpAttrsTxt += ' src="' + attrValue(tmpSrc) + '"'; - } else if (attrs[i].name === 'alt') { - tmpAlt = attrs[i].value == null ? '' : String(attrs[i].value); - } else if (attrs[i].name === 'data-class') { - tmpAttrsTxt += ' class="' + attrValue(attrs[i].value) + '"'; - } else if (attrs[i].name === 'width') { - // used when converting svg to img - the result image was too big - tmpAttrsTxt += ' width="' + attrValue(attrs[i].value) + '"'; - } else if (attrs[i].name === 'height') { - // used when converting svg to img - the result image was too big - tmpAttrsTxt += ' height="' + attrValue(attrs[i].value) + '"'; - } - } - if (tmpSrc === '') { - // ignore imgs without source - return; - } - if (tmpAlt !== null) { - tmpAttrsTxt += ' alt="' + attrValue(tmpAlt.replace(/\s+/g, ' ').trim()) + '"'; - } - } else if (tag === 'a') { - for (let i = 0; i < attrs.length; i++) { - if (attrs[i].name === 'href') { - let href = linkHref(attrs[i].value); - if (href !== '') { - tmpAttrsTxt += ' href="' + attrValue(href) + '"'; - } - } else if (attrs[i].name === 'data-class') { - tmpAttrsTxt += ' class="' + attrValue(attrs[i].value) + '"'; - } - } - } else if (mathMLTags.indexOf(tag) > -1) { - if (tag === 'math') { - // in xhtml, MathML is only MathML because of this - tmpAttrsTxt += ' xmlns="' + MATHML_NS + '"'; - } - for (let i = 0; i < attrs.length; i++) { - if (isAllowedMathMLAttribute(attrs[i].name)) { - tmpAttrsTxt += ' ' + attrs[i].name + '="' + attrValue(attrs[i].value) + '"'; - } - } - } else { - for (let i = 0; i < attrs.length; i++) { - if (attrs[i].name === 'data-class') { - tmpAttrsTxt += ' class="' + attrValue(attrs[i].value) + '"'; - } - } - } - - // MathML has been through its own allowlist just above, and the - // two vocabularies do not share attributes - lang and dir mean - // nothing on an , and an id there is denied on purpose. - if (mathMLTags.indexOf(tag) < 0) { - tmpAttrsTxt += extraHtmlAttributes(tag, attrs); - } - - // A void tag, or a tag the page self-closed (""), never gets - // an end callback - close it here or the output is unbalanced - if (isVoidTag(tag) || unary) { - lastFragment = '<' + tag + tmpAttrsTxt + '/>'; - } else { - lastFragment = '<' + tag + tmpAttrsTxt + '>'; - openTags.push(tag); - } - - results += lastFragment; - lastFragment = ''; - }, - end: function(tag) { - if (skippedTags.length > 0) { - for (let i = skippedTags.length - 1; i >= 0; i--) { - if (skippedTags[i] === tag) { - skippedTags.length = i; - return; - } - } - // End tag for something never opened inside the dropped content - return; - } - - if (allowedTags.indexOf(tag) < 0 || isVoidTag(tag)) { - return; - } - - for (let i = openTags.length - 1; i >= 0; i--) { - if (openTags[i] === tag) { - // Anything still open inside it was never closed by the page - closeTagsDownTo(i); - return; - } - } - // End tag without a matching start tag - dropped - }, - chars: function(text) { - if (skippedTags.length > 0) { - return; - } - // already decoded by the parser - see attrValue above - results += escapeXMLText(text); - }, - comment: function(text) { - // results += ""; - } - }); - - // Whatever the page left open - closeTagsDownTo(0); - - return results; - - } catch (e) { - console.log('Error:', e); - // Keep the part that was parsed before the error rather than losing the - // whole page, but close it first so the epub stays valid - try { - closeTagsDownTo(0); - } catch (e2) { - console.log('Error:', e2); - } - return results; - } - +// How the shared sanitizer resolves what only a live page can answer: where a +// url points, and which images are being downloaded. What these read and append +// to - imageFileNames, allImages, usedElementIds - is job-wide rather than +// per fragment, which is what lets a selection spanning several ranges come out +// as one chapter with one set of images and no id handed out twice. +// +// The editor answers the same questions without a page - see sanitizeOptions() +// in sanitizeHtml.js, whose defaults are exactly that. +function extractionSanitizeOptions() { + return sanitizeOptions({ + resolveImageSrc: getImageSrc, + resolveLinkHref: linkHref, + resolveUrl: citeUrl, + // the class attribute on a clone is still the page's own; the generated + // name is what applyReadState() wrote into data-class + classAttribute: 'data-class', + usedIds: usedElementIds + }); } // htmlContent is already detached: either a clone of the body or the contents of @@ -1232,7 +804,7 @@ function getContent(htmlContent, state) { // where a stray or - what a selection inside a table produces - // is dropped. Parsed on its own it is parsed "in template", which keeps // table cells intact. - return '
    ' + parseHTML(tmp.innerHTML) + '
    '; + return '
    ' + parseHTML(tmp.innerHTML, extractionSanitizeOptions()) + '
    '; } catch (e) { console.log('Error:', e); // Never hand back the node itself - it ends up spliced into the xhtml page diff --git a/web-extension/sanitizeHtml.js b/web-extension/sanitizeHtml.js new file mode 100644 index 0000000..58b545a --- /dev/null +++ b/web-extension/sanitizeHtml.js @@ -0,0 +1,572 @@ +// The sanitizer and serializer, shared by everything that produces chapter +// content. It used to live inside extractHtml.js and could therefore only run as +// a content script, which made a page of our own - the chapter editor - unable +// to write content the build stage would accept. +// +// The contract: any string that comes out of parseHTML() satisfies the +// invariants the build stage relies on, whatever went in. +// +// - well formed XML: every tag is closed, void tags are self-closed, and tags +// the input left open or closed twice are balanced here +// - every attribute value is double quoted and XML-escaped, which is what lets +// saveEbook.js read the content back with regular expressions (HEADING_REGEX, +// ID_ATTR_REGEX, HREF_ATTR_REGEX) instead of parsing it again +// - only allowlisted tags and attributes survive, so nothing scripts, loads a +// remote resource, or names a vocabulary the package document never declares +// - text carries no entity but the five XML predefines, and no character XML +// 1.0 forbids +// - every id is an XML name and unique within the chapter +// +// It is also idempotent: running its own output through it again returns the +// same string. That is what makes an edit round trip - the editor serializes a +// DOM built from content this produced, and hands it straight back. +// +// Requires utils.js. + +var MATHML_NS = 'http://www.w3.org/1998/Math/MathML'; + +// MathML Core is what every WebKit and Chromium reading system implements - +// Apple Books, Thorium, Kobo, anything built on either engine - and it dropped a +// large part of what MathML 3 allowed. Everything Core removed is deliberately +// missing from this list and rewritten by normalizeMathMl() instead, because +// passing one of them through means a reader draws nothing where the element +// was: +// +// mfenced expanded into the mrow of fences the MathML 3 spec defines +// it to be equivalent to. A Core reader lays out an mfenced's +// children with no brackets at all, so "f(x)" comes out as "fx" - +// and older MathJax output is full of it. +// mlabeledtr renamed to , keeping the label as its first cell. +// Unwrapping it would put s straight into the . +// menclose unwrapped: the box or strikethrough it draws is lost either +// way, and its content is the part worth keeping. +// mstack, msgroup, mlongdiv +// unwrapped. These lay digits out in a grid for long division; +// without the layout the digits are all that is left, but they +// are at least still in reading order. +// mscarries, mscarry +// dropped with their content - a carried digit annotates a +// position in that grid, and outside it it is a stray number in +// the middle of another number. +// +// mprescripts and none are the other direction: both are required by +// mmultiscripts and neither was ever allowed, so a prescripted symbol lost the +// marker saying where its prescripts begin. +var mathMLTags = [ + 'math', 'maction', 'merror', 'mfrac', 'mglyph', 'mi', 'mmultiscripts', 'mn', 'mo', + 'mover', 'mpadded', 'mphantom', 'mprescripts', 'mroot', 'mrow', 'ms', 'mspace', + 'msqrt', 'mstyle', 'msub', 'msup', 'msubsup', 'mtable', 'mtd', 'mtext', 'mtr', + 'munder', 'munderover', 'none', 'semantics' +]; +// blockquote was missing rather than excluded, and it is not a tag whose absence +// shows up as a gap: it is not in strippedContentTags either, so a quotation was +// unwrapped - the tag went, the words stayed - and ran into the paragraph before +// it with nothing to say where the quote started or ended. +var allowedTags = [ + 'address', 'article', 'aside', 'blockquote', 'footer', 'header', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', + 'hgroup', 'nav', 'section', 'dd', 'div', 'dl', 'dt', 'figcaption', 'figure', 'hr', 'li', + 'main', 'ol', 'p', 'pre', 'ul', 'a', 'abbr', 'b', 'bdi', 'bdo', 'br', 'cite', 'code', 'data', + 'dfn', 'em', 'i', 'img', 'kbd', 'mark', 'q', 'rb', 'rp', 'rt', 'rtc', 'ruby', 's', 'samp', 'small', 'span', + 'strong', 'sub', 'sup', 'time', 'u', 'var', 'wbr', 'del', 'ins', 'caption', 'col', 'colgroup', + 'table', 'tbody', 'td', 'tfoot', 'th', 'thead', 'tr' + // TODO ? + // ,'form', 'button' + + // TODO svg support ? + // , 'svg', 'g', 'path', 'line', 'circle', 'text' +].concat(mathMLTags); +// const svgTags = ['svg', 'g', 'path', 'line', 'circle', 'text'] +// Allowed tags that have no content and must be written self-closed, otherwise +// the generated page is not well formed XML +var voidTags = ['br', 'col', 'hr', 'img', 'wbr']; +// Tags that are dropped together with everything inside them. Any other tag +// that is not in allowedTags is unwrapped instead - the tag goes, its text stays. +var strippedContentTags = [ + 'script', 'style', 'noscript', 'template', 'iframe', 'object', 'embed', 'applet', + 'head', 'title', 'meta', 'link', 'base', + 'textarea', 'select', 'option', 'optgroup', 'input', + 'audio', 'video', 'canvas', 'svg', 'map', 'area', + // keeps its presentation MathML and loses its annotations. An + // annotation is an alternative encoding of the same formula - almost always + // the TeX it was typeset from - and a conforming reader renders only the + // first child of , so it is never meant to be seen. But + // was in neither list, so it was unwrapped instead and its raw + // TeX printed as visible text right beside the rendered formula. That is the + // single most visible MathML bug on a KaTeX page. + // + // The TeX itself is not thrown away: normalizeMathMl() lifts it into the + // first, where it is a fallback rather than a duplicate. + 'annotation', 'annotation-xml', + 'mscarries', 'mscarry' +]; + +// ---- attributes ------------------------------------------------------------- +// +// MathML carries its meaning in its attributes at least as much as in its tags. +// display="block" alone is the difference between a centred display equation and +// something jammed into the middle of a sentence; stretchy and largeop size the +// brackets and the integral signs; columnalign is what makes a matrix line up; +// linethickness="0" is how a binomial coefficient is written. Only alttext used +// to survive, so all of that was lost on every formula. +// +// The vocabulary is allowed wholesale rather than enumerated, which is the +// opposite of how the HTML attributes below are handled and deliberately so: +// MathML's is a closed vocabulary defined by a spec, an attribute this extension +// has not heard of is far more likely to be one MathML has than one an attacker +// invented, and a list of thirty names would go out of date silently. What is +// excluded is what is dangerous, or what would break the file: +// +// on* event handlers +// href, src MathML's own link and image attributes. href can hold a +// javascript: url, and src would point at a file that is not +// in the archive. +// style, class the chapter stylesheet is generated from computed styles, +// never copied from the page +// id, data-* an html id is carried over - it is what a link inside the +// page points at - but nothing ever links into a formula, and +// an id here would only be one more name the ids minted while +// the chapter is written have to avoid colliding with +// anything with a colon, or that is not a plain lowercase name. An xmlns: or +// xlink: attribute needs a namespace declaration the chapter documents do not +// make, and an undeclared prefix does not make one formula wrong, it makes the +// whole xhtml file unparseable. +var deniedMathMLAttributes = ['href', 'src', 'style', 'class', 'id', 'xmlns']; + +function isAllowedMathMLAttribute(name) { + return /^[a-z][a-z0-9-]*$/.test(name) && + name.indexOf('on') !== 0 && + name.indexOf('data-') !== 0 && + deniedMathMLAttributes.indexOf(name) < 0; +} + +// HTML attributes are enumerated, where MathML's are allowed wholesale. The +// vocabularies differ in the way that matters: HTML's is huge, mostly about +// behaviour rather than meaning, and full of names that either do nothing in a +// reader (onclick, target, srcset) or actively break the file (style and class, +// which the generated stylesheet owns). +// +// What is here is the subset that changes what the chapter says. Everything on +// this list was previously dropped, and dropping it was not a loss of polish: +// +// colspan, rowspan a merged cell renders as an unmerged one, so every row +// after it is short and the columns stop lining up. This +// is a wrong table, not a less navigable one. +// span same, for a column group +// start, reversed a list that should read 7, 8, 9 renders 1, 2, 3 +// value same, for one item in it +// scope the only cheap way to say which cells a header heads +// lang a chapter is tagged with one language; a quotation in +// another is what a nested lang is for, and a speech +// synthesiser reads it in the wrong voice without it +// dir bdo is on the allowed tag list and does nothing at all +// without dir, so an RTL run inside an LTR paragraph came +// out reversed +// datetime, value what
      and
    1. are the two that may be negative or zero +function attrInteger(value) { + var normalized = value.trim(); + return /^-?[0-9]{1,9}$/.test(normalized) ? String(parseInt(normalized, 10)) : ''; +} + +function attrText(value) { + return value.replace(/\s+/g, ' ').trim(); +} + +// datetime is a format, not free text, and epubcheck validates it: letting a +// page's junk through would turn a tag that carries a date into an error in the +// file. The subset here - a year, a month, a date, any of them with a time and +// an offset, or a bare time - is what pages actually write. The exotic +// remainder of the html grammar (weeks, durations, yearless dates) is dropped +// rather than guessed at, which costs the attribute and keeps the element. +var DATETIME_REGEX = new RegExp( + '^(' + + '[0-9]{4}(-[0-9]{2}(-[0-9]{2})?)?' + + '([T ][0-9]{2}:[0-9]{2}(:[0-9]{2}(\\.[0-9]{1,3})?)?' + + '(Z|[+-][0-9]{2}:?[0-9]{2})?)?' + + '|' + + '[0-9]{2}:[0-9]{2}(:[0-9]{2}(\\.[0-9]{1,3})?)?' + + ')$'); + +function attrDateTime(value) { + var normalized = value.trim(); + return DATETIME_REGEX.test(normalized) ? normalized : ''; +} + +// A url attribute that is not the one being followed for content -
      and point at a source, and nothing in the archive depends on +// them resolving, but they are still urls a reader may open. +function attrUrl(value, options) { + return options.resolveUrl(value.trim()); +} + +// Valid on every element, so they are checked before the per-tag table +var globalHtmlAttributes = { + lang: normalizeLanguageTag, + dir: attrEnum(['ltr', 'rtl', 'auto']) +}; + +var htmlAttributes = { + a: {}, + abbr: {title: attrText}, + blockquote: {cite: attrUrl}, + q: {cite: attrUrl}, + del: {cite: attrUrl, datetime: attrDateTime}, + ins: {cite: attrUrl, datetime: attrDateTime}, + time: {datetime: attrDateTime}, + data: {value: attrText}, + // A boolean attribute is true because it is there, whatever it says - and + // XHTML has no bare form, so it is written back out in the long one + ol: {start: attrInteger, reversed: function() { return 'reversed'; }}, + li: {value: attrInteger}, + col: {span: attrCount(1)}, + colgroup: {span: attrCount(1)}, + // rowspan="0" is legal and means "to the end of the section"; colspan="0" is + // not, which is why the two have different floors + td: {colspan: attrCount(1), rowspan: attrCount(0)}, + th: {colspan: attrCount(1), rowspan: attrCount(0), scope: attrEnum(['row', 'col', 'rowgroup', 'colgroup'])} +}; + +// walkHtmlFragment hands over values the HTML parser has already decoded, so +// they only need escaping - decoding here as well would turn a literal +// "&lt;" on the page into a "<" in the epub +function attrValue(value) { + return escapeXMLChars(value == null ? '' : String(value)); +} + +// The id, the globals and whatever the tag itself allows. Attributes the caller +// has already written - src, alt, href, class and the img dimensions - are not +// in any of these tables and cannot be duplicated from here. +function extraHtmlAttributes(tag, attrs, options) { + let result = ''; + let perTag = htmlAttributes[tag] || {}; + // A page is free to write the same attribute twice. The parser hands both + // over, and writing both would make the chapter unparseable. + let written = new Set(); + + for (let i = 0; i < attrs.length; i++) { + let name = String(attrs[i].name).toLowerCase(); + let rawValue = attrs[i].value == null ? '' : String(attrs[i].value); + + if (written.has(name)) { + continue; + } + + if (name === 'id') { + if (!isUsableId(rawValue) || options.usedIds.has(rawValue)) { + continue; + } + options.usedIds.add(rawValue); + written.add(name); + result += ' id="' + attrValue(rawValue) + '"'; + continue; + } + + let filter = globalHtmlAttributes[name] || perTag[name]; + if (!filter) { + continue; + } + let value = filter(rawValue, options); + if (value === '') { + continue; + } + written.add(name); + result += ' ' + name + '="' + attrValue(value) + '"'; + } + + return result; +} + +// ---- where urls and images resolve ------------------------------------------ +// +// The only part of sanitizing that depends on where the content came from. +// Extraction has a live page to resolve relative references against and images +// it is about to download; the editor page has neither - what it hands back is +// content this module already wrote once, edited in place. So the defaults below +// are the editor's: keep what is already resolved, drop what cannot be. + +// An image reference that names a file in the archive, as getImageSrc() writes +// it. Anything else - a url the user pasted in, a leftover reference from a +// chapter buffered by an older version - would point at a resource the book does +// not contain, which fails the whole book rather than one picture. +var ARCHIVE_IMAGE_SRC_REGEX = /^(?:\.{1,2}\/)*images\/[^/?#]+$/i; + +function keepArchiveImageSrc(value) { + let text = String(value == null ? '' : value).trim(); + return ARCHIVE_IMAGE_SRC_REGEX.test(text) ? text : ''; +} + +// A url that is already absolute is kept as it stands. A relative one has +// nothing to resolve against here - the editor page's own address is certainly +// not it - so it loses its href and keeps its text. +function keepAbsoluteUrl(value) { + let text = String(value == null ? '' : value).trim(); + if (text === '' || !isSafeLinkUrl(text)) { + return ''; + } + try { + return new URL(text).href; + } catch (e) { + return ''; + } +} + +// ...and a link into the chapter itself stays one. resolveInternalLinks() drops +// the ones whose target did not survive, at build time, when the ids are final. +function keepKnownHref(value) { + let text = String(value == null ? '' : value).trim(); + if (text.indexOf('#') === 0) { + return isUsableId(text.substring(1)) ? text : ''; + } + return keepAbsoluteUrl(text); +} + +function sanitizeOptions(overrides) { + let options = { + // an as the page wrote it -> the src to write, or '' to drop + // the image along with its tag + resolveImageSrc: keepArchiveImageSrc, + // an -> the href to write, or '' to keep the text and drop the + // link + resolveLinkHref: keepKnownHref, + // every other url-valued attribute - see attrUrl + resolveUrl: keepAbsoluteUrl, + // Which attribute carries the generated css class name. Extraction marks + // the clone with data-class, because a class attribute at that point is + // still the page's own and must not survive; by the time the content is + // written the generated name is what the class attribute holds, so + // re-sanitizing reads it from there. + classAttribute: 'class', + // Ids already written into this chapter, so that a page repeating one - + // invalid html, but common - does not repeat it here. Shared across + // calls by a caller producing one chapter from several fragments. + usedIds: new Set() + }; + if (overrides) { + Object.keys(overrides).forEach(function(key) { + if (overrides[key] !== undefined && overrides[key] !== null) { + options[key] = overrides[key]; + } + }); + } + return options; +} + +// ---- the sanitizer ---------------------------------------------------------- + +function parseHTML(rawContentString, options) { + options = options || sanitizeOptions(); + let results = ''; + let lastFragment = ''; + // Tags written to the output that still need a closing tag. Kept here rather + // than trusting the input to be balanced: a page can close tags it never + // opened, or leave tags open, and the epub still has to be valid XML. + let openTags = []; + // Tags whose content is being dropped, innermost last + let skippedTags = []; + + let isVoidTag = (tag) => voidTags.indexOf(tag) > -1; + + let closeTagsDownTo = (index) => { + for (let i = openTags.length - 1; i >= index; i--) { + results += ''; + } + openTags.length = index; + }; + + try { + walkHtmlFragment(rawContentString, { + start: function(tag, attrs, unary) { + if (skippedTags.length > 0) { + // Inside dropped content - track nesting so that the matching + // end tag, and only it, ends the skip + if (!unary && !isVoidTag(tag)) { + skippedTags.push(tag); + } + return; + } + + if (allowedTags.indexOf(tag) < 0) { + if (strippedContentTags.indexOf(tag) > -1 && !unary && !isVoidTag(tag)) { + skippedTags.push(tag); + } + return; + } + + let tmpAttrsTxt = ''; + + if (tag === 'img') { + let tmpSrc = '' + // null and '' are different answers: null is a source that + // never said anything about this image, '' is one that + // deliberately marked it decorative. Collapsing them - which + // is what writing alt="" on everything did - turns every + // undescribed image into a false claim that it needs no + // description, and the accessibility metadata is computed + // from exactly this distinction. + let tmpAlt = null; + for (let i = 0; i < attrs.length; i++) { + if (attrs[i].name === 'src') { + tmpSrc = options.resolveImageSrc(attrs[i].value) + tmpAttrsTxt += ' src="' + attrValue(tmpSrc) + '"'; + } else if (attrs[i].name === 'alt') { + tmpAlt = attrs[i].value == null ? '' : String(attrs[i].value); + } else if (attrs[i].name === options.classAttribute) { + tmpAttrsTxt += ' class="' + attrValue(attrs[i].value) + '"'; + } else if (attrs[i].name === 'width') { + // used when converting svg to img - the result image was too big + tmpAttrsTxt += ' width="' + attrValue(attrs[i].value) + '"'; + } else if (attrs[i].name === 'height') { + // used when converting svg to img - the result image was too big + tmpAttrsTxt += ' height="' + attrValue(attrs[i].value) + '"'; + } + } + if (tmpSrc === '') { + // ignore imgs without source + return; + } + if (tmpAlt !== null) { + tmpAttrsTxt += ' alt="' + attrValue(tmpAlt.replace(/\s+/g, ' ').trim()) + '"'; + } + } else if (tag === 'a') { + for (let i = 0; i < attrs.length; i++) { + if (attrs[i].name === 'href') { + let href = options.resolveLinkHref(attrs[i].value); + if (href !== '') { + tmpAttrsTxt += ' href="' + attrValue(href) + '"'; + } + } else if (attrs[i].name === options.classAttribute) { + tmpAttrsTxt += ' class="' + attrValue(attrs[i].value) + '"'; + } + } + } else if (mathMLTags.indexOf(tag) > -1) { + if (tag === 'math') { + // in xhtml, MathML is only MathML because of this + tmpAttrsTxt += ' xmlns="' + MATHML_NS + '"'; + } + for (let i = 0; i < attrs.length; i++) { + if (isAllowedMathMLAttribute(attrs[i].name)) { + tmpAttrsTxt += ' ' + attrs[i].name + '="' + attrValue(attrs[i].value) + '"'; + } + } + } else { + for (let i = 0; i < attrs.length; i++) { + if (attrs[i].name === options.classAttribute) { + tmpAttrsTxt += ' class="' + attrValue(attrs[i].value) + '"'; + } + } + } + + // MathML has been through its own allowlist just above, and the + // two vocabularies do not share attributes - lang and dir mean + // nothing on an , and an id there is denied on purpose. + if (mathMLTags.indexOf(tag) < 0) { + tmpAttrsTxt += extraHtmlAttributes(tag, attrs, options); + } + + // A void tag, or a tag the page self-closed (""), never gets + // an end callback - close it here or the output is unbalanced + if (isVoidTag(tag) || unary) { + lastFragment = '<' + tag + tmpAttrsTxt + '/>'; + } else { + lastFragment = '<' + tag + tmpAttrsTxt + '>'; + openTags.push(tag); + } + + results += lastFragment; + lastFragment = ''; + }, + end: function(tag) { + if (skippedTags.length > 0) { + for (let i = skippedTags.length - 1; i >= 0; i--) { + if (skippedTags[i] === tag) { + skippedTags.length = i; + return; + } + } + // End tag for something never opened inside the dropped content + return; + } + + if (allowedTags.indexOf(tag) < 0 || isVoidTag(tag)) { + return; + } + + for (let i = openTags.length - 1; i >= 0; i--) { + if (openTags[i] === tag) { + // Anything still open inside it was never closed by the page + closeTagsDownTo(i); + return; + } + } + // End tag without a matching start tag - dropped + }, + chars: function(text) { + if (skippedTags.length > 0) { + return; + } + // already decoded by the parser - see attrValue above + results += escapeXMLText(text); + }, + comment: function(text) { + // results += ""; + } + }); + + // Whatever the page left open + closeTagsDownTo(0); + + return results; + + } catch (e) { + console.log('Error:', e); + // Keep the part that was parsed before the error rather than losing the + // whole page, but close it first so the epub stays valid + try { + closeTagsDownTo(0); + } catch (e2) { + console.log('Error:', e2); + } + return results; + } + +} + +// The entry point for a caller holding a chapter that has already been through +// extraction - the editor, after the user has changed something. The result is +// safe to store as page.content and to hand to the build stage. +function sanitizeChapterContent(content) { + return parseHTML(String(content == null ? '' : content), sanitizeOptions()); +} diff --git a/web-extension/saveEbook.js b/web-extension/saveEbook.js index b340bd7..7a1a46e 100644 --- a/web-extension/saveEbook.js +++ b/web-extension/saveEbook.js @@ -1,8 +1,83 @@ var cssFileName = 'ebook.css'; -var ebookTitle = null; -// Set by buildEbookFromChapters() from storage. Left null on the one-shot -// "save this page" path, which has no stored identity - see getBookId(). -var ebookUuid = null; + +// ---- the stylesheets the archive carries ------------------------------------- +// +// Two kinds of css reach a book, and only one of them is filtered by content. +// What extraction captures is scraped off somebody else's page, and supportedCss +// in extractHtml.js decides which of its declarations are worth carrying. What a +// user writes in the chapter editor is not that: it is theirs, it is meant to +// change how the book looks, and holding it to an allowlist would only mean most +// of what they typed silently did nothing. Nothing they can write in it makes the +// package invalid either - css a reading system does not understand is css it +// ignores. +// +// What this removes is neither of those judgements. It is the two ways a +// stylesheet reaches outside the archive, which both kinds can do: +// +// @import pulls in another stylesheet, which for a remote address is a +// network fetch from inside a file that is supposed to read +// offline, and for a local one names a file the archive has no +// way to contain. +// remote url() a background image or a font fetched off the network. EPUB +// requires a manifest item referencing a remote resource to say +// so (properties="remote-resources"), and this build makes no +// such claim about any file it writes - so rather than have the +// claim and the content disagree, the reference goes. Captured +// css hits this as well: 'list-style' serializes an absolute +// url whenever the page gave its bullets a picture. +// +// A url() that is not remote is left alone: "../images/photo.jpg" from a chapter +// stylesheet resolves to a picture the book really does contain, and using one as +// a background is a reasonable thing to want. data: urls stay for the same +// reason - they are carried in the file rather than fetched. + +// A url() target that leaves the archive: anything with a scheme, plus the +// scheme-relative "//host/path" form. data: is not one of these - it is the +// resource itself, not an address to go and get it from. +function isRemoteCssUrl(target) { + var value = String(target || '').trim(); + if (/^data:/i.test(value)) { + return false; + } + return /^(?:[a-z][a-z0-9+.\-]*:|\/\/)/i.test(value); +} + +// An @import rule runs to the first semicolon; a quoted string inside it may +// contain one, so strings are matched rather than scanned through. A rule left +// unterminated at the end of the file, or at the end of the block it sits in, +// ends there. +var CSS_IMPORT_REGEX = /@import\b(?:[^;{}'"]|'[^']*'|"[^"]*")*(?:;|(?=\})|$)/gi; +var CSS_URL_REGEX = /\burl\(\s*(?:"([^"]*)"|'([^']*)'|([^)"'\s]*))\s*\)/gi; + +// What a removed url() becomes. Dropping the token outright would leave +// "background-image: ;", which a parser discards along with any fallback beside +// it; 'none' is a value the properties that take a url actually accept, and +// where it is not one the declaration is dropped by css error recovery, which is +// the same outcome. +var REMOVED_CSS_URL = 'none'; + +function sanitizeStylesheet(css) { + return String(css == null ? '' : css) + .replace(CSS_IMPORT_REGEX, '') + .replace(CSS_URL_REGEX, function(rule, quoted, single, bare) { + var target = quoted !== undefined ? quoted : + single !== undefined ? single : bare; + return isRemoteCssUrl(target) ? REMOVED_CSS_URL : rule; + }); +} + +// The stylesheet a chapter is written with: what extraction captured from the +// page, then what the user added in the editor. In that order, so that a rule +// the user writes wins over the scraped one it collides with - which is the +// whole point of being able to write one. +function chapterStyleContent(page) { + var captured = typeof page.styleFileContent === 'string' ? page.styleFileContent : ''; + var authored = typeof page.customCss === 'string' ? page.customCss : ''; + if (authored.trim() === '') { + return sanitizeStylesheet(captured); + } + return sanitizeStylesheet(captured === '' ? authored : captured + '\n' + authored); +} // Compression is set per file, never globally: OCF requires 'mimetype' to be // the first entry and STORE-d, so readers can sniff the magic bytes. @@ -63,7 +138,12 @@ function normalizeChapters(allPages) { images: Array.isArray(page.images) ? page.images : [], styleFileName: typeof page.styleFileName === 'string' && page.styleFileName ? page.styleFileName : 'style' + index + '.css', - styleFileContent: typeof page.styleFileContent === 'string' ? page.styleFileContent : '' + styleFileContent: typeof page.styleFileContent === 'string' ? page.styleFileContent : '', + // kept beside the captured stylesheet rather than merged into it, so + // that reopening the editor shows the user what they wrote and not + // the page's computed styles with their rules buried at the bottom. + // chapterStyleContent() is where the two become one file. + customCss: typeof page.customCss === 'string' ? page.customCss : '' })); }).map(dropMissingImages); } @@ -156,8 +236,8 @@ function getImagesIndex(allImages) { // so that rebuilding after an edit updates the same book in a library instead of // adding a second copy. A single page saved straight to disk was never stored // and is a new book every time, so it gets a fresh id. -function getBookId() { - return 'urn:uuid:' + (ebookUuid || generateUuid()); +function getBookId(uuid) { + return 'urn:uuid:' + (uuid || generateUuid()); } // dc:source, one per chapter. sourceUrl was added after baseUrl already existed; @@ -812,29 +892,39 @@ function getExternalLinksIndex() { // TODO }, ''); } -function buildEbookFromChapters() { +// Reads the whole book out of storage and builds it. For a caller that has no +// chapters in hand - everything else passes its own array to buildEbook(), which +// is what keeps a build from racing the storage write that preceded it. +function buildEbookFromStorage() { getEbookTitle(function (title) { - ebookTitle = title; - if (!ebookTitle || ebookTitle.trim().length === 0) { - ebookTitle = 'eBook'; - } getEbookUuid(function (uuid) { - ebookUuid = uuid; - getEbookPages(_buildEbook); + getBookCss(function (css) { + getEbookPages(function (allPages) { + buildEbook(allPages, {title: title, uuid: uuid, css: css}); + }); + }) }) }) } -// FIXME remove - keep one function -function buildEbook(allPages, fromMenu=false) { - // the one-shot "save this page" path: nothing about it was stored, so it is - // a new book every time and must not inherit an id from a chapter build - ebookUuid = null; - _buildEbook(allPages, fromMenu); -} - +// The one way an ebook is built - the one-shot "save this page" and the editor's +// "generate" differ only in what they pass here. Everything that is not +// derivable from the chapters themselves arrives in bookMeta: +// +// title what the book is called. Absent, the first chapter names it, which is +// the right answer for a single saved page. +// uuid dc:identifier. Absent, a fresh one is minted: the one-shot path +// stored nothing, so it is a new book every time and must not inherit +// an id from a chapter build - see getBookId(). +// css the book-wide stylesheet, written by the user in the chapter editor. +// Absent, ebook.css is written empty, which is what it was for every +// book built before there was anywhere to type one. +// // http://ebooks.stackexchange.com/questions/1183/what-is-the-minimum-required-content-for-a-valid-epub -function _buildEbook(allPages, fromMenu=false) { +function buildEbook(allPages, bookMeta) { + bookMeta = bookMeta || {}; + var ebookTitle = typeof bookMeta.title === 'string' && bookMeta.title.trim().length > 0 ? + bookMeta.title : null; allPages = normalizeChapters(allPages); if (allPages.length === 0) { finishJob('There are no valid chapters to save.'); @@ -845,6 +935,7 @@ function _buildEbook(allPages, fromMenu=false) { console.log('Prepare Content...'); var ebookFileName = 'eBook.epub'; + var ebookName = ''; if (ebookTitle) { // ~TODO a pre-processing function to apply escapeXMLChars to all page.titles @@ -858,7 +949,7 @@ function _buildEbook(allPages, fromMenu=false) { // One instant for the whole package: dc:date (when this file was made) and // dcterms:modified disagreeing by a second reads as two separate events. var buildDate = new Date().toISOString().replace(/\.[0-9]+Z/i, 'Z'); - var bookId = getBookId(); + var bookId = getBookId(bookMeta.uuid); var bookLanguage = getBookLanguage(allPages); var bookDirection = getBookDirection(allPages); var publishers = collectDistinct(allPages, function(m) { return m.publisher; }); @@ -965,10 +1056,13 @@ function _buildEbook(allPages, fromMenu=false) { DEFLATED ); - oebps.file(cssFileName, '', DEFLATED); //TODO + // The book-wide stylesheet: written by hand in the chapter editor, empty for + // every book whose author never opened that box. Every chapter links it, and + // so does the table of contents. + oebps.file(cssFileName, sanitizeStylesheet(bookMeta.css), DEFLATED); var styleFolder = oebps.folder('style'); allPages.forEach(function(page) { - styleFolder.file(page.styleFileName, page.styleFileContent, DEFLATED); + styleFolder.file(page.styleFileName, chapterStyleContent(page), DEFLATED); }); var pagesFolder = oebps.folder('pages'); @@ -989,6 +1083,12 @@ function _buildEbook(allPages, fromMenu=false) { dirAttribute(pageDirection) + '>' + '' + '' + tmpPageTitle+ '' + + // In this order, and this is the cascade the preview reproduces: the + // book-wide stylesheet states what the book looks like, the chapter's + // own - the source page's computed styles, plus whatever the user + // added for this chapter alone - is the more specific answer and + // comes second. + '' + '' + '' + getChapterBody(page, outlines[index]) + diff --git a/web-extension/utils.js b/web-extension/utils.js index 524f41b..a27de7f 100644 --- a/web-extension/utils.js +++ b/web-extension/utils.js @@ -62,6 +62,27 @@ function saveEbookTitle(title) { }); } +// The book-wide stylesheet, kept beside the title and the identifier because it +// belongs to the book being assembled rather than to any one chapter - and is +// discarded with them. Not to be confused with the per-site styles behind +// getStyles(): those are applied to a page while it is being captured and +// outlive any single book. +function getBookCss(callback) { + chrome.runtime.sendMessage({ + type: "get book css" + }, function(response) { + callback(response && typeof response.css === 'string' ? response.css : ''); + }); +} + +function saveBookCss(css) { + chrome.runtime.sendMessage({ + type: "set book css", + css: css + }, function(response) { + }); +} + // The identifier of the ebook being assembled from chapters. Minted once by the // background and kept next to the chapters, because dc:identifier is what a // library uses to decide whether a file is a new book or a newer copy of one it