Nested Math in Non-Default Text Fix (#1111)

* Fixing nested math in non-default text. Added appropriate screenshot tests. * Adding appropriate logic for buildMathML and resolving tests. * Addressing Kevin's comment. Adding tests. * Adding appropriate screenshots. * Removing unnecessary data, adding consistent naming convention. * Cleanup * Handling possible edge cases. * Updating per PR comments
2025-10-05 19:28:39 +00:00 · 2018-02-19 17:37:08 -05:00
parent b341034d2b
commit 7de91f73eb
12 changed files with 109 additions and 82 deletions
--- a/src/Options.js
+++ b/src/Options.js
@@ -42,9 +42,8 @@ export type OptionsData = {
    size?: number;
    textSize?: number;
    phantom?: boolean;
-    // TODO(#1009): Keep consistent with fontFamily/fontWeight. Ensure this has a
-    // string value.
-    fontFamily?: string | void;
+    font?: string;
+    fontFamily?: string;
    fontWeight?: string;
    fontShape?: string;
    sizeMultiplier?: number;
@@ -64,7 +63,11 @@ class Options {
    size: number;
    textSize: number;
    phantom: boolean;
-    fontFamily: string | void;
+    // A font family applies to a group of fonts (i.e. SansSerif), while a font
+    // represents a specific font (i.e. SansSerif Bold).
+    // See: https://tex.stackexchange.com/questions/22350/difference-between-textrm-and-mathrm
+    font: string;
+    fontFamily: string;
    fontWeight: string;
    fontShape: string;
    sizeMultiplier: number;
@@ -82,7 +85,8 @@ class Options {
        this.size = data.size || Options.BASESIZE;
        this.textSize = data.textSize || this.size;
        this.phantom = !!data.phantom;
-        this.fontFamily = data.fontFamily;
+        this.font = data.font || "";
+        this.fontFamily = data.fontFamily || "";
        this.fontWeight = data.fontWeight || '';
        this.fontShape = data.fontShape || '';
        this.sizeMultiplier = sizeMultipliers[this.size - 1];
@@ -101,6 +105,7 @@ class Options {
            textSize: this.textSize,
            color: this.color,
            phantom: this.phantom,
+            font: this.font,
            fontFamily: this.fontFamily,
            fontWeight: this.fontWeight,
            fontShape: this.fontShape,
@@ -193,29 +198,42 @@ class Options {
    }

    /**
-     * Create a new options objects with the give font.
+     * Creates a new options object with the given math font or old text font.
+     * @type {[type]}
     */
-    withFontFamily(fontFamily: ?string): Options {
+    withFont(font: string): Options {
        return this.extend({
-            fontFamily: fontFamily || this.fontFamily,
+            font,
+        });
+    }
+
+    /**
+     * Create a new options objects with the given fontFamily.
+     */
+    withTextFontFamily(fontFamily: string) {
+        return this.extend({
+            fontFamily,
+            font: "",
        });
    }

    /**
     * Creates a new options object with the given font weight
     */
-    withFontWeight(fontWeight: string): Options {
+    withTextFontWeight(fontWeight: string): Options {
        return this.extend({
            fontWeight,
+            font: "",
        });
    }

    /**
     * Creates a new options object with the given font weight
     */
-    withFontShape(fontShape: string): Options {
+    withTextFontShape(fontShape: string): Options {
        return this.extend({
            fontShape,
+            font: "",
        });
    }

--- a/src/buildCommon.js
+++ b/src/buildCommon.js
@@ -33,7 +33,7 @@ const mainitLetters = [
 const lookupSymbol = function(
    value: string,
    // TODO(#963): Use a union type for this.
-    fontFamily: string,
+    fontName: string,
    mode: Mode,
 ): {value: string, metrics: ?CharacterMetrics} {
    // Replace the value with its replaced value from symbol.js
@@ -42,7 +42,7 @@ const lookupSymbol = function(
    }
    return {
        value: value,
-        metrics: fontMetrics.getCharacterMetrics(value, fontFamily, mode),
+        metrics: fontMetrics.getCharacterMetrics(value, fontName, mode),
    };
 };

@@ -58,12 +58,12 @@ const lookupSymbol = function(
 */
 const makeSymbol = function(
    value: string,
-    fontFamily: string,
+    fontName: string,
    mode: Mode,
    options?: Options,
    classes?: string[],
 ): domTree.symbolNode {
-    const lookup = lookupSymbol(value, fontFamily, mode);
+    const lookup = lookupSymbol(value, fontName, mode);
    const metrics = lookup.metrics;
    value = lookup.value;

@@ -80,7 +80,7 @@ const makeSymbol = function(
        // TODO(emily): Figure out a good way to only print this in development
        typeof console !== "undefined" && console.warn(
            "No character metrics for '" + value + "' in style '" +
-                fontFamily + "'");
+                fontName + "'");
        symbolNode = new domTree.symbolNode(value, 0, 0, 0, 0, 0, classes);
    }

@@ -117,7 +117,7 @@ const mathsym = function(
    // text ordinal and is therefore not present as a symbol in the symbols
    // table for text, as well as a special case for boldsymbol because it
    // can be used for bold + and -
-    if ((options && options.fontFamily && options.fontFamily === "boldsymbol") &&
+    if ((options && options.font && options.font === "boldsymbol") &&
            lookupSymbol(value, "Main-Bold", mode).metrics) {
        return makeSymbol(value, "Main-Bold", mode, options,
            classes.concat(["mathbf"]));
@@ -231,27 +231,28 @@ const makeOrd = function(

    const classes = ["mord"];

-    const fontFamily = options.fontFamily;
-    if (fontFamily) {
+    // Math mode or Old font (i.e. \rm)
+    const isFont = mode === "math" || (mode === "text" && options.font);
+    const fontOrFamily = isFont ? options.font : options.fontFamily;
+    if (fontOrFamily) {
        let fontName;
        let fontClasses;
-        if (fontFamily === "boldsymbol") {
+        if (fontOrFamily === "boldsymbol") {
            const fontData = boldsymbol(value, mode, options, classes);
            fontName = fontData.fontName;
            fontClasses = [fontData.fontClass];
-        } else if (fontFamily === "mathit" ||
+        } else if (fontOrFamily === "mathit" ||
                   utils.contains(mainitLetters, value)) {
            const fontData = mathit(value, mode, options, classes);
            fontName = fontData.fontName;
            fontClasses = [fontData.fontClass];
-        } else if (fontFamily.indexOf("math") !== -1 || mode === "math") {
-            // To support old font functions (i.e. \rm \sf etc.) or math mode.
-            fontName = fontMap[fontFamily].fontName;
-            fontClasses = [fontFamily];
+        } else if (isFont) {
+            fontName = fontMap[fontOrFamily].fontName;
+            fontClasses = [fontOrFamily];
        } else {
-            fontName = retrieveTextFontName(fontFamily, options.fontWeight,
+            fontName = retrieveTextFontName(fontOrFamily, options.fontWeight,
                                            options.fontShape);
-            fontClasses = [fontFamily, options.fontWeight, options.fontShape];
+            fontClasses = [fontOrFamily, options.fontWeight, options.fontShape];
        }
        if (lookupSymbol(value, fontName, mode).metrics) {
            return makeSymbol(value, fontName, mode, options,
--- a/src/buildMathML.js
+++ b/src/buildMathML.js
@@ -31,7 +31,7 @@ export const makeText = function(text, mode) {
 * Returns the math variant as a string or null if none is required.
 */
 const getVariant = function(group, options) {
-    const font = options.fontFamily;
+    const font = options.font;
    if (!font) {
        return null;
    }
--- a/src/functions/font.js
+++ b/src/functions/font.js
@@ -10,12 +10,14 @@ import * as mml from "../buildMathML";

 const htmlBuilder = (group, options) => {
    const font = group.value.font;
-    return html.buildGroup(group.value.body, options.withFontFamily(font));
+    const newOptions = options.withFont(font);
+    return html.buildGroup(group.value.body, newOptions);
 };

 const mathmlBuilder = (group, options) => {
    const font = group.value.font;
-    return mml.buildGroup(group.value.body, options.withFontFamily(font));
+    const newOptions = options.withFont(font);
+    return mml.buildGroup(group.value.body, newOptions);
 };

 const fontAliases = {
--- a/src/functions/operatorname.js
+++ b/src/functions/operatorname.js
@@ -39,7 +39,7 @@ defineFunction({

            // Consolidate Greek letter function names into symbol characters.
            const temp = html.buildExpression(
-                group.value.value, options.withFontFamily("mathrm"), true);
+                group.value.value, options.withFont("mathrm"), true);

            // All we want from temp are the letters. With them, we'll
            // create a text operator similar to \tan or \cos.
@@ -69,7 +69,7 @@ defineFunction({
        let output = [];
        if (group.value.value.length > 0) {
            const temp = mml.buildExpression(
-                group.value.value, options.withFontFamily("mathrm"));
+                group.value.value, options.withFont("mathrm"));

            let word = temp.map(node => node.toText()).join("");

--- a/src/functions/styling.js
+++ b/src/functions/styling.js
@@ -41,7 +41,7 @@ defineFunction({
    htmlBuilder: (group, options) => {
        // Style changes are handled in the TeXbook on pg. 442, Rule 3.
        const newStyle = styleMap[group.value.style];
-        const newOptions = options.havingStyle(newStyle);
+        const newOptions = options.havingStyle(newStyle).withFont('');
        return sizingGroup(group.value.value, newOptions, options);
    },
    mathmlBuilder: (group, options) => {
--- a/src/functions/text.js
+++ b/src/functions/text.js
@@ -49,11 +49,11 @@ defineFunction({
        // Checks if the argument is a font family or a font style.
        let newOptions;
        if (textFontFamilies[font]) {
-            newOptions = options.withFontFamily(textFontFamilies[font]);
+            newOptions = options.withTextFontFamily(textFontFamilies[font]);
        } else if (textFontWeights[font]) {
-            newOptions = options.withFontWeight(textFontWeights[font]);
+            newOptions = options.withTextFontWeight(textFontWeights[font]);
        } else {
-            newOptions = options.withFontShape(textFontShapes[font]);
+            newOptions = options.withTextFontShape(textFontShapes[font]);
        }
        const inner = html.buildExpression(group.value.body, newOptions, true);
        buildCommon.tryCombineChars(inner);
--- a/test/snapshots/katex-spec.js.snap
+++ b/test/snapshots/katex-spec.js.snap
@@ -88,51 +88,7 @@ exports[`An implicit group parser within optional groups should work with \\colo
 ]
 `;

-exports[`An implicit group parser within optional groups should work with sizing commands: \\sqrt[\\small 3]{x} 1`] = `
-[
-  {
-    "type": "sqrt",
-    "mode": "math",
-    "value": {
-      "type": "sqrt",
-      "body": {
-        "type": "ordgroup",
-        "mode": "math",
-        "value": [
-          {
-            "type": "mathord",
-            "mode": "math",
-            "value": "x"
-          }
-        ]
-      },
-      "index": {
-        "type": "ordgroup",
-        "mode": "math",
-        "value": [
-          {
-            "type": "sizing",
-            "mode": "math",
-            "value": {
-              "type": "sizing",
-              "size": 5,
-              "value": [
-                {
-                  "type": "textord",
-                  "mode": "math",
-                  "value": "3"
-                }
-              ]
-            }
-          }
-        ]
-      }
-    }
-  }
-]
-`;
-
-exports[`An implicit group parser within optional groups should work wwith old font functions: \\sqrt[\\tt 3]{x} 1`] = `
+exports[`An implicit group parser within optional groups should work with old font functions: \\sqrt[\\tt 3]{x} 1`] = `
 [
  {
    "type": "sqrt",
@@ -179,3 +135,47 @@ exports[`An implicit group parser within optional groups should work wwith old f
  }
 ]
 `;
+
+exports[`An implicit group parser within optional groups should work with sizing commands: \\sqrt[\\small 3]{x} 1`] = `
+[
+  {
+    "type": "sqrt",
+    "mode": "math",
+    "value": {
+      "type": "sqrt",
+      "body": {
+        "type": "ordgroup",
+        "mode": "math",
+        "value": [
+          {
+            "type": "mathord",
+            "mode": "math",
+            "value": "x"
+          }
+        ]
+      },
+      "index": {
+        "type": "ordgroup",
+        "mode": "math",
+        "value": [
+          {
+            "type": "sizing",
+            "mode": "math",
+            "value": {
+              "type": "sizing",
+              "size": 5,
+              "value": [
+                {
+                  "type": "textord",
+                  "mode": "math",
+                  "value": "3"
+                }
+              ]
+            }
+          }
+        ]
+      }
+    }
+  }
+]
+`;
--- a/test/katex-spec.js
+++ b/test/katex-spec.js
@@ -607,7 +607,7 @@ describe("An implicit group parser", function() {
            expect(tree).toMatchSnapshot();
        });

-        it("should work wwith old font functions: \\sqrt[\\tt 3]{x}", () => {
+        it("should work with old font functions: \\sqrt[\\tt 3]{x}", () => {
            const tree = stripPositions(getParsed("\\sqrt[\\tt 3]{x}"));
            expect(tree).toMatchSnapshot();
        });
--- a/test/screenshotter/images/TextWithMath-chrome.png
+++ b/test/screenshotter/images/TextWithMath-chrome.png
--- a/test/screenshotter/images/TextWithMath-firefox.png
+++ b/test/screenshotter/images/TextWithMath-firefox.png
--- a/test/screenshotter/ss_data.yaml
+++ b/test/screenshotter/ss_data.yaml
@@ -309,7 +309,13 @@ TextStacked:
      \textsf{\textrm{\textbf{abc123}} \textbf{abc123} \textit{abc123}}\\
      \textit{abc123 \textbf{abc123} \textsf{abc123}}\\
  \end{matrix}
-TextWithMath: \text{for $a < b$ and $ c < d $}.
+TextWithMath: 
+  \begin{matrix}
+      \text{for $a < b$ and $ c < d $}. \\
+      \textsf{for $a < b$ and $ c < d $}. \\
+      \textsf{for $a < b \textbf{ and } c < d $} \\
+      \text{\sf for $a < b$ and $c < d$.}
+  \end{matrix}
 Unicode: \begin{matrix}\text{ÀàÇçÉéÏïÖöÛû} \\ \text{БГДЖЗЙЛФЦШЫЮЯ} \\ \text{여보세요} \\ \text{私はバナナです} \end{matrix}
 Units: |
    \begin{array}{ll}