From 3d2e13c21bf1a8b91dbb3e222260d3ad6c0aee62 Mon Sep 17 00:00:00 2001 From: Wyatt Allen Date: Wed, 13 Jul 2016 14:34:23 -0700 Subject: [PATCH] Organizes gr-annotation tests and fixes `splitTextNode` bug Moves the tests for gr-annotation functions into their own test file and fixes a subtle bug regarding `splitTextNode`'s Unicode branch. In the DOM implementation of `node.splitText`, `node` is kept in the DOM and its `textContent` is modified, whereas the Unicode path of `splitTextNode` would replace it with an entirely new Text node. This led to the function behaving differently when the Node contained or did not contain astral code-points. With this change, `splitTextNode` more-closely behaves like `splitText` and this behavior is captured in a new unit-test. Change-Id: I70460694040ba9a3c49937aaafc9db261ca3be3d --- .../diff/gr-diff-highlight/gr-annotation.js | 12 +- .../gr-diff-highlight/gr-annotation_test.html | 190 ++++++++++++++++++ .../gr-diff-highlight_test.html | 154 -------------- polygerrit-ui/app/test/index.html | 1 + 4 files changed, 199 insertions(+), 158 deletions(-) create mode 100644 polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation_test.html diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation.js index f0a1b9f503..ef83ebf625 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation.js @@ -164,11 +164,15 @@ // TODO (viktard): Polyfill Array.from for IE10. var head = Array.from(node.textContent); var tail = head.splice(offset); - var parent = node.parentElement; - var headNode = document.createTextNode(head.join('')); - parent.replaceChild(headNode, node); + var parent = node.parentNode; + + // Split the content of the original node. + node.textContent = head.join(''); + var tailNode = document.createTextNode(tail.join('')); - parent.insertBefore(tailNode, headNode.nextSibling); + if (parent) { + parent.insertBefore(tailNode, node.nextSibling); + } return tailNode; } else { return node.splitText(offset); diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation_test.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation_test.html new file mode 100644 index 0000000000..27a684ddb3 --- /dev/null +++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation_test.html @@ -0,0 +1,190 @@ + + + + +gr-annotation + + + + + + + + + + + + diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html index 88f6be5600..ddc081eb32 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html @@ -20,7 +20,6 @@ limitations under the License. - @@ -892,159 +891,6 @@ limitations under the License. assert.equal(getActionSide(), 'left'); }); - suite('annotation', function() { - var str; - var parent; - var textNode; - - setup(function() { - str = 'Lorem ipsum dolor sit amet, suspendisse inceptos vehicula'; - parent = document.createElement('div'); - parent.appendChild(document.createTextNode(str)); - textNode = parent.childNodes[0]; - }); - - test('library is loaded', function() { - assert.isOk(GrAnnotation); - }); - - test('_annotateText Case 1', function() { - assert.equal(parent.childNodes.length, 1); - assert.instanceOf(parent.childNodes[0], Text); - - GrAnnotation._annotateText(textNode, 0, str.length, 'foobar'); - - assert.equal(parent.childNodes.length, 1); - assert.instanceOf(parent.childNodes[0], HTMLElement); - assert.equal(parent.childNodes[0].className, 'foobar'); - assert.instanceOf(parent.childNodes[0].childNodes[0], Text); - assert.equal(parent.childNodes[0].childNodes[0].textContent, str); - }); - - test('_annotateText Case 2', function() { - var length = 12; - var substr = str.substr(0, length); - var remainder = str.substr(length); - - assert.equal(parent.childNodes.length, 1); - assert.instanceOf(parent.childNodes[0], Text); - - GrAnnotation._annotateText(textNode, 0, length, 'foobar'); - - assert.equal(parent.childNodes.length, 2); - - assert.instanceOf(parent.childNodes[0], HTMLElement); - assert.equal(parent.childNodes[0].className, 'foobar'); - assert.instanceOf(parent.childNodes[0].childNodes[0], Text); - assert.equal(parent.childNodes[0].childNodes[0].textContent, substr); - - assert.instanceOf(parent.childNodes[1], Text); - assert.equal(parent.childNodes[1].textContent, remainder); - }); - - test('_annotateText Case 3', function() { - var index = 12; - var length = str.length - index; - var remainder = str.substr(0, index); - var substr = str.substr(index); - - assert.equal(parent.childNodes.length, 1); - assert.instanceOf(parent.childNodes[0], Text); - - GrAnnotation._annotateText(textNode, index, length, 'foobar'); - - assert.equal(parent.childNodes.length, 2); - - assert.instanceOf(parent.childNodes[0], Text); - assert.equal(parent.childNodes[0].textContent, remainder); - - assert.instanceOf(parent.childNodes[1], HTMLElement); - assert.equal(parent.childNodes[1].className, 'foobar'); - assert.instanceOf(parent.childNodes[1].childNodes[0], Text); - assert.equal(parent.childNodes[1].childNodes[0].textContent, substr); - }); - - test('_annotateText Case 4', function() { - var index = str.indexOf('dolor'); - var length = 'dolor '.length; - - var remainderPre = str.substr(0, index); - var substr = str.substr(index, length); - var remainderPost = str.substr(index + length); - - assert.equal(parent.childNodes.length, 1); - assert.instanceOf(parent.childNodes[0], Text); - - GrAnnotation._annotateText(textNode, index, length, 'foobar'); - - assert.equal(parent.childNodes.length, 3); - - assert.instanceOf(parent.childNodes[0], Text); - assert.equal(parent.childNodes[0].textContent, remainderPre); - - assert.instanceOf(parent.childNodes[1], HTMLElement); - assert.equal(parent.childNodes[1].className, 'foobar'); - assert.instanceOf(parent.childNodes[1].childNodes[0], Text); - assert.equal(parent.childNodes[1].childNodes[0].textContent, substr); - - assert.instanceOf(parent.childNodes[2], Text); - assert.equal(parent.childNodes[2].textContent, remainderPost); - }); - - test('_annotateElement design doc example', function() { - var layers = [ - 'amet, ', - 'inceptos ', - 'amet, ', - 'et, suspendisse ince' - ]; - - // Apply the layers successively. - layers.forEach(function(layer, i) { - GrAnnotation.annotateElement( - parent, str.indexOf(layer), layer.length, 'layer-' + (i + 1)); - }); - - assert.equal(parent.textContent, str); - - // Layer 1: - var layer1 = parent.querySelectorAll('.layer-1'); - assert.equal(layer1.length, 1); - assert.equal(layer1[0].textContent, layers[0]); - assert.equal(layer1[0].parentElement, parent); - - // Layer 2: - var layer2 = parent.querySelectorAll('.layer-2'); - assert.equal(layer2.length, 1); - assert.equal(layer2[0].textContent, layers[1]); - assert.equal(layer2[0].parentElement, parent); - - // Layer 3: - var layer3 = parent.querySelectorAll('.layer-3'); - assert.equal(layer3.length, 1); - assert.equal(layer3[0].textContent, layers[2]); - assert.equal(layer3[0].parentElement, layer1[0]); - - // Layer 4: - var layer4 = parent.querySelectorAll('.layer-4'); - assert.equal(layer4.length, 3); - - assert.equal(layer4[0].textContent, 'et, '); - assert.equal(layer4[0].parentElement, layer3[0]); - - assert.equal(layer4[1].textContent, 'suspendisse '); - assert.equal(layer4[1].parentElement, parent); - - assert.equal(layer4[2].textContent, 'ince'); - assert.equal(layer4[2].parentElement, layer2[0]); - - assert.equal(layer4[0].textContent + - layer4[1].textContent + - layer4[2].textContent, - layers[3]); - }); - }); - // TODO (viktard): Selection starts in line number. // TODO (viktard): Empty lines in selection start. // TODO (viktard): Empty lines in selection end. diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html index 153db20326..feb4f49ab5 100644 --- a/polygerrit-ui/app/test/index.html +++ b/polygerrit-ui/app/test/index.html @@ -48,6 +48,7 @@ limitations under the License. 'diff/gr-diff-comment/gr-diff-comment_test.html', 'diff/gr-diff-cursor/gr-diff-cursor_test.html', 'diff/gr-diff-highlight/gr-diff-highlight_test.html', + 'diff/gr-diff-highlight/gr-annotation_test.html', 'diff/gr-diff-preferences/gr-diff-preferences_test.html', 'diff/gr-diff-processor/gr-diff-processor_test.html', 'diff/gr-diff-selection/gr-diff-selection_test.html',