Fix MathML output for ' and large operators with limits (#788)

Summary: (@kevinbarabash)
This diff extracts MathML builder tests out into a separate file and
updates them to use jest snapshot testing.  It updates the output of
prime ' from using identifier nodes <mi> to operator nodes <mo>.  It
also updates large operators with limits to use munderover instead
of msupsub. I added an option to remove unnecessary mrows to buildGroup.
Right now it's only used for by groupTypes.supsub. I'll see if it can be used
elsewhere (everywhere?) in a follow up PR.

Test Plan:
- make test w/o errors
- verify mathml snapshots contain the desired markup

package.json

@@ -28,6 +28,7 @@
     "express": "^4.14.0",
     "glob": "^7.1.1",
     "jest": "^20.0.4",
+    "jest-serializer-html": "^4.0.0",
     "js-yaml": "^3.3.1",
     "jspngopt": "^0.2.0",
     "less": "~2.7.1",
@@ -48,6 +49,9 @@
     "match-at": "^0.1.0"
   },
   "jest": {
+    "snapshotSerializers": [
+      "jest-serializer-html"
+    ],
     "testMatch": [
       "**/test/*-spec.js"
     ],

src/buildMathML.js

@@ -8,6 +8,7 @@ import buildCommon, { makeSpan, fontMap } from "./buildCommon";
 import fontMetrics from "./fontMetrics";
 import mathMLTree from "./mathMLTree";
 import ParseError from "./ParseError";
+import Style from "./Style";
 import symbols from "./symbols";
 import utils from "./utils";
 import stretchy from "./stretchy";
@@ -91,6 +92,8 @@ groupTypes.textord = function(group, options) {
         // TODO(kevinb) merge adjacent <mn> nodes
         // do it as a post processing step
         node = new mathMLTree.MathNode("mn", [text]);
+    } else if (group.value === "\\prime") {
+        node = new mathMLTree.MathNode("mo", [text]);
     } else {
         node = new mathMLTree.MathNode("mi", [text]);
     }
@@ -207,14 +210,18 @@ groupTypes.supsub = function(group, options) {
         }
     }
 
-    const children = [buildGroup(group.value.base, options)];
+    const removeUnnecessaryRow = true;
+    const children = [
+        buildGroup(group.value.base, options, removeUnnecessaryRow)];
 
     if (group.value.sub) {
-        children.push(buildGroup(group.value.sub, options));
+        children.push(
+            buildGroup(group.value.sub, options, removeUnnecessaryRow));
     }
 
     if (group.value.sup) {
-        children.push(buildGroup(group.value.sup, options));
+        children.push(
+            buildGroup(group.value.sup, options, removeUnnecessaryRow));
     }
 
     let nodeType;
@@ -224,9 +231,14 @@ groupTypes.supsub = function(group, options) {
         nodeType = "msup";
     } else if (!group.value.sup) {
         nodeType = "msub";
+    } else {
+        const base = group.value.base;
+        if (base && base.value.limits && options.style === Style.DISPLAY) {
+            nodeType = "munderover";
         } else {
             nodeType = "msubsup";
         }
+    }
 
     const node = new mathMLTree.MathNode(nodeType, children);
 
@@ -457,7 +469,20 @@ groupTypes.delimsizing = function(group) {
 };
 
 groupTypes.styling = function(group, options) {
-    const inner = buildExpression(group.value.value, options);
+    // Figure out what style we're changing to.
+    // TODO(kevinb): dedupe this with buildHTML.js
+    // This will be easier of handling of styling nodes is in the same file.
+    const styleMap = {
+        "display": Style.DISPLAY,
+        "text": Style.TEXT,
+        "script": Style.SCRIPT,
+        "scriptscript": Style.SCRIPTSCRIPT,
+    };
+
+    const newStyle = styleMap[group.value.style];
+    const newOptions = options.havingStyle(newStyle);
+
+    const inner = buildExpression(group.value.value, newOptions);
 
     const node = new mathMLTree.MathNode("mstyle", inner);
 
@@ -643,14 +668,21 @@ const buildExpression = function(expression, options) {
  * Takes a group from the parser and calls the appropriate groupTypes function
  * on it to produce a MathML node.
  */
-const buildGroup = function(group, options) {
+// TODO(kevinb): determine if removeUnnecessaryRow should always be true
+const buildGroup = function(group, options, removeUnnecessaryRow = false) {
     if (!group) {
         return new mathMLTree.MathNode("mrow");
     }
 
     if (groupTypes[group.type]) {
         // Call the groupTypes function
-        return groupTypes[group.type](group, options);
+        const result = groupTypes[group.type](group, options);
+        if (removeUnnecessaryRow) {
+            if (result.type === "mrow" && result.children.length === 1) {
+                return result.children[0];
+            }
+        }
+        return result;
     } else {
         throw new ParseError(
             "Got group of unknown type: '" + group.type + "'");

test/__snapshots__/mathml-spec.js.snap

@@ -0,0 +1,133 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`A MathML builder should generate <mphantom> nodes for \\phantom 1`] = `
+
+<math>
+  <semantics>
+    <mrow>
+      <mphantom>
+        <mi>
+          x
+        </mi>
+      </mphantom>
+    </mrow>
+    <annotation encoding="application/x-tex">
+      \\phantom{x}
+    </annotation>
+  </semantics>
+</math>
+
+`;
+
+exports[`A MathML builder should generate the right types of nodes 1`] = `
+
+<math>
+  <semantics>
+    <mrow>
+      <mi>
+        sin
+      </mi>
+      <mrow>
+        <mi>
+          x
+        </mi>
+      </mrow>
+      <mo>
+        +
+      </mo>
+      <mn>
+        1
+      </mn>
+      <mspace width="0.277778em">
+      </mspace>
+      <mtext>
+        a
+      </mtext>
+    </mrow>
+    <annotation encoding="application/x-tex">
+      \\sin{x}+1\\;\\text{a}
+    </annotation>
+  </semantics>
+</math>
+
+`;
+
+exports[`A MathML builder should make prime operators into <mo> nodes 1`] = `
+
+<math>
+  <semantics>
+    <mrow>
+      <msup>
+        <mi>
+          f
+        </mi>
+        <mo mathvariant="normal">
+          ′
+        </mo>
+      </msup>
+    </mrow>
+    <annotation encoding="application/x-tex">
+      f&#x27;
+    </annotation>
+  </semantics>
+</math>
+
+`;
+
+exports[`A MathML builder should use <msupsub> for regular operators 1`] = `
+
+<math>
+  <semantics>
+    <mrow>
+      <mstyle scriptlevel="0"
+              displaystyle="false"
+      >
+        <msubsup>
+          <mo>
+            ∑
+          </mo>
+          <mi>
+            a
+          </mi>
+          <mi>
+            b
+          </mi>
+        </msubsup>
+      </mstyle>
+    </mrow>
+    <annotation encoding="application/x-tex">
+      \\textstyle\\sum_a^b
+    </annotation>
+  </semantics>
+</math>
+
+`;
+
+exports[`A MathML builder should use <munderover> for large operators 1`] = `
+
+<math>
+  <semantics>
+    <mrow>
+      <mstyle scriptlevel="0"
+              displaystyle="true"
+      >
+        <munderover>
+          <mo>
+            ∑
+          </mo>
+          <mi>
+            a
+          </mi>
+          <mi>
+            b
+          </mi>
+        </munderover>
+      </mstyle>
+    </mrow>
+    <annotation encoding="application/x-tex">
+      \\displaystyle\\sum_a^b
+    </annotation>
+  </semantics>
+</math>
+
+`;

test/katex-spec.js

@@ -2192,50 +2192,6 @@ describe("An aligned environment", function() {
 
 });
 
-const getMathML = function(expr, settings) {
-    const usedSettings = settings ? settings : defaultSettings;
-
-    expect(expr).toParse(usedSettings);
-
-    const built = buildMathML(parseTree(expr, usedSettings), expr, usedSettings);
-
-    // Strip off the surrounding <span>
-    return built.children[0];
-};
-
-describe("A MathML builder", function() {
-    it("should generate math nodes", function() {
-        const node = getMathML("x^2");
-
-        expect(node.type).toEqual("math");
-    });
-
-    it("should generate appropriate MathML types", function() {
-        const identifier = getMathML("x").children[0].children[0];
-        expect(identifier.children[0].type).toEqual("mi");
-
-        const number = getMathML("1").children[0].children[0];
-        expect(number.children[0].type).toEqual("mn");
-
-        const operator = getMathML("+").children[0].children[0];
-        expect(operator.children[0].type).toEqual("mo");
-
-        const space = getMathML("\\;").children[0].children[0];
-        expect(space.children[0].type).toEqual("mspace");
-
-        const text = getMathML("\\text{a}").children[0].children[0];
-        expect(text.children[0].type).toEqual("mtext");
-
-        const textop = getMathML("\\sin").children[0].children[0];
-        expect(textop.children[0].type).toEqual("mi");
-    });
-
-    it("should generate a <mphantom> node for \\phantom", function() {
-        const phantom = getMathML("\\phantom{x}").children[0].children[0];
-        expect(phantom.children[0].type).toEqual("mphantom");
-    });
-});
-
 describe("A parser that does not throw on unsupported commands", function() {
     // The parser breaks on unsupported commands unless it is explicitly
     // told not to

test/mathml-spec.js

@@ -0,0 +1,52 @@
+/* global expect: false */
+/* global it: false */
+/* global describe: false */
+
+import buildMathML from "../src/buildMathML";
+import parseTree from "../src/parseTree";
+import Options from "../src/Options";
+import Settings from "../src/Settings";
+import Style from "../src/Style";
+
+const defaultSettings = new Settings({});
+
+const getMathML = function(expr, settings) {
+    const usedSettings = settings ? settings : defaultSettings;
+
+    let startStyle = Style.TEXT;
+    if (usedSettings.displayMode) {
+        startStyle = Style.DISPLAY;
+    }
+
+    // Setup the default options
+    const options = new Options({
+        style: startStyle,
+    });
+
+    const built = buildMathML(parseTree(expr, usedSettings), expr, options);
+
+    // Strip off the surrounding <span>
+    return built.children[0].toMarkup();
+};
+
+describe("A MathML builder", function() {
+    it('should generate the right types of nodes', () => {
+        expect(getMathML("\\sin{x}+1\\;\\text{a}")).toMatchSnapshot();
+    });
+
+    it('should make prime operators into <mo> nodes', () => {
+        expect(getMathML("f'")).toMatchSnapshot();
+    });
+
+    it('should generate <mphantom> nodes for \\phantom', () => {
+        expect(getMathML("\\phantom{x}")).toMatchSnapshot();
+    });
+
+    it('should use <munderover> for large operators', () => {
+        expect(getMathML("\\displaystyle\\sum_a^b")).toMatchSnapshot();
+    });
+
+    it('should use <msupsub> for regular operators', () => {
+        expect(getMathML("\\textstyle\\sum_a^b")).toMatchSnapshot();
+    });
+});