From 4ac7f48bec9bc52f475fa30a6c1a29fadd6c9a14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Ho=CC=88=C3=9Fl?= Date: Sun, 13 Nov 2016 10:20:00 +0100 Subject: [PATCH] Bugfix: Caching of extracted line numbers was invalid when a fragment was passed as an argument --- openslides/motions/static/js/motions/base.js | 3 +- openslides/motions/static/js/motions/diff.js | 37 ++++++++++++-------- openslides/motions/static/js/motions/site.js | 3 +- tests/karma/motions/diff.service.test.js | 35 +++++++++--------- 4 files changed, 44 insertions(+), 34 deletions(-) diff --git a/openslides/motions/static/js/motions/base.js b/openslides/motions/static/js/motions/base.js index a16ecccce..829875a5f 100644 --- a/openslides/motions/static/js/motions/base.js +++ b/openslides/motions/static/js/motions/base.js @@ -248,8 +248,7 @@ angular.module('OpenSlidesApp.motions', [ var change = changes[i]; if (statusCompareCb === undefined || statusCompareCb(change.status)) { html = lineNumberingService.insertLineNumbers(html, lineLength); - fragment = diffService.htmlToFragment(html); - html = diffService.replaceLines(fragment, change.text, change.line_from, change.line_to); + html = diffService.replaceLines(html, change.text, change.line_from, change.line_to); } } diff --git a/openslides/motions/static/js/motions/diff.js b/openslides/motions/static/js/motions/diff.js index b8a5411d8..e9b4bfc5b 100644 --- a/openslides/motions/static/js/motions/diff.js +++ b/openslides/motions/static/js/motions/diff.js @@ -326,17 +326,20 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi * - followingHtmlStartSnippet: A HTML snippet that opens all HTML tags necessary to render "followingHtml" * */ - this.extractRangeByLineNumbers = function(fragment, fromLine, toLine, debug) { + this.extractRangeByLineNumbers = function(htmlIn, fromLine, toLine, debug) { + if (typeof(htmlIn) !== 'string') { + throw 'Invalid call - extractRangeByLineNumbers expects a string as first argument'; + } - var cacheKey = fromLine + "-" + toLine + "-" + lineNumberingService.djb2hash(fragment), + var cacheKey = fromLine + "-" + toLine + "-" + lineNumberingService.djb2hash(htmlIn), cached = diffCache.get(cacheKey); + if (!angular.isUndefined(cached)) { return cached; } - if (typeof(fragment) == 'string') { - fragment = this.htmlToFragment(fragment); - } + var fragment = this.htmlToFragment(htmlIn); + this._insertInternalLineMarkers(fragment); this._insertInternalLiNumbers(fragment); if (toLine === null) { @@ -353,7 +356,7 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi toChildTraceRel = ancestorData.trace2, toChildTraceAbs = this._getNodeContextTrace(toLineNode), ancestor = ancestorData.commonAncestor, - html = '', + htmlOut = '', outerContextStart = '', outerContextEnd = '', innerContextStart = '', @@ -407,13 +410,13 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi if (ancestor.childNodes[i] == fromChildTraceRel[0]) { found = true; fromChildTraceRel.shift(); - html += this._serializePartialDomFromChild(ancestor.childNodes[i], fromChildTraceRel, true); + htmlOut += this._serializePartialDomFromChild(ancestor.childNodes[i], fromChildTraceRel, true); } else if (ancestor.childNodes[i] == toChildTraceRel[0]) { found = false; toChildTraceRel.shift(); - html += this._serializePartialDomToChild(ancestor.childNodes[i], toChildTraceRel, true); + htmlOut += this._serializePartialDomToChild(ancestor.childNodes[i], toChildTraceRel, true); } else if (found === true) { - html += this._serializeDom(ancestor.childNodes[i], true); + htmlOut += this._serializeDom(ancestor.childNodes[i], true); } } @@ -431,7 +434,7 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi } var ret = { - 'html': html, + 'html': htmlOut, 'ancestor': ancestor, 'outerContextStart': outerContextStart, 'outerContextEnd': outerContextEnd, @@ -566,8 +569,14 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi return type; }; - this.replaceLines = function (fragment, newHTML, fromLine, toLine) { - var data = this.extractRangeByLineNumbers(fragment, fromLine, toLine), + /** + * @param {string} oldHtml + * @param {string} newHTML + * @param {number} fromLine + * @param {number} toLine + */ + this.replaceLines = function (oldHtml, newHTML, fromLine, toLine) { + var data = this.extractRangeByLineNumbers(oldHtml, fromLine, toLine), previousHtml = data.previousHtml + '' + data.previousHtmlEndSnippet, previousFragment = this.htmlToFragment(previousHtml), followingHtml = data.followingHtmlStartSnippet + '' + data.followingHtml, @@ -603,8 +612,8 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi node.setAttribute('class', classes); }; - this.addDiffMarkup = function (fragment, newHTML, fromLine, toLine, diffFormatterCb) { - var data = this.extractRangeByLineNumbers(fragment, fromLine, toLine), + this.addDiffMarkup = function (originalHTML, newHTML, fromLine, toLine, diffFormatterCb) { + var data = this.extractRangeByLineNumbers(originalHTML, fromLine, toLine), previousHtml = data.previousHtml + '' + data.previousHtmlEndSnippet, previousFragment = this.htmlToFragment(previousHtml), followingHtml = data.followingHtmlStartSnippet + '' + data.followingHtml, diff --git a/openslides/motions/static/js/motions/site.js b/openslides/motions/static/js/motions/site.js index 4f0534610..2f2cacca8 100644 --- a/openslides/motions/static/js/motions/site.js +++ b/openslides/motions/static/js/motions/site.js @@ -1251,8 +1251,7 @@ angular.module('OpenSlidesApp.motions.site', [ $scope.alert = {}; var html = motion.getTextWithLineBreaks(version), - fragment = diffService.htmlToFragment(html), - lineData = diffService.extractRangeByLineNumbers(fragment, lineFrom, lineTo); + lineData = diffService.extractRangeByLineNumbers(html, lineFrom, lineTo); $scope.model = { text: lineData.outerContextStart + lineData.innerContextStart + diff --git a/tests/karma/motions/diff.service.test.js b/tests/karma/motions/diff.service.test.js index b9b107851..1d7acc4cc 100644 --- a/tests/karma/motions/diff.service.test.js +++ b/tests/karma/motions/diff.service.test.js @@ -2,7 +2,7 @@ describe('linenumbering', function () { beforeEach(module('OpenSlidesApp.motions.diff')); - var diffService, baseHtmlDom1, baseHtmlDom2, baseHtmlDom3, + var diffService, baseHtml1, baseHtmlDom1, baseHtml2, baseHtmlDom2, baseHtml3, baseHtmlDom3, brMarkup = function (no) { return '
' + ' '; @@ -14,7 +14,7 @@ describe('linenumbering', function () { beforeEach(inject(function (_diffService_) { diffService = _diffService_; - baseHtmlDom1 = diffService.htmlToFragment('

' + + baseHtml1 = '

' + noMarkup(1) + 'Line 1 ' + brMarkup(2) + 'Line 2 ' + brMarkup(3) + 'Line 3
' + noMarkup(4) + 'Line 4 ' + brMarkup(5) + 'Line 5

' + '' + '' + - '

' + noMarkup(10) + 'Line 10 ' + brMarkup(11) + 'Line 11

'); + '

' + noMarkup(10) + 'Line 10 ' + brMarkup(11) + 'Line 11

'; + baseHtmlDom1 = diffService.htmlToFragment(baseHtml1); - baseHtmlDom2 = diffService.htmlToFragment('

' + noMarkup(1) + 'Single text line

\ + baseHtml2 = '

' + noMarkup(1) + 'Single text line

\

' + noMarkup(2) + 'sdfsdfsdfsdf dsfsdfsdfdsflkewjrl ksjfl ksdjf klnlkjBavaria ipsum dolor sit amet Biazelt Auffisteign ' + brMarkup(3) + 'Schorsch mim Radl foahn Ohrwaschl Steckerleis wann griagd ma nacha wos z’dringa glacht Mamalad, ' + brMarkup(4) + 'muass? I bin a woschechta Bayer sowos oamoi und sei und glei wirds no fui lustiga: Jo mei khkhis des ' + brMarkup(5) + 'schee middn ognudelt, Trachtnhuat Biawambn gscheid: Griasd eich midnand etza nix Gwiass woass ma ned ' + brMarkup(6) + 'owe. Dahoam gscheckate middn Spuiratz des is a gmahde Wiesn. Des is schee so Obazda san da, Haferl ' + brMarkup(7) + 'pfenningguat schoo griasd eich midnand.

\ @@ -45,9 +46,10 @@ describe('linenumbering', function () {

' + noMarkup(24) + 'Wui helfgod Wiesn, ognudelt schaugn: Dahoam gelbe Rüam Schneid singan wo hi sauba i moan scho aa no ' + brMarkup(25) + 'a Maß a Maß und no a Maß nimma. Is umananda a ganze Hoiwe zwoa, Schneid. Vui huift vui Brodzeid kumm ' + brMarkup(26) + 'geh naa i daad vo de allerweil, gor. Woaß wia Gams, damischa. A ganze Hoiwe Ohrwaschl Greichats ' + brMarkup(27) + 'iabaroi Prosd Engelgwand nix Reiwadatschi.Weibaleid ognudelt Ledahosn noch da Giasinga Heiwog i daad ' + brMarkup(28) + 'Almrausch, Ewig und drei Dog nackata wea ko, dea ko. Meidromml Graudwiggal nois dei, nackata. No ' + brMarkup(29) + 'Diandldrahn nix Gwiass woass ma ned hod boarischer: Samma sammawiedaguad wos, i hoam Brodzeid. Jo ' + - brMarkup(30) + 'mei Sepp Gaudi, is ma Wuascht do Hendl Xaver Prosd eana an a bravs. Sauwedda an Brezn, abfieseln.

'); + brMarkup(30) + 'mei Sepp Gaudi, is ma Wuascht do Hendl Xaver Prosd eana an a bravs. Sauwedda an Brezn, abfieseln.

'; + baseHtmlDom2 = diffService.htmlToFragment(baseHtml2); - baseHtmlDom3 = diffService.htmlToFragment('
    ' + + baseHtml3 = '
      ' + '
    1. ' + noMarkup(1) + 'Line 1
      2. ' + '
    2. ' + noMarkup(2) + 'Line 2
      3. ' + '
      3. ' + @@ -55,7 +57,8 @@ describe('linenumbering', function () { '
      1. ' + noMarkup(4) + 'Line 3.2
        2. ' + '
      2. ' + noMarkup(5) + 'Line 3.3
        3. ' + '
      4. ' + - '
    4. ' + noMarkup(6) + ' Line 4
    '); + '
  1. ' + noMarkup(6) + ' Line 4
'; + baseHtmlDom3 = diffService.htmlToFragment(baseHtml3); diffService._insertInternalLineMarkers(baseHtmlDom1); diffService._insertInternalLineMarkers(baseHtmlDom2); @@ -122,14 +125,14 @@ describe('linenumbering', function () { }); it('extracts a single line', function () { - var diff = diffService.extractRangeByLineNumbers(baseHtmlDom1, 1, 2); + var diff = diffService.extractRangeByLineNumbers(baseHtml1, 1, 2); expect(diff.html).toBe('

Line 1 '); expect(diff.outerContextStart).toBe(''); expect(diff.outerContextEnd).toBe(''); }); it('extracts lines from nested UL/LI-structures', function () { - var diff = diffService.extractRangeByLineNumbers(baseHtmlDom1, 7, 9); + var diff = diffService.extractRangeByLineNumbers(baseHtml1, 7, 9); expect(diff.html).toBe('Line 7