Make KaTeX work in Quirks mode (#608)

* Make KaTeX work in Quirks mode

Summary: In issue #601, it was noticed that the KaTeX bug with the fraction
bars overlapping the text was occuring with an XHTML doctype. This indicated
that the bug we were seeing was caused by both quirks mode and limited-quirks
mode (which is a version of quirks mode with fewer quirks and is enabled for
various doctypes including some XHTML ones).

Based on the [quirks spec](https://quirks.spec.whatwg.org/), it appears that
there are only two quirks in limited-quirks mode, both having to do with a
line-height calculation. @gagern figured out that if we added some zero-width
spaces in our elements, we would stop triggering the quirk, which would make
our fractions render correctly in limited-quirks mode.

I implemented that change, and ran the screenshotter in limited-quirks mode.
There were several other places that suffered from the same quirk, but were
also easily fixed via adding zero-width spaces. I then ran the screenshotter in
quirks mode, and discovered that (once an appropriate meta charset was added),
everything looked correct still.

So, this diff fixes all of the places that the limited-quirks mode quirks
affect our rendering, and removes the warning about rendering in quirks mode.

I also added support to our screenshotter to render things in both no-quirks
and quirks mode, to ensure that things don't break in the future. I copied the
non-quirks images to the quirks images, and ran the screenshotter with
`--verify` to make sure that they look the same.

I have some thoughts that I'd like to hear opinions about:
 - I'm not super happy with how the screenshot tests work. Ideally we'd test
   both quirks mode and non-quirks mode against the same images, since we'd
   like them to be the same. I'm not sure how to make that work well, though,
   since then people wouldn't be able to tell if it's a quirks-mode problem or
   not.
 - I removed the doctype in the testing page file, so all testing would now be
   done in quirks-mode. Not sure if we really want that.
 - I need to test this in IE, but it looks like the trailing commas change we
   made with eslinting is causing problems (cause IE doesn't like trailing
   commas).

Test Plan:
 - `./dockers/Screenshotter/screenshotter.sh --verify`

* Compare quirks mode against same screenshot files

Now the screenshotter itself can run more than one mode.  It does serve the
HTML file from its own JavaScript code now, so that it can include different
doctype headers without needing distinct files for each.  There is a
provision to mark specific tests as quirky in case they produce different
results depending on the mode.

* Some cleaning up and comments

* Restore access to the babelified version of the HTML page for screenshots

* Reference unicode fonts using absolute path names

This avoids issues caused by the fact that the dynamically generated
ss-render.html is mounted to a different location than the test.html from
which it is derived.

* do chrome screenshots first

* remove commented out code, simplify hadle_search_string call

dockers/Screenshotter/README.md

@@ -21,7 +21,7 @@ docker containers with the selenium setups running.  Essentially you
 are encouraged to reproduce the steps from `screenshotter.sh`
 manually.  Example run for Firefox:
 
-    container=$(docker run -d -P selenium/standalone-firefox:2.46.0)
+    container=$(docker run -d -P selenium/standalone-firefox:2.48.2)
     node dockers/Screenshotter/screenshotter.js -b firefox -c ${container}
     # possibly repeat the above command as often as you need, then eventually
     docker stop ${container}

dockers/Screenshotter/screenshotter.js

@@ -86,6 +86,14 @@ const opts = require("nomnom")
     .option("wait", {
         help: "Wait this many seconds between page load and screenshot",
     })
+    .option("mode", {
+        choices: ["no-quirks", "limited-quirks", "quirks", "all"],
+        list: true,
+        help: "Render mode, determined by doctype, " +
+            "may be given multiple times, " +
+            "values [no-quirks|limited-quirks|quirks|all], " +
+            "defaults to all for --verify or no-quirks otherwise",
+    })
     .parse();
 
 let listOfCases;
@@ -101,6 +109,17 @@ if (opts.exclude) {
     });
 }
 
+let modes = opts.mode || [];
+if (!modes.length) {
+    modes = [opts.verify ? "all" : "no-quirks"];
+}
+if (modes.indexOf("all") !== -1) {
+    modes = ["no-quirks", "limited-quirks", "quirks"];
+}
+if (modes.length > 1 && !opts.verify) {
+    modes.length = 1;
+}
+
 let seleniumURL = opts.seleniumURL;
 let seleniumIP = opts.seleniumIP;
 let seleniumPort = opts.seleniumPort;
@@ -358,16 +377,41 @@ function findHostIP() {
 //////////////////////////////////////////////////////////////////////
 // Take the screenshots
 
-let countdown = listOfCases.length;
+let countdown = listOfCases.length * modes.length;
 
 let exitStatus = 0;
 const listOfFailed = [];
 
+const doctypes = {
+    "no-quirks": "<!DOCTYPE html>",
+    "limited-quirks": '<!DOCTYPE HTML PUBLIC ' +
+        '"-//W3C//DTD HTML 4.01 Transitional//EN" ' +
+        '"http://www.w3.org/TR/html4/loose.dtd">',
+    "quirks": '<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Draft//">',
+};
+
+// Use two characters per abbreviation for better alignment of output
+const qabbr = {
+    "no-quirks": "nq",
+    "limited-quirks": "lq",
+    "quirks": " q",
+};
+
 function takeScreenshots() {
-    listOfCases.forEach(takeScreenshot);
+    const html = fs.readFileSync(require.resolve(
+        "../../test/screenshotter/test.html"));
+    function handler(req, res, next) {
+        res.send(doctypes[req.query.mode] + "\n" + html);
+    }
+    app.get("/ss-render.html", handler);
+    modes.forEach(function(mode) {
+        listOfCases.forEach(function(key) {
+            takeScreenshot(key, mode);
+        });
+    });
 }
 
-function takeScreenshot(key) {
+function takeScreenshot(key, mode) {
     const itm = data[key];
     if (!itm) {
         console.error("Test case " + key + " not known!");
@@ -379,14 +423,19 @@ function takeScreenshot(key) {
         return;
     }
 
-    let file = path.join(dstDir, key + "-" + opts.browser + ".png");
+    let basename = key + "-" + opts.browser;
+    if (itm.quirky && itm.quirky.indexOf(mode) !== -1) {
+        // a test case known to differ depending on mode may make use of this
+        basename += "-" + qabbr[mode];
+    }
+    let file = path.join(dstDir, basename + ".png");
     let retry = 0;
     let loadExpected = null;
     if (opts.verify) {
         loadExpected = promisify(fs.readFile, file);
     }
 
-    const url = katexURL + "test/screenshotter/test.html?" + itm.query;
+    const url = katexURL + "ss-render.html?mode=" + mode + "&" + itm.query;
     driver.call(loadMath);
 
     function loadMath() {
@@ -428,8 +477,9 @@ function takeScreenshot(key) {
              * the other has something else.  By using a different
              * output file name for one of these cases, we accept both.
              */
+            basename = basename.replace(key, key + "_alt");
             key += "_alt";
-            file = path.join(dstDir, key + "-" + opts.browser + ".png");
+            file = path.join(dstDir, basename + ".png");
             if (loadExpected) {
                 loadExpected = promisify(fs.readFile, file);
             }
@@ -438,18 +488,19 @@ function takeScreenshot(key) {
             pako: pako,
         });
         const buf = opt.bufferSync(img.buf);
+        const line = qabbr[mode] + " " + key;
         if (loadExpected) {
             return loadExpected.then(function(expected) {
                 if (!buf.equals(expected)) {
                     if (++retry >= opts.attempts) {
-                        console.error("FAIL! " + key);
+                        console.error("FAIL! " + line);
-                        listOfFailed.push(key);
+                        listOfFailed.push(key + "[" + qabbr[mode] + "]");
                         exitStatus = 3;
                         if (opts.diff) {
                             return saveScreenshotDiff(key, buf);
                         }
                     } else {
-                        console.log("error " + key);
+                        console.log("error " + line);
                         browserSideWait(300 * retry);
                         if (retry > 1) {
                             driverReady = false; // reload fully
@@ -457,12 +508,12 @@ function takeScreenshot(key) {
                         return driver.call(loadMath);
                     }
                 } else {
-                    console.log("* ok  " + key);
+                    console.log("* ok  " + line);
                 }
             });
         } else {
             return promisify(fs.writeFile, file, buf).then(function() {
-                console.log(key);
+                console.log(line);
             });
         }
     }

dockers/Screenshotter/screenshotter.sh

@@ -29,7 +29,7 @@ popd || exit 2
 container=
 trap cleanup EXIT
 status=0
-for browserTag in firefox:2.48.2 chrome:2.48.2; do
+for browserTag in chrome:2.48.2 firefox:2.48.2; do
     browser=${browserTag%:*}
     image=selenium/standalone-${browserTag}
     echo "Starting container for ${image}"

katex.js

@@ -18,7 +18,7 @@ import utils from "./src/utils";
  * Parse and build an expression, and place that expression in the DOM node
  * given.
  */
-let render = function(expression, baseNode, options) {
+const render = function(expression, baseNode, options) {
     utils.clearNode(baseNode);
 
     const settings = new Settings(options);
@@ -29,20 +29,6 @@ let render = function(expression, baseNode, options) {
     baseNode.appendChild(node);
 };
 
-// KaTeX's styles don't work properly in quirks mode. Print out an error, and
-// disable rendering.
-if (typeof document !== "undefined") {
-    if (document.compatMode !== "CSS1Compat") {
-        typeof console !== "undefined" && console.warn(
-            "Warning: KaTeX doesn't work in quirks mode. Make sure your " +
-                "website has a suitable doctype.");
-
-        render = function() {
-            throw new ParseError("KaTeX doesn't work in quirks mode.");
-        };
-    }
-}
-
 /**
  * Parse and build an expression, and return the markup for that.
  */

src/buildCommon.js

@@ -347,7 +347,8 @@ const makeVList = function(children, positionType, positionData, options) {
         } else {
             const child = children[i].elem;
 
-            const childWrap = makeSpan([], [pstrut, child]);
+            const childWrap = makeSpan(
+                [], [pstrut, child, new domTree.symbolNode("\u200b")]);
             childWrap.style.top = (-pstrutSize - currPos - child.depth) + "em";
             if (children[i].marginLeft) {
                 childWrap.style.marginLeft = children[i].marginLeft;

src/buildHTML.js

@@ -761,16 +761,20 @@ groupTypes.spacing = function(group, options) {
 
 groupTypes.llap = function(group, options) {
     const inner = makeSpan(
-        ["inner"], [buildGroup(group.value.body, options)]);
+        ["inner"], [
-    const fix = makeSpan(["fix"], []);
+            buildGroup(group.value.body, options),
+            new domTree.symbolNode("\u200b")]);
+    const fix = makeSpan(["fix"], [new domTree.symbolNode("\u200b")]);
     return makeSpan(
         ["mord", "llap"], [inner, fix], options);
 };
 
 groupTypes.rlap = function(group, options) {
     const inner = makeSpan(
-        ["inner"], [buildGroup(group.value.body, options)]);
+        ["inner"], [
-    const fix = makeSpan(["fix"], []);
+            buildGroup(group.value.body, options),
+            new domTree.symbolNode("\u200b")]);
+    const fix = makeSpan(["fix"], [new domTree.symbolNode("\u200b")]);
     return makeSpan(
         ["mord", "rlap"], [inner, fix], options);
 };
@@ -1739,7 +1743,10 @@ const buildHTML = function(tree, options) {
     bottomStrut.style.verticalAlign = -body.depth + "em";
 
     // Wrap the struts and body together
-    const htmlNode = makeSpan(["katex-html"], [topStrut, bottomStrut, body]);
+    const htmlNode = makeSpan(["katex-html"], [
+        topStrut, bottomStrut, body,
+        // Quirks mode fix
+        new domTree.symbolNode("\u200b")]);
 
     htmlNode.setAttribute("aria-hidden", "true");
 

static/index.html

@@ -1,4 +1,3 @@
-<!DOCTYPE html>
 <html>
     <head>
         <meta charset="UTF-8">

test/screenshotter/test.html

@@ -1,9 +1,9 @@
-<!doctype html>
 <html>
   <head>
+    <meta http-equiv="content-type" content="text/html; charset=UTF-8">
     <title>Screenshotter test</title>
-    <script src="../../katex.js" type="text/javascript"></script>
+    <script src="katex.js" type="text/javascript"></script>
-    <link href="../../katex.css" rel="stylesheet" type="text/css">
+    <link href="katex.css" rel="stylesheet" type="text/css">
     <style type="text/css">
       #math, #pre, #post {
         font-size: 4em;
@@ -13,11 +13,11 @@
       }
       @font-face {
         font-family: "Mincho";
-        src: url("unicode-fonts/mincho/font_1_honokamin.ttf") format("truetype");
+        src: url("/test/screenshotter/unicode-fonts/mincho/font_1_honokamin.ttf") format("truetype");
       }
       @font-face {
         font-family: "Batang";
-        src: url("unicode-fonts/batang/batang.ttf") format("truetype");
+        src: url("/test/screenshotter/unicode-fonts/batang/batang.ttf") format("truetype");
       }
       .katex .cjk_fallback {
           font-family: "Mincho",serif;