From 6891471c4558bfa1ebf6397bfd4506c724b69f7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Ho=CC=88=C3=9Fl?= Date: Sun, 29 Oct 2017 13:51:30 +0100 Subject: [PATCH] Better handling of inserted paragraphs --- CHANGELOG | 2 +- openslides/motions/static/js/motions/diff.js | 59 +++++++++++++++++++- tests/karma/motions/diff.service.test.js | 18 +++++- 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 5ce071a13..9bc398b6c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -38,7 +38,7 @@ Motions: - Bugfix: Several bugfixes regarding splitting list items in change recommendations [#3288]. - Bugfix: Several bugfixes regarding diff version [#3407, #3408, #3410, - #3440, #3450]. + #3440, #3450, #3465]. - Added inline Editing for motion reason [#3361]. - Added multiselect filter for motion comments [#3372]. - Added support for pinning personal notes to the window [#3360]. diff --git a/openslides/motions/static/js/motions/diff.js b/openslides/motions/static/js/motions/diff.js index 6dc646ffa..04deff5bd 100644 --- a/openslides/motions/static/js/motions/diff.js +++ b/openslides/motions/static/js/motions/diff.js @@ -626,6 +626,7 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi this._normalizeHtmlForDiff = function (html) { // Convert all HTML tags to uppercase, but leave the values of attributes unchanged // All attributes and CSS class names are sorted alphabetically + // If an attribute is empty, it is removed html = html.replace(/<(\/?[a-z]*)( [^>]*)?>/ig, function (html, tag, attributes) { var tagNormalized = tag.toUpperCase(); if (attributes === undefined) { @@ -641,11 +642,13 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi attrValue = match[5]; if (match[2] !== undefined) { if (attrNormalized === ' CLASS') { - attrValue = attrValue.split(' ').sort().join(' '); + attrValue = attrValue.split(' ').sort().join(' ').replace(/^\s+/, '').replace(/\s+$/, ''); } attrNormalized += "=" + match[4] + attrValue + match[4]; } - attributesList.push(attrNormalized); + if (attrValue !== '') { + attributesList.push(attrNormalized); + } } } while (match); attributes = attributesList.sort().join(''); @@ -1113,6 +1116,24 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi }); }; + this._addClassToLastNode = function (html, className) { + var node = document.createElement('div'); + node.innerHTML = html; + var foundLast = false; + for (var i = node.childNodes.length - 1; i >= 0 && !foundLast; i--) { + if (node.childNodes[i].nodeType === ELEMENT_NODE) { + var classes = []; + if (node.childNodes[i].getAttribute("class")) { + classes = node.childNodes[i].getAttribute("class").split(" "); + } + classes.push(className); + node.childNodes[i].setAttribute("class", classes.sort().join(' ').replace(/^\s+/, '').replace(/\s+$/, '')); + foundLast = true; + } + } + return node.innerHTML; + }; + /** * This function removes color-Attributes from the styles of this node or a descendant, * as they interfer with the green/red color in HTML and PDF @@ -1173,6 +1194,22 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi // to make sure this situation does not happen (and uses invisible pseudo-tags in case something goes wrong) var workaroundPrepend = ""; + // os-split-after should not be considered for detecting changes in paragraphs, so we strip it here + // and add it afterwards. + // We only do this for P for now, as for more complex types like UL/LI that tend to be nestend, + // information would get lost by this that we will need to recursively merge it again later on. + var oldIsSplitAfter = false, + newIsSplitAfter = false; + htmlOld = htmlOld.replace(/(\s*]+class\s*=\s*["'][^"']*)os-split-after/gi, function(match, beginning) { + oldIsSplitAfter = true; + return beginning; + }); + htmlNew = htmlNew.replace(/(\s*]+class\s*=\s*["'][^"']*)os-split-after/gi, function(match, beginning) { + newIsSplitAfter = true; + return beginning; + }); + + // Performing the actual diff var str = this._diffString(workaroundPrepend + htmlOld, workaroundPrepend + htmlNew), diffUnnormalized = str.replace(/^\s+/g, '').replace(/\s+$/g, '').replace(/ {2,}/g, ' '); @@ -1201,6 +1238,20 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi } ); + // Fixes HTML produced by the diff like this: + // from:

Inserted Text

\n

More inserted text

 + // into: Inserted Text

\n

More inserted text

+ diffUnnormalized = diffUnnormalized.replace( + /<\/p><\/del>([\s\S]*?)<\/p><\/ins>/gim, + "$1

" + ); + diffUnnormalized = diffUnnormalized.replace( + /[\s\S]*?<\/ins>/gim, + function(match) { + return match.replace(/(<\/p>\s*

)/gi, "$1"); + } + ); + // If only a few characters of a word have changed, don't display this as a replacement of the whole word, // but only of these specific characters diffUnnormalized = diffUnnormalized.replace(/([a-z0-9,_-]* ?)<\/del>([a-z0-9,_-]* ?)<\/ins>/gi, function (found, oldText, newText) { @@ -1279,6 +1330,10 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi } } + if (oldIsSplitAfter || newIsSplitAfter) { + diff = this._addClassToLastNode(diff, "os-split-after"); + } + diffCache.put(cacheKey, diff); return diff; }; diff --git a/tests/karma/motions/diff.service.test.js b/tests/karma/motions/diff.service.test.js index 3348e4984..fd46b39bd 100644 --- a/tests/karma/motions/diff.service.test.js +++ b/tests/karma/motions/diff.service.test.js @@ -502,9 +502,21 @@ describe('linenumbering', function () { var before = "

rief sie alle sieben herbei und sprach 'liebe Kinder, ich will hinaus in den Wald, seid

", after = "

rief sie alle sieben herbei und sprach 'liebe Kinder, ich will hinaus in den Wald, seid Noch

" + "

Test 123

", - expected = "

rief sie alle sieben herbei und sprach 'liebe Kinder, ich will hinaus in den Wald, seid

" + - "

rief sie alle sieben herbei und sprach 'liebe Kinder, ich will hinaus in den Wald, seid Noch

" + - "

Test 123

"; + expected = "

rief sie alle sieben herbei und sprach 'liebe Kinder, ich will hinaus in den Wald, seid Noch

" + + "

Test 123

"; + + var diff = diffService.diff(before, after); + expect(diff).toBe(expected); + }); + + it('handles insterted paragraphs (3)', function () { + // Hint: os-split-after should be moved from the first paragraph to the second one + var before = "

 Lorem ipsum dolor sit amet, consetetur sadipscing elitr,

", + after = "

Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum.

\n" + + "\n" + + "

Stet clita kasd gubergren, no sea takimata sanctus est.

", + expected = "

 Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum.

\n" + + "

Stet clita kasd gubergren, no sea takimata sanctus est.

"; var diff = diffService.diff(before, after); expect(diff).toBe(expected);