Fix `annotateText()` for nodes of length 0
This has come up while investigating comment range highlighting of
comments that have `end_char:0`.
Alternatively, we could just forbid annotations of length 0, but maybe
they can be useful, so let's just make them work with this simple fix.
Release-Notes: skip
Google-Bug-Id: b/259670228
Change-Id: Ieb5535ebcf2cf3586f70dcf080209d10ba7ad52e
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation.ts b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation.ts
index 92175eb..38bd707 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation.ts
@@ -171,18 +171,20 @@
cssClass: string,
firstPart?: boolean
) {
- if (this.getLength(node) === offset || offset === 0) {
- return this.wrapInHighlight(node, cssClass);
- } else {
- if (firstPart) {
- this.splitNode(node, offset);
- // Node points to first part of the Text, second one is sibling.
- } else {
- // if node is Text then splitNode will return a Text
- node = this.splitNode(node, offset) as Text;
- }
+ if (
+ (this.getLength(node) === offset && firstPart) ||
+ (offset === 0 && !firstPart)
+ ) {
return this.wrapInHighlight(node, cssClass);
}
+ if (firstPart) {
+ this.splitNode(node, offset);
+ // Node points to first part of the Text, second one is sibling.
+ } else {
+ // if node is Text then splitNode will return a Text
+ node = this.splitNode(node, offset) as Text;
+ }
+ return this.wrapInHighlight(node, cssClass);
},
/**
@@ -225,7 +227,6 @@
*/
splitTextNode(node: Text, offset: number) {
if (node.textContent?.match(REGEX_ASTRAL_SYMBOL)) {
- // TODO (viktard): Polyfill Array.from for IE10.
const head = Array.from(node.textContent);
const tail = head.splice(offset);
const parent = node.parentNode;
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation_test.ts
index 593a686..f319a3c 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-annotation_test.ts
@@ -27,6 +27,36 @@
str = textNode.textContent!;
});
+ test('_annotateText length:0 offset:0', () => {
+ GrAnnotation._annotateText(textNode, 0, 0, 'foobar');
+
+ assert.equal(parent.textContent, str);
+ assert.equal(
+ parent.innerHTML,
+ '<hl class="foobar"></hl>Lorem ipsum dolor sit amet, suspendisse inceptos vehicula'
+ );
+ });
+
+ test('_annotateText length:0 offset:1', () => {
+ GrAnnotation._annotateText(textNode, 1, 0, 'foobar');
+
+ assert.equal(parent.textContent, str);
+ assert.equal(
+ parent.innerHTML,
+ 'L<hl class="foobar"></hl>orem ipsum dolor sit amet, suspendisse inceptos vehicula'
+ );
+ });
+
+ test('_annotateText length:0 offset:str.length', () => {
+ GrAnnotation._annotateText(textNode, str.length, 0, 'foobar');
+
+ assert.equal(parent.textContent, str);
+ assert.equal(
+ parent.innerHTML,
+ 'Lorem ipsum dolor sit amet, suspendisse inceptos vehicula<hl class="foobar"></hl>'
+ );
+ });
+
test('_annotateText Case 1', () => {
GrAnnotation._annotateText(textNode, 0, str.length, 'foobar');
