Fix js crash while trying to justify empty lines (#3570)

* Fix js crash while trying to justify empty lines There is no need to justify empty lines. If we have a text that contains multiple empty lines in the middle, JS crashes in `justifyLine` function. The `start` glyph index is bigger than the `end` index in such a case. * Tests for 'justify empty lines crash fix'
mapbox · Nov 8, 2016 · 1a9dd3e · 1a9dd3e
1 parent c08b084
commit 1a9dd3e
Show file tree

Hide file tree

Showing 3 changed files with 158 additions and 1 deletion.
diff --git a/js/symbol/shaping.js b/js/symbol/shaping.js
@@ -115,7 +115,7 @@ function linewrap(shaping, glyphs, lineHeight, maxWidth, horizontalAlign, vertic
                     positionedGlyphs[k].x -= lineLength;
                 }
 
-                if (justify) {
+                if (justify && lastSafeBreak > lineStartIndex) {
                     // Collapse invisible characters.
                     let lineEnd = lastSafeBreak;
                     if (invisible[positionedGlyphs[lastSafeBreak].codePoint]) {

diff --git a/test/expected/text-shaping-newlines-in-middle.json b/test/expected/text-shaping-newlines-in-middle.json
@@ -0,0 +1,151 @@
+{
+  "positionedGlyphs": [
+    {
+      "codePoint": 97,
+      "x": -32.5,
+      "y": -41,
+      "glyph": {
+        "id": 97,
+        "width": 11,
+        "height": 15,
+        "left": 1,
+        "top": -12,
+        "advance": 13
+      }
+    },
+    {
+      "codePoint": 98,
+      "x": -19.5,
+      "y": -41,
+      "glyph": {
+        "id": 98,
+        "width": 12,
+        "height": 20,
+        "left": 2,
+        "top": -7,
+        "advance": 14
+      }
+    },
+    {
+      "codePoint": 99,
+      "x": -5.5,
+      "y": -41,
+      "glyph": {
+        "id": 99,
+        "width": 10,
+        "height": 15,
+        "left": 1,
+        "top": -12,
+        "advance": 11
+      }
+    },
+    {
+      "codePoint": 100,
+      "x": 5.5,
+      "y": -41,
+      "glyph": {
+        "id": 100,
+        "width": 12,
+        "height": 20,
+        "left": 1,
+        "top": -7,
+        "advance": 14
+      }
+    },
+    {
+      "codePoint": 101,
+      "x": 19.5,
+      "y": -41,
+      "glyph": {
+        "id": 101,
+        "width": 12,
+        "height": 15,
+        "left": 1,
+        "top": -12,
+        "advance": 13
+      }
+    },
+    {
+      "codePoint": 10,
+      "x": 65,
+      "y": -41,
+      "glyph": null
+    },
+    {
+      "codePoint": 10,
+      "x": 0,
+      "y": -17,
+      "glyph": null
+    },
+    {
+      "codePoint": 97,
+      "x": -32.5,
+      "y": 7,
+      "glyph": {
+        "id": 97,
+        "width": 11,
+        "height": 15,
+        "left": 1,
+        "top": -12,
+        "advance": 13
+      }
+    },
+    {
+      "codePoint": 98,
+      "x": -19.5,
+      "y": 7,
+      "glyph": {
+        "id": 98,
+        "width": 12,
+        "height": 20,
+        "left": 2,
+        "top": -7,
+        "advance": 14
+      }
+    },
+    {
+      "codePoint": 99,
+      "x": -5.5,
+      "y": 7,
+      "glyph": {
+        "id": 99,
+        "width": 10,
+        "height": 15,
+        "left": 1,
+        "top": -12,
+        "advance": 11
+      }
+    },
+    {
+      "codePoint": 100,
+      "x": 5.5,
+      "y": 7,
+      "glyph": {
+        "id": 100,
+        "width": 12,
+        "height": 20,
+        "left": 1,
+        "top": -7,
+        "advance": 14
+      }
+    },
+    {
+      "codePoint": 101,
+      "x": 19.5,
+      "y": 7,
+      "glyph": {
+        "id": 101,
+        "width": 12,
+        "height": 15,
+        "left": 1,
+        "top": -12,
+        "advance": 13
+      }
+    }
+  ],
+  "text": "abcde\n\nabcde",
+  "top": -36,
+  "bottom": 36,
+  "left": -32.5,
+  "right": 32.5
+}
diff --git a/test/js/symbol/shaping.test.js b/test/js/symbol/shaping.test.js
@@ -50,6 +50,12 @@ test('shaping', (t) => {
     shaped = shaping.shapeText('abcde\r\nabcde', glyphs, 15 * oneEm, oneEm, 0.5, 0.5, 0.5, 0, [0, 0]);
     t.deepEqual(shaped.positionedGlyphs, expectedNewLine.positionedGlyphs);
 
+    const expectedNewLinesInMiddle = JSON.parse(fs.readFileSync(path.join(__dirname, '/../../expected/text-shaping-newlines-in-middle.json')));
+
+    shaped = shaping.shapeText('abcde\n\nabcde', glyphs, 15 * oneEm, oneEm, 0.5, 0.5, 0.5, 0, [0, 0]);
+    if (UPDATE) fs.writeFileSync(path.join(__dirname, '/../../expected/text-shaping-newlines-in-middle.json'), JSON.stringify(shaped, null, 2));
+    t.deepEqual(shaped, expectedNewLinesInMiddle);
+
     // Null shaping.
     shaped = shaping.shapeText('', glyphs, 15 * oneEm, oneEm, 0.5, 0.5, 0.5, 0 * oneEm, [0, 0]);
     t.equal(false, shaped);