simplify combining chars (#1633)

* simplify comibining chars

* try combining chars in text operators instead of all ordgroups

* ensure that adjacent chars have the same classes

* fix phantom tests

* extract canCombe from tryCombineChars, check for skew and maxFontSize

* check prev.italic !== next.italic

* use the last character's italic correction.

src/buildCommon.js

@@ -5,7 +5,7 @@
  * different kinds of domTree nodes in a consistent manner.
  */
 
-import {SymbolNode, Anchor, Span, PathNode, SvgNode} from "./domTree";
+import {SymbolNode, Anchor, Span, PathNode, SvgNode, createClass} from "./domTree";
 import {getCharacterMetrics} from "./fontMetrics";
 import symbols, {ligatures} from "./symbols";
 import utils from "./utils";
@@ -15,7 +15,6 @@ import {DocumentFragment} from "./tree";
 
 import type Options from "./Options";
 import type {ParseNode} from "./parseNode";
-import type {NodeType} from "./parseNode";
 import type {CharacterMetrics} from "./fontMetrics";
 import type {FontVariant, Mode} from "./types";
 import type {documentFragment as HtmlDocumentFragment} from "./domTree";
@@ -132,47 +131,6 @@ const mathsym = function(
     }
 };
 
-/**
- * Makes a symbol in the default font for mathords and textords.
- */
-const mathDefault = function(
-    value: string,
-    mode: Mode,
-    options: Options,
-    classes: string[],
-    type: NodeType,
-): SymbolNode {
-    if (type === "mathord") {
-        const fontLookup = mathit(value, mode, options, classes);
-        return makeSymbol(value, fontLookup.fontName, mode, options,
-            classes.concat([fontLookup.fontClass]));
-    } else if (type === "textord") {
-        const font = symbols[mode][value] && symbols[mode][value].font;
-        if (font === "ams") {
-            const fontName = retrieveTextFontName("amsrm", options.fontWeight,
-                  options.fontShape);
-            return makeSymbol(
-                value, fontName, mode, options,
-                classes.concat("amsrm", options.fontWeight, options.fontShape));
-        } else if (font === "main" || !font) {
-            const fontName = retrieveTextFontName("textrm", options.fontWeight,
-                  options.fontShape);
-            return makeSymbol(
-                value, fontName, mode, options,
-                classes.concat(options.fontWeight, options.fontShape));
-        } else { // fonts added by plugins
-            const fontName = retrieveTextFontName(font, options.fontWeight,
-                  options.fontShape);
-            // We add font name as a css class
-            return makeSymbol(
-                value, fontName, mode, options,
-                classes.concat(fontName, options.fontWeight, options.fontShape));
-        }
-    } else {
-        throw new Error("unexpected type: " + type + " in mathDefault");
-    }
-};
-
 /**
  * Determines which of the two font names (Main-Italic and Math-Italic) and
  * corresponding style tags (mainit or mathit) to use for font "mathit",
@@ -282,21 +240,88 @@ const makeOrd = function<NODETYPE: "spacing" | "mathord" | "textord">(
                                       classes.concat(fontClasses)));
             }
             return makeFragment(parts);
-        } else {
+        }
-            return mathDefault(text, mode, options, classes, type);
+    }
+
+    // Makes a symbol in the default font for mathords and textords.
+    if (type === "mathord") {
+        const fontLookup = mathit(text, mode, options, classes);
+        return makeSymbol(text, fontLookup.fontName, mode, options,
+            classes.concat([fontLookup.fontClass]));
+    } else if (type === "textord") {
+        const font = symbols[mode][text] && symbols[mode][text].font;
+        if (font === "ams") {
+            const fontName = retrieveTextFontName("amsrm", options.fontWeight,
+                  options.fontShape);
+            return makeSymbol(
+                text, fontName, mode, options,
+                classes.concat("amsrm", options.fontWeight, options.fontShape));
+        } else if (font === "main" || !font) {
+            const fontName = retrieveTextFontName("textrm", options.fontWeight,
+                  options.fontShape);
+            return makeSymbol(
+                text, fontName, mode, options,
+                classes.concat(options.fontWeight, options.fontShape));
+        } else { // fonts added by plugins
+            const fontName = retrieveTextFontName(font, options.fontWeight,
+                  options.fontShape);
+            // We add font name as a css class
+            return makeSymbol(
+                text, fontName, mode, options,
+                classes.concat(fontName, options.fontWeight, options.fontShape));
         }
     } else {
-        return mathDefault(text, mode, options, classes, type);
+        throw new Error("unexpected type: " + type + " in makeOrd");
     }
 };
 
 /**
- * Combine as many characters as possible in the given array of characters
+ * Returns true if subsequent symbolNodes have the same classes, skew, maxFont,
- * via their tryCombine method.
+ * and styles.
  */
-const tryCombineChars = function(chars: HtmlDomNode[]): HtmlDomNode[] {
+const canCombine = (prev: SymbolNode, next: SymbolNode) => {
+    if (createClass(prev.classes) !== createClass(next.classes)
+        || prev.skew !== next.skew
+        || prev.maxFontSize !== next.maxFontSize) {
+        return false;
+    }
+
+    for (const style in prev.style) {
+        if (prev.style.hasOwnProperty(style)
+            && prev.style[style] !== next.style[style]) {
+            return false;
+        }
+    }
+
+    for (const style in next.style) {
+        if (next.style.hasOwnProperty(style)
+            && prev.style[style] !== next.style[style]) {
+            return false;
+        }
+    }
+
+    return true;
+};
+
+/**
+ * Combine consequetive domTree.symbolNodes into a single symbolNode.
+ * Note: this function mutates the argument.
+ */
+const tryCombineChars = (chars: HtmlDomNode[]): HtmlDomNode[] => {
     for (let i = 0; i < chars.length - 1; i++) {
-        if (chars[i].tryCombine(chars[i + 1])) {
+        const prev = chars[i];
+        const next = chars[i + 1];
+        if (prev instanceof SymbolNode
+            && next instanceof SymbolNode
+            && canCombine(prev, next)) {
+
+            prev.text += next.text;
+            prev.height = Math.max(prev.height, next.height);
+            prev.depth = Math.max(prev.depth, next.depth);
+            // Use the last character's italic correction since we use
+            // it to add padding to the right of the span created from
+            // the combined characters.
+            prev.italic = next.italic;
             chars.splice(i + 1, 1);
             i--;
         }

src/domTree.js

@@ -24,7 +24,7 @@ import type {VirtualNode} from "./tree";
  * Create an HTML className based on a list of classes. In addition to joining
  * with spaces, we also remove empty classes.
  */
-const createClass = function(classes: string[]): string {
+export const createClass = function(classes: string[]): string {
     return classes.filter(cls => cls).join(" ");
 };
 
@@ -135,7 +135,6 @@ export interface HtmlDomNode extends VirtualNode {
     style: CssStyle;
 
     hasClass(className: string): boolean;
-    tryCombine(sibling: HtmlDomNode): boolean;
 }
 
 // Span wrapping other DOM nodes.
@@ -189,15 +188,6 @@ export class Span<ChildType: VirtualNode> implements HtmlDomNode {
         return utils.contains(this.classes, className);
     }
 
-    /**
-     * Try to combine with given sibling.  Returns true if the sibling has
-     * been successfully merged into this node, and false otherwise.
-     * Default behavior fails (returns false).
-     */
-    tryCombine(sibling: HtmlDomNode): boolean {
-        return false;
-    }
-
     toNode(): HTMLElement {
         return toNode.call(this, "span");
     }
@@ -239,10 +229,6 @@ export class Anchor implements HtmlDomNode {
         return utils.contains(this.classes, className);
     }
 
-    tryCombine(sibling: HtmlDomNode): boolean {
-        return false;
-    }
-
     toNode(): HTMLElement {
         return toNode.call(this, "a");
     }
@@ -317,34 +303,6 @@ export class SymbolNode implements HtmlDomNode {
         return utils.contains(this.classes, className);
     }
 
-    tryCombine(sibling: HtmlDomNode): boolean {
-        if (!sibling
-            || !(sibling instanceof SymbolNode)
-            || this.italic > 0
-            || createClass(this.classes) !== createClass(sibling.classes)
-            || this.skew !== sibling.skew
-            || this.maxFontSize !== sibling.maxFontSize) {
-            return false;
-        }
-        for (const style in this.style) {
-            if (this.style.hasOwnProperty(style)
-                && this.style[style] !== sibling.style[style]) {
-                return false;
-            }
-        }
-        for (const style in sibling.style) {
-            if (sibling.style.hasOwnProperty(style)
-                && this.style[style] !== sibling.style[style]) {
-                return false;
-            }
-        }
-        this.text += sibling.text;
-        this.height = Math.max(this.height, sibling.height);
-        this.depth = Math.max(this.depth, sibling.depth);
-        this.italic = sibling.italic;
-        return true;
-    }
-
     /**
      * Creates a text node or span from a symbol node. Note that a span is only
      * created if it is needed.

src/functions/op.js

@@ -95,7 +95,8 @@ export const htmlBuilder: HtmlBuilderSupSub<"op"> = (grp, options) => {
             base = inner[0];
             base.classes[0] = "mop"; // replace old mclass
         } else {
-            base = buildCommon.makeSpan(["mop"], inner, options);
+            base = buildCommon.makeSpan(
+                ["mop"], buildCommon.tryCombineChars(inner), options);
         }
     } else {
         // Otherwise, this is a text operator. Build the text from the

src/functions/text.js

@@ -62,8 +62,8 @@ defineFunction({
     htmlBuilder(group, options) {
         const newOptions = optionsWithFont(group, options);
         const inner = html.buildExpression(group.body, newOptions, true);
-        buildCommon.tryCombineChars(inner);
+        return buildCommon.makeSpan(
-        return buildCommon.makeSpan(["mord", "text"], inner, newOptions);
+            ["mord", "text"], buildCommon.tryCombineChars(inner), newOptions);
     },
     mathmlBuilder(group, options) {
         const newOptions = optionsWithFont(group, options);

src/functions/verb.js

@@ -32,10 +32,11 @@ defineFunction({
             body.push(buildCommon.makeSymbol(c, "Typewriter-Regular",
                 group.mode, newOptions, ["mord", "texttt"]));
         }
-        buildCommon.tryCombineChars(body);
         return buildCommon.makeSpan(
             ["mord", "text"].concat(newOptions.sizingClasses(options)),
-            body, newOptions);
+            buildCommon.tryCombineChars(body),
+            newOptions,
+        );
     },
     mathmlBuilder(group, options) {
         const text = new mathMLTree.TextNode(buildCommon.makeVerb(group, options));

src/tree.js

@@ -41,10 +41,6 @@ export class DocumentFragment<ChildType: VirtualNode>
         return utils.contains(this.classes, className);
     }
 
-    tryCombine(sibling: HtmlDomNode): boolean {
-        return false;
-    }
-
     /** Convert the fragment into a node. */
     toNode(): Node {
         const frag = document.createDocumentFragment();