Fixes broken diff test cases + Bugfix for: TypeError: Failed to execute 'insertBefore' on 'Node'

openslides/motions/static/js/motions/diff.js

@@ -901,7 +901,7 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi
         };
 
         this._diffParagraphs = function(oldText, newText, lineLength, firstLineNumber) {
-            var oldTextWithBreaks, newTextWithBreaks;
+            var oldTextWithBreaks, newTextWithBreaks, currChild;
 
             if (lineLength !== undefined) {
                 oldTextWithBreaks = lineNumberingService.insertLineNumbersNode(oldText, lineLength, null, firstLineNumber);
@@ -914,10 +914,26 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi
             }
 
             for (var i = 0; i < oldTextWithBreaks.childNodes.length; i++) {
-                this.addCSSClass(oldTextWithBreaks.childNodes[i], 'delete');
+                currChild = oldTextWithBreaks.childNodes[i];
+                if (currChild.nodeType === TEXT_NODE) {
+                    var wrapDel = document.createElement('del');
+                    oldTextWithBreaks.insertBefore(wrapDel, currChild);
+                    oldTextWithBreaks.removeChild(currChild);
+                    wrapDel.appendChild(currChild);
+                } else {
+                    this.addCSSClass(currChild, 'delete');
+                }
             }
             for (i = 0; i < newTextWithBreaks.childNodes.length; i++) {
-                this.addCSSClass(newTextWithBreaks.childNodes[i], 'insert');
+                currChild = newTextWithBreaks.childNodes[i];
+                if (currChild.nodeType === TEXT_NODE) {
+                    var wrapIns = document.createElement('ins');
+                    newTextWithBreaks.insertBefore(wrapIns, currChild);
+                    newTextWithBreaks.removeChild(currChild);
+                    wrapIns.appendChild(currChild);
+                } else {
+                    this.addCSSClass(currChild, 'insert');
+                }
             }
 
             var mergedFragment = document.createDocumentFragment(),

openslides/motions/static/js/motions/linenumbering.js

@@ -343,6 +343,7 @@ angular.module('OpenSlidesApp.motions.lineNumbering', [])
 
             this._currentInlineOffset = 0;
             this._prependLineNumberToFirstText = true;
+            this._ignoreNextRegularLineNumber = false;
 
             return node;
         };
@@ -353,8 +354,11 @@ angular.module('OpenSlidesApp.motions.lineNumbering', [])
             }
             if (this._isIgnoredByLineNumbering(node)) {
                 if (this._currentInlineOffset === 0) {
-                    node.insertBefore(this._createLineNumber(), node.firstChild);
+                    var lineNumberNode = this._createLineNumber();
-                    this._ignoreNextRegularLineNumber = true;
+                    if (lineNumberNode) {
+                        node.insertBefore(lineNumberNode, node.firstChild);
+                        this._ignoreNextRegularLineNumber = true;
+                    }
                 }
                 return node;
             } else if (this._isInlineElement(node)) {

tests/karma/motions/diff.service.test.js

@@ -326,12 +326,28 @@ describe('linenumbering', function () {
       expect(diff).toBe('The <strong>brown</strong> spotted fox <del>jum</del><ins>lea</ins>ped over the rolling log.');
     });
 
-    it('merges multiple inserts and deletes', function () {
+    it('too many changes result in separate paragraphs', function () {
+      var before = "<p>Test1 Test2 Test3 Test4 Test5 Test9</p>",
+          after = "<p>Test1 Test6 Test7 Test8 Test9</p>";
+      var diff = diffService.diff(before, after);
+
+      expect(diff).toBe('<P class="delete">Test1 Test2 Test3 Test4 Test5 Test9</P><P class="insert">Test1 Test6 Test7 Test8 Test9</P>');
+    });
+
+    it('too many changes result in separate paragraphs - special case with un-wrapped text', function () {
       var before = "Test1 Test2 Test3 Test4 Test5 Test9",
           after = "Test1 Test6 Test7 Test8 Test9";
       var diff = diffService.diff(before, after);
 
-      expect(diff).toBe('Test1 <del>Test2 Test3 Test4 Test5 </del><ins>Test6 Test7 Test8 </ins>Test9');
+      expect(diff).toBe('<DEL>Test1 Test2 Test3 Test4 Test5 Test9</DEL><INS>Test1 Test6 Test7 Test8 Test9</INS>');
+    });
+
+    it('merges multiple inserts and deletes', function () {
+      var before = "Some additional text to circumvent the threshold Test1 Test2 Test3 Test4 Test5 Test9",
+          after = "Some additional text to circumvent the threshold Test1 Test6 Test7 Test8 Test9";
+      var diff = diffService.diff(before, after);
+
+      expect(diff).toBe('Some additional text to circumvent the threshold Test1 <del>Test2 Test3 Test4 Test5 </del><ins>Test6 Test7 Test8 </ins>Test9');
     });
 
     it('detects insertions and deletions in a word (1)', function () {
@@ -362,12 +378,16 @@ describe('linenumbering', function () {
       var before = "<P>liebliche Stimme, aber deine Stimme ist rauh; du bist der Wolf.' Da gieng der </P>",
           after = "<p>liebliche Stimme, aber deine Stimme ist rauh; du bist der Wolf.'</p>\
 \
-<p>Der Wolf hatte danach richtig schlechte laune, trank eine Flasche Rum,</p>\
+<p>Der Wolf hatte danach richtig schlechte laune, trank eine Flasche Rum,</p>\
 \
-<p>machte eine Weltreise und kam danach wieder um die Ziegen zu fressen. Da ging der</p>";
+<p>machte eine Weltreise und kam danach wieder um die Ziegen zu fressen. Da ging der</p>",
-      var diff = diffService.diff(before, after);
+      expected = "<P class=\"delete\">liebliche Stimme, aber deine Stimme ist rauh; du bist der Wolf.' Da gieng der </P>" +
+          "<P class=\"insert\">liebliche Stimme, aber deine Stimme ist rauh; du bist der Wolf.'</P>" +
+          "<P class=\"insert\">Der Wolf hatte danach richtig schlechte laune, trank eine Flasche Rum,</P>" +
+          "<P class=\"insert\">machte eine Weltreise und kam danach wieder um die Ziegen zu fressen. Da ging der</P>";
 
-      expect(diff).toBe("<p>liebliche Stimme, aber deine Stimme ist rauh; du bist der <del>Wolf.' </del><ins>Wolf.'</ins></p><p><ins>Der Wolf hatte danach richtig schlechte laune, trank eine Flasche Rum,</ins></p><p><ins>machte eine Weltreise und kam danach wieder um die Ziegen zu fressen. </ins>Da gi<del>e</del>ng der</p>");
+      var diff = diffService.diff(before, after);
+      expect(diff).toBe(expected);
     });
 
     it('handles completely deleted paragraphs', function () {

tests/karma/motions/linenumbering.service.test.js

@@ -241,6 +241,13 @@ describe('linenumbering', function () {
       expect(outHtml).toBe('<p>' + noMarkup(1) + '<span>Duis autem vel eum iriure dolor in hendrerit in vulputate velit esse molestie ' + brMarkup(2) + '<strong>consequat, vel illum dolore eu feugiat nulla facilisis at vero eros et accumsan ' + brMarkup(3) + 'et iusto odio</strong>.</span></p>');
       expect(lineNumberingService.stripLineNumbers(outHtml)).toBe(inHtml);
     });
+
+    it('does not fail in a weird case', function () {
+      var inHtml = "<ins>seid Noch</ins><p></p><p><ins>Test 123</ins></p>";
+      var outHtml = lineNumberingService.insertLineNumbers(inHtml, 80);
+      expect(outHtml).toBe(noMarkup(1) + '<ins>seid Noch</ins><p></p><p>' + noMarkup(2) + '<ins>Test 123</ins></p>');
+      expect(lineNumberingService.stripLineNumbers(outHtml)).toBe(inHtml);
+    });
   });
 
   describe('line numbering in regard to the inline diff', function() {