ag-admin/OpenSlides
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #2690 from CatoTH/bugfix-incorrectly-merging-words

Browse Source 
Bugfix for merging change recommendations
This commit is contained in:
Emanuel Schütze 2016-11-28 21:28:38 +01:00 committed by GitHub
commit b11463fc60
2 changed files with 46 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
openslides/motions/static/js/motions/diff.js
View File

@ -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);

16
tests/karma/motions/diff.service.test.js
View File

@ -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() {