From 189194bd7b9ab6463d06b7fc30b793cd829c3641 Mon Sep 17 00:00:00 2001 From: Tomina Date: Mon, 17 Feb 2025 15:17:11 +0100 Subject: [PATCH] fix: prevent surrogate pairs from being split by the selected range This is already handled in functions like `expandSelectionInDirection` which won't select a half of emoji etc. but always the whole surrogate pair. The solution here is the same, if the cursor is placed in the middle of a surrogate pair, it will be moved to the end of the pair. Similarly a selection will either select both or neither. --- src/test/system.js | 1 + src/test/system/set_selected_range_test.js | 193 +++++++++++++++++++++ src/trix/models/composition.js | 13 +- 3 files changed, 206 insertions(+), 1 deletion(-) create mode 100644 src/test/system/set_selected_range_test.js diff --git a/src/test/system.js b/src/test/system.js index 513f7a4f8..a1d3c3b1c 100644 --- a/src/test/system.js +++ b/src/test/system.js @@ -17,5 +17,6 @@ import "test/system/level_2_input_test" import "test/system/list_formatting_test" import "test/system/mutation_input_test" import "test/system/pasting_test" +import "test/system/set_selected_range_test" import "test/system/text_formatting_test" import "test/system/undo_test" diff --git a/src/test/system/set_selected_range_test.js b/src/test/system/set_selected_range_test.js new file mode 100644 index 000000000..e9edc1fc9 --- /dev/null +++ b/src/test/system/set_selected_range_test.js @@ -0,0 +1,193 @@ +import { assert, insertString, test, testGroup } from "test/test_helper" + +testGroup( + "Set selected range at the start of the text", + { template: "editor_empty" }, + () => { + test("selection of the surrogate pair", () => { + insertString("🙂foo") + getComposition().setSelectedRange([ 0, 2 ]) + assert.selectedRange([ 0, 2 ]) + }) + + test("selection of the first surrogate pair character", () => { + insertString("🙂foo") + getComposition().setSelectedRange([ 0, 1 ]) + assert.selectedRange([ 0, 2 ]) + }) + + test("selection of the second surrogate pair character", () => { + insertString("🙂foo") + getComposition().setSelectedRange([ 1, 2 ]) + assert.selectedRange([ 2, 2 ]) + }) + + test("collapssed selection in the middle of the surrogate pair", () => { + insertString("🙂foo") + getComposition().setSelectedRange([ 1, 1 ]) + assert.selectedRange([ 2, 2 ]) + }) + + test("collapsed selection after surrogate pair", () => { + insertString("🙂foo") + getComposition().setSelectedRange([ 2, 2 ]) + assert.selectedRange([ 2, 2 ]) + }) + + test("selection after surrogate pair", () => { + insertString("🙂foo") + getComposition().setSelectedRange([ 2, 4 ]) + assert.selectedRange([ 2, 4 ]) + }) + + test("collapsed selection far after surrogate pair", () => { + insertString("🙂foo") + getComposition().setSelectedRange([ 3, 3 ]) + assert.selectedRange([ 3, 3 ]) + }) + + test("selection far after surrogate pair", () => { + insertString("🙂foo") + getComposition().setSelectedRange([ 3, 5 ]) + assert.selectedRange([ 3, 5 ]) + }) + }, +) + +testGroup( + "Set selected range in the middle of the text", + { template: "editor_empty" }, + () => { + test("collapsed selection far before surrogate pair", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 2, 2 ]) + assert.selectedRange([ 2, 2 ]) + }) + + test("selection far before surrogate pair", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 0, 2 ]) + assert.selectedRange([ 0, 2 ]) + }) + + test("collapsed selection before surrogate pair", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 3, 3 ]) + assert.selectedRange([ 3, 3 ]) + }) + + test("selection before surrogate pair", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 1, 3 ]) + assert.selectedRange([ 1, 3 ]) + }) + + test("selection of the surrogate pair", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 3, 5 ]) + assert.selectedRange([ 3, 5 ]) + }) + + test("selection of the first surrogate pair character", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 3, 4 ]) + assert.selectedRange([ 3, 5 ]) + }) + + test("selection of the second surrogate pair character", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 4, 5 ]) + assert.selectedRange([ 5, 5 ]) + }) + + test("collapssed selection in the middle of the surrogate pair", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 4, 4 ]) + assert.selectedRange([ 5, 5 ]) + }) + + test("selection after surrogate pair", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 5, 7 ]) + assert.selectedRange([ 5, 7 ]) + }) + + test("collapsed selection after surrogate pair", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 5, 5 ]) + assert.selectedRange([ 5, 5 ]) + }) + + test("selection far after surrogate pair", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 6, 8 ]) + assert.selectedRange([ 6, 8 ]) + }) + + test("collapsed selection far after surrogate pair", () => { + insertString("foo🙂bar") + getComposition().setSelectedRange([ 6, 6 ]) + assert.selectedRange([ 6, 6 ]) + }) + }, +) + +testGroup( + "Set selected range at the end of the text", + { template: "editor_empty" }, + () => { + test("collapsed selection far before surrogate pair", () => { + insertString("foo🙂") + getComposition().setSelectedRange([ 2, 2 ]) + assert.selectedRange([ 2, 2 ]) + }) + + test("selection far before surrogate pair", () => { + insertString("foo🙂") + getComposition().setSelectedRange([ 0, 2 ]) + assert.selectedRange([ 0, 2 ]) + }) + + test("collapsed selection before surrogate pair", () => { + insertString("foo🙂") + getComposition().setSelectedRange([ 3, 3 ]) + assert.selectedRange([ 3, 3 ]) + }) + + test("selection before surrogate pair", () => { + insertString("foo🙂") + getComposition().setSelectedRange([ 1, 3 ]) + assert.selectedRange([ 1, 3 ]) + }) + + test("selection of the surrogate pair", () => { + insertString("foo🙂") + getComposition().setSelectedRange([ 3, 5 ]) + assert.selectedRange([ 3, 5 ]) + }) + + test("selection of the first surrogate pair character", () => { + insertString("foo🙂") + getComposition().setSelectedRange([ 3, 4 ]) + assert.selectedRange([ 3, 5 ]) + }) + + test("selection of the second surrogate pair character", () => { + insertString("foo🙂") + getComposition().setSelectedRange([ 4, 5 ]) + assert.selectedRange([ 5, 5 ]) + }) + + test("collapssed selection in the middle of the surrogate pair", () => { + insertString("foo🙂") + getComposition().setSelectedRange([ 4, 4 ]) + assert.selectedRange([ 5, 5 ]) + }) + + test("collapsed selection after surrogate pair", () => { + insertString("foo🙂") + getComposition().setSelectedRange([ 5, 5 ]) + assert.selectedRange([ 5, 5 ]) + }) + }, +) diff --git a/src/trix/models/composition.js b/src/trix/models/composition.js index 0438d3b12..3f88df67d 100644 --- a/src/trix/models/composition.js +++ b/src/trix/models/composition.js @@ -521,7 +521,18 @@ export default class Composition extends BasicObject { } setSelectedRange(selectedRange) { - const locationRange = this.document.locationRangeFromRange(selectedRange) + const [ start, end ] = normalizeRange(selectedRange) + + // Make sure that surrogate pairs are always selected both or neither. + const correctedSelectedRange = [ + this.translateUTF16PositionFromOffset(start, 0), + this.translateUTF16PositionFromOffset(end, 0), + ] + + const locationRange = this.document.locationRangeFromRange( + correctedSelectedRange, + ) + return this.getSelectionManager().setLocationRange(locationRange) }