Repository: KaTeX/KaTeX (js)

Issue:
Small alignment issue with minner
The following have a span of class `minner` containing the entire things.
![image](https://user-images.githubusercontent.com/41368919/50839694-31a56d00-13a4-11e9-83b6-2bb7c18dd418.png)
However, making a nonempty new line, the line _above_ changes: a span of class `mspace` has created. (`<span class="mspace" style="margin-right: 0.166667em;"></span>`)
![image](https://user-images.githubusercontent.com/41368919/50839679-2b16f580-13a4-11e9-84dd-acb75725bb38.png)
Differences:
![image](https://user-images.githubusercontent.com/41368919/50839708-38cc7b00-13a4-11e9-91b0-dc87a18019e3.png)

Was it intended? I think the new line should not affect the line above.

I checked that the line 57 of cr.js occurs the issue:
```js
const span = buildCommon.makeSpan(["mspace"], [], options);
```

Pull request:
Reset to leftmost spacing mode after newline
Newlines have class "`mspace newline`" but shouldn't be fully ignored: they should reset the spacing mode back to "leftmost" so as to not accidentally create trailing spaces at end of lines.  Fix #1828.

At least, I'm pretty sure no kerns are generated before `\\`/`\newline`, but it wouldn't hurt to verify that this is indeed the case in (La)TeX.

Patch:
diff --git a/src/buildHTML.js b/src/buildHTML.js
index 56186e19..a69de4d0 100644
--- a/src/buildHTML.js
+++ b/src/buildHTML.js
@@ -59,7 +59,7 @@ type DomType = $Keys<typeof DomEnum>;
 export const buildExpression = function(
     expression: AnyParseNode[],
     options: Options,
-    isRealGroup: boolean,
+    isRealGroup: boolean | "root",
     surrounding: [?DomType, ?DomType] = [null, null],
 ): HtmlDomNode[] {
     // Parse expressions into `groups`.
@@ -102,6 +102,7 @@ export const buildExpression = function(
 
     // Before determining what spaces to insert, perform bin cancellation.
     // Binary operators change to ordinary symbols in some contexts.
+    const isRoot = (isRealGroup === "root");
     traverseNonSpaceNodes(groups, (node, prev) => {
         const prevType = prev.classes[0];
         const type = node.classes[0];
@@ -110,7 +111,7 @@ export const buildExpression = function(
         } else if (type === "mbin" && utils.contains(binLeftCanceller, prevType)) {
             node.classes[0] = "mord";
         }
-    }, {node: dummyPrev}, dummyNext);
+    }, {node: dummyPrev}, dummyNext, isRoot);
 
     traverseNonSpaceNodes(groups, (node, prev) => {
         const prevType = getTypeOfDomTree(prev);
@@ -123,7 +124,7 @@ export const buildExpression = function(
         if (space) { // Insert glue (spacing) after the `prev`.
             return buildCommon.makeGlue(space, glueOptions);
         }
-    }, {node: dummyPrev}, dummyNext);
+    }, {node: dummyPrev}, dummyNext, isRoot);
 
     return groups;
 };
@@ -141,6 +142,7 @@ const traverseNonSpaceNodes = function(
         insertAfter?: HtmlDomNode => void,
     |},
     next: ?HtmlDomNode,
+    isRoot: boolean,
 ) {
     if (next) { // temporarily append the right node, if exists
         nodes.push(next);
@@ -151,27 +153,31 @@ const traverseNonSpaceNodes = function(
         const partialGroup = checkPartialGroup(node);
         if (partialGroup) { // Recursive DFS
             // $FlowFixMe: make nodes a $ReadOnlyArray by returning a new array
-            traverseNonSpaceNodes(partialGroup.children, callback, prev);
+            traverseNonSpaceNodes(partialGroup.children,
+                callback, prev, null, isRoot);
             continue;
         }
 
         // Ignore explicit spaces (e.g., \;, \,) when determining what implicit
         // spacing should go between atoms of different classes
-        if (node.classes[0] === "mspace") {
-            continue;
-        }
-
-        const result = callback(node, prev.node);
-        if (result) {
-            if (prev.insertAfter) {
-                prev.insertAfter(result);
-            } else { // insert at front
-                nodes.unshift(result);
-                i++;
+        const nonspace = !node.hasClass("mspace");
+        if (nonspace) {
+            const result = callback(node, prev.node);
+            if (result) {
+                if (prev.insertAfter) {
+                    prev.insertAfter(result);
+                } else { // insert at front
+                    nodes.unshift(result);
+                    i++;
+                }
             }
         }
 
-        prev.node = node;
+        if (nonspace) {
+            prev.node = node;
+        } else if (isRoot && node.hasClass("newline")) {
+            prev.node = makeSpan(["leftmost"]); // treat like beginning of line
+        }
         prev.insertAfter = (index => n => {
             nodes.splice(index + 1, 0, n);
             i++;
@@ -310,7 +316,7 @@ export default function buildHTML(tree: AnyParseNode[], options: Options): DomSp
     }
 
     // Build the expression contained in the tree
-    const expression = buildExpression(tree, options, true);
+    const expression = buildExpression(tree, options, "root");
 
     const children = [];
 


Test patch:
diff --git a/test/__snapshots__/katex-spec.js.snap b/test/__snapshots__/katex-spec.js.snap
index 99dec740..59f67494 100755
--- a/test/__snapshots__/katex-spec.js.snap
+++ b/test/__snapshots__/katex-spec.js.snap
@@ -1228,10 +1228,6 @@ exports[`Newlines via \\\\ and \\newline \\\\ causes newline, even after mrel an
       <span class="mrel">
         =
       </span>
-      <span class="mspace"
-            style="margin-right:0.2777777777777778em;"
-      >
-      </span>
     </span>
     <span class="mspace newline">
     </span>
@@ -1250,10 +1246,6 @@ exports[`Newlines via \\\\ and \\newline \\\\ causes newline, even after mrel an
       <span class="mbin">
         +
       </span>
-      <span class="mspace"
-            style="margin-right:0.2222222222222222em;"
-      >
-      </span>
     </span>
     <span class="mspace newline">
     </span>