Bugfix for merging change recommendations

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

@@ -450,6 +450,32 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi
             return ret;
         };
 
+        /*
+         * This is a workardoun to prevent the last word of the inserted text from accidently being merged with the
+         * first word of the following line.
+         *
+         * This happens as trailing spaces in the change recommendation's text are frequently stripped,
+         * which is pretty nasty if the original text goes on after the affected line. So we insert a space
+         * if the original line ends with one.
+         */
+        this._insertDanglingSpace = function(element) {
+            if (element.childNodes.length > 0) {
+                var lastChild = element.childNodes[element.childNodes.length - 1];
+                if (lastChild.nodeType == TEXT_NODE && !lastChild.nodeValue.match(/[\S]/) && element.childNodes.length > 1) {
+                    // If the text node only contains whitespaces, chances are high it's just space between block elmeents,
+                    // like a line break between </LI> and </UL>
+                    lastChild = element.childNodes[element.childNodes.length - 2];
+                }
+                if (lastChild.nodeType == TEXT_NODE) {
+                    if (lastChild.nodeValue === '' || lastChild.nodeValue.substr(-1) != ' ') {
+                        lastChild.nodeValue += ' ';
+                    }
+                } else {
+                    this._insertDanglingSpace(lastChild);
+                }
+            }
+        };
+
         /*
          * This functions merges to arrays of nodes. The last element of nodes1 and the first element of nodes2
          * are merged, if they are of the same type.
@@ -583,6 +609,10 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi
                 followingFragment = this.htmlToFragment(followingHtml),
                 newFragment = this.htmlToFragment(newHTML);
 
+            if (data.html.length > 0 && data.html.substr(-1) == ' ') {
+                this._insertDanglingSpace(newFragment);
+            }
+
             var merged = this._replaceLinesMergeNodeArrays(previousFragment.childNodes, newFragment.childNodes);
             merged = this._replaceLinesMergeNodeArrays(merged, followingFragment.childNodes);
 

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

@@ -221,6 +221,22 @@ describe('linenumbering', function () {
       expect(merged).toBe('<P>Line 1 Line 2 Line <STRONG>3<BR>Line 4 Line</STRONG> 5</P><UL class="ul-class"><LI class="li-class">Line 6 Line 7</LI><LI class="li-class"><UL><LI>Level 2 LI 8</LI><LI>Level 2 LI 9</LI></UL></LI></UL><P>Replaced Line 10</P><P>Inserted Line 11 Line 11</P>');
     });
 
+    it('does not accidently merge two separate words', function() {
+      var merged = diffService.replaceLines(baseHtml1, '<p>Line 1INSERTION</p>', 1, 2),
+          containsError = merged.indexOf("Line 1INSERTIONLine 2"),
+          containsCorrectVersion = merged.indexOf("Line 1INSERTION Line 2");
+      expect(containsError).toBe(-1);
+      expect(containsCorrectVersion).toBe(3);
+    });
+
+    it('does not accidently merge two separate words, even in lists', function() {
+      // The newlines between UL and LI are the problem here
+      var merged = diffService.replaceLines(baseHtml1, '<ul class="ul-class">' + "\n" + '<li class="li-class">Line 6Inserted</li>' + "\n" + '</ul>', 6, 7),
+          containsError = merged.indexOf("Line 6InsertedLine 7"),
+          containsCorrectVersion = merged.indexOf("Line 6Inserted Line 7");
+      expect(containsError).toBe(-1);
+      expect(containsCorrectVersion > 0).toBe(true);
+    });
   });
 
   describe('detecting the type of change', function() {