Improve JS spacing (#1103)

* Remove dummy spans for spacing around \href

Spacing around \href is handled in the `buildHTML`

* Make bin cancellation aware of children of fragment and anchor

+ Added getOutermostNode function

* Fix tight spacing not applied

* Add surrounding argument to html.buildExpression

It is an array consisting type of nodes that will be added to the left
and the right, and if given, will be used to determine bin cancellation
and spacings of outermost nodes.

* Fix html.buildExpression call in leftright

* Add dummy span only when given

* Update buildHTML.js

src/buildHTML.js

@@ -22,17 +22,13 @@ const makeSpan = buildCommon.makeSpan;
 // Binary atoms (first class `mbin`) change into ordinary atoms (`mord`)
 // depending on their surroundings. See TeXbook pg. 442-446, Rules 5 and 6,
 // and the text before Rule 19.
-const isBin = function(node) {
-    return node && node.classes[0] === "mbin";
-};
-
 const isBinLeftCanceller = function(node, isRealGroup) {
     // TODO: This code assumes that a node's math class is the first element
     // of its `classes` array. A later cleanup should ensure this, for
     // instance by changing the signature of `makeSpan`.
     if (node) {
         return utils.contains(["mbin", "mopen", "mrel", "mop", "mpunct"],
-                              node.classes[0]);
+                              getTypeOfDomTree(node, "right"));
     } else {
         return isRealGroup;
     }
@@ -40,7 +36,8 @@ const isBinLeftCanceller = function(node, isRealGroup) {
 
 const isBinRightCanceller = function(node, isRealGroup) {
     if (node) {
-        return utils.contains(["mrel", "mclose", "mpunct"], node.classes[0]);
+        return utils.contains(["mrel", "mclose", "mpunct"],
+                              getTypeOfDomTree(node, "left"));
     } else {
         return isRealGroup;
     }
@@ -58,9 +55,11 @@ const styleMap = {
  * nodes. documentFragments are flattened into their contents, so the
  * returned list contains no fragments. `isRealGroup` is true if `expression`
  * is a real group (no atoms will be added on either side), as opposed to
- * a partial group (e.g. one created by \color).
+ * a partial group (e.g. one created by \color). `surrounding` is an array
+ * consisting type of nodes that will be added to the left and right.
  */
-export const buildExpression = function(expression, options, isRealGroup) {
+export const buildExpression = function(expression, options, isRealGroup,
+        surrounding = [null, null]) {
     // Parse expressions into `groups`.
     const rawGroups = [];
     for (let i = 0; i < expression.length; i++) {
@@ -75,18 +74,27 @@ export const buildExpression = function(expression, options, isRealGroup) {
     // At this point `rawGroups` consists entirely of `symbolNode`s and `span`s.
 
     // Ignore explicit spaces (e.g., \;, \,) when determining what implicit
-    // spacing should go between atoms of different classes.
+    // spacing should go between atoms of different classes, and add dummy
-    const nonSpaces =
+    // spans for determining spacings between surrounding atoms
-        rawGroups.filter(group => group && group.classes[0] !== "mspace");
+    const nonSpaces = [
+        surrounding[0] && makeSpan([surrounding[0]], [], options),
+        ...rawGroups.filter(group => group && group.classes[0] !== "mspace"),
+        surrounding[1] && makeSpan([surrounding[1]], [], options),
+    ];
 
     // Before determining what spaces to insert, perform bin cancellation.
     // Binary operators change to ordinary symbols in some contexts.
-    for (let i = 0; i < nonSpaces.length; i++) {
+    for (let i = 1; i < nonSpaces.length - 1; i++) {
-        if (isBin(nonSpaces[i])) {
+        const left = getOutermostNode(nonSpaces[i], "left");
-            if (isBinLeftCanceller(nonSpaces[i - 1], isRealGroup)
+        if (left.classes[0] === "mbin" &&
-                    || isBinRightCanceller(nonSpaces[i + 1], isRealGroup)) {
+                isBinLeftCanceller(nonSpaces[i - 1], isRealGroup)) {
-                nonSpaces[i].classes[0] = "mord";
+            left.classes[0] = "mord";
         }
+
+        const right = getOutermostNode(nonSpaces[i], "right");
+        if (right.classes[0] === "mbin" &&
+                isBinRightCanceller(nonSpaces[i + 1], isRealGroup)) {
+            right.classes[0] = "mord";
         }
     }
 
@@ -99,6 +107,13 @@ export const buildExpression = function(expression, options, isRealGroup) {
         // lookup what implicit space should be placed between those atoms and
         // add it to groups.
         if (rawGroups[i].classes[0] !== "mspace" && j < nonSpaces.length - 1) {
+            // if current non-space node is left dummy span, add a glue before
+            // first real non-space node
+            if (j === 0) {
+                groups.pop();
+                i--;
+            }
+
             // Get the type of the current non-space node.  If it's a document
             // fragment, get the type of the rightmost node in the fragment.
             const left = getTypeOfDomTree(nonSpaces[j], "right");
@@ -150,20 +165,30 @@ export const buildExpression = function(expression, options, isRealGroup) {
     return groups;
 };
 
-// Return math atom class (mclass) of a domTree.
+// Return the outermost node of a domTree.
-export const getTypeOfDomTree = function(node, side = "right") {
+const getOutermostNode = function(node, side = "right") {
     if (node instanceof domTree.documentFragment ||
             node instanceof domTree.anchor) {
         if (node.children.length) {
             if (side === "right") {
-                return getTypeOfDomTree(
+                return getOutermostNode(
                     node.children[node.children.length - 1]);
             } else if (side === "left") {
-                return getTypeOfDomTree(
+                return getOutermostNode(
                     node.children[0]);
             }
         }
-    } else {
+    }
+    return node;
+};
+
+// Return math atom class (mclass) of a domTree.
+export const getTypeOfDomTree = function(node, side = "right") {
+    if (!node) {
+        return null;
+    }
+
+    node = getOutermostNode(node, side);
     // This makes a lot of assumptions as to where the type of atom
     // appears.  We should do a better job of enforcing this.
     if (utils.contains([
@@ -172,7 +197,6 @@ export const getTypeOfDomTree = function(node, side = "right") {
     ], node.classes[0])) {
         return node.classes[0];
     }
-    }
     return null;
 };
 
@@ -181,14 +205,8 @@ export const getTypeOfDomTree = function(node, side = "right") {
 // leftmost node in the fragment.
 // 'mtight' indicates that the node is script or scriptscript style.
 export const isLeftTight = function(node) {
-    if (node instanceof domTree.documentFragment) {
+    node = getOutermostNode(node, "left");
-        if (node.children.length) {
-            return isLeftTight(node.children[0]);
-        }
-    } else {
     return utils.contains(node.classes, "mtight");
-    }
-    return false;
 };
 
 /**

src/functions/delimsizing.js

@@ -5,8 +5,6 @@ import delimiter from "../delimiter";
 import mathMLTree from "../mathMLTree";
 import ParseError from "../ParseError";
 import utils from "../utils";
-import { calculateSize } from "../units";
-import { spacings, tightSpacings } from "../spacingData";
 
 import * as html from "../buildHTML";
 import * as mml from "../buildMathML";
@@ -161,7 +159,8 @@ defineFunction({
     },
     htmlBuilder: (group, options) => {
         // Build the inner expression
-        const inner = html.buildExpression(group.value.body, options, true);
+        const inner = html.buildExpression(group.value.body, options, true,
+            [null, "mclose"]);
 
         let innerHeight = 0;
         let innerDepth = 0;
@@ -210,18 +209,6 @@ defineFunction({
             }
         }
 
-        const lastChildType = html.getTypeOfDomTree(inner[inner.length - 1]);
-        const activeSpacings = options.style.isTight() ? tightSpacings : spacings;
-
-        if (lastChildType && activeSpacings[lastChildType]["mclose"]) {
-            const glue =
-                buildCommon.makeSpan(["mord", "rule"], [], options);
-            const dimension =
-                calculateSize(activeSpacings[lastChildType]["mclose"], options);
-            glue.style.marginRight = `${dimension}em`;
-            inner.push(glue);
-        }
-
         let rightDelim;
         // Same for the right delimiter
         if (group.value.right === ".") {

src/functions/href.js

@@ -31,40 +31,7 @@ defineFunction({
 
         const href = group.value.href;
 
-        /**
+        return new buildCommon.makeAnchor(href, [], elements, options);
-         * Determining class for anchors.
-         * 1. if it has the only element, use its class;
-         * 2. if it has more than two elements, and the classes
-         *    of its first and last elements coincide, then use it;
-         * 3. otherwise, we will inject an empty <span>s at both ends,
-         *    with the same classes of both ends of elements, with the
-         *    first span having the same class as the first element of body,
-         *    and the second one the same as the last.
-         */
-
-        let classes = []; // Default behaviour for Case 3.
-        let first; // mathtype of the first child
-        let last;  // mathtype of the last child
-        // Invariants: both first and last must be non-null if classes is null.
-        if (elements.length === 1) { // Case 1
-            classes = elements[0].classes;
-        } else if (elements.length >= 2) {
-            first = html.getTypeOfDomTree(elements[0]) || 'mord';
-            last = html.getTypeOfDomTree(elements[elements.length - 1]) || 'mord';
-            if (first === last) { // Case 2 : type of both ends coincides
-                classes = [first];
-            } else { // Case 3: both ends have different types.
-                // TODO(kevinb): figure out a better way to communicate this
-                // information to buildHTML.js#buildExpression.
-                const anc = buildCommon.makeAnchor(href, [], elements, options);
-                return new buildCommon.makeFragment([
-                    new buildCommon.makeSpan([first], [], options),
-                    anc,
-                    new buildCommon.makeSpan([last], [], options),
-                ]);
-            }
-        }
-        return new buildCommon.makeAnchor(href, classes, elements, options);
     },
     mathmlBuilder: (group, options) => {
         const inner = mml.buildExpression(group.value.body, options);