Fix intraline highlight w/ surrogate pairs
For intraline highlighting, line lengths are currently determined by looking at `String.prototype.length` which counts code units, not characters. This breaks for strings that contain surrogate pairs (e.g. emojis).
This change fixes these instances by using `GrAnnotation.getStringLength` instead, which returns character counts instead.
Release-Notes: Fix intraline highlight w/ surrogate pairs
Change-Id: I3f9977d828e4b70f1387e01f7015c49ca3da536e
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
index 6cd3cb0..d64b164 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
@@ -518,7 +518,7 @@
// If endIndex isn't present, continue to the end of the line.
const endIndex =
highlight.endIndex === undefined
- ? line.text.length
+ ? GrAnnotation.getStringLength(line.text)
: highlight.endIndex;
GrAnnotation.annotateElement(
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts
index 5eabc59..0f02d71 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts
@@ -271,10 +271,11 @@
const str0 = slice(str, 0, 6);
const str1 = slice(str, 6);
+ const numHighlightedChars = GrAnnotation.getStringLength(str1);
layer.annotate(el, lineNumberEl, l, Side.LEFT);
- assert.isTrue(annotateElementSpy.called);
+ assert.isTrue(annotateElementSpy.calledWith(el, 6, numHighlightedChars));
assert.equal(el.childNodes.length, 2);
assert.instanceOf(el.childNodes[0], Text);
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor.ts b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor.ts
index 9f874b8..22a71a5 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor.ts
@@ -21,6 +21,7 @@
import {debounce, DelayedTask} from '../../../utils/async-util';
import {RenderPreferences} from '../../../api/diff';
import {assertIsDefined} from '../../../utils/common-util';
+import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
const WHOLE_FILE = -1;
@@ -639,16 +640,19 @@
rows: string[],
intralineInfos: number[][]
): Highlights[] {
+ // +1 to account for the \n that is not part of the rows passed here
+ const lineLengths = rows.map(r => GrAnnotation.getStringLength(r) + 1);
+
let rowIndex = 0;
let idx = 0;
const normalized = [];
for (const [skipLength, markLength] of intralineInfos) {
- let line = rows[rowIndex] + '\n';
+ let lineLength = lineLengths[rowIndex];
let j = 0;
while (j < skipLength) {
- if (idx === line.length) {
+ if (idx === lineLength) {
idx = 0;
- line = rows[++rowIndex] + '\n';
+ lineLength = lineLengths[++rowIndex];
continue;
}
idx++;
@@ -660,10 +664,10 @@
};
j = 0;
- while (line && j < markLength) {
- if (idx === line.length) {
+ while (lineLength && j < markLength) {
+ if (idx === lineLength) {
idx = 0;
- line = rows[++rowIndex] + '\n';
+ lineLength = lineLengths[++rowIndex];
normalized.push(lineHighlight);
lineHighlight = {
contentIndex: rowIndex,
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
index 6caeb62..f6f7052 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
@@ -723,6 +723,25 @@
endIndex: 41,
},
]);
+
+ content = ['🙈 a', '🙉 b', '🙊 c'];
+ highlights = [[2, 7]];
+ results = element.convertIntralineInfos(content, highlights);
+ assert.deepEqual(results, [
+ {
+ contentIndex: 0,
+ startIndex: 2,
+ },
+ {
+ contentIndex: 1,
+ startIndex: 0,
+ },
+ {
+ contentIndex: 2,
+ startIndex: 0,
+ endIndex: 1,
+ },
+ ]);
});
test('scrolling pauses rendering', () => {
