Auto merge of #10350 - mbrubeck:selection-chars, r=SimonSapin

Fix some char/byte bugs in textinput

Fixes #9569. r? @SimonSapin or @Ms2ger

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10350)
<!-- Reviewable:end -->

components/script/lib.rs

@@ -11,6 +11,7 @@
 #![feature(custom_attribute)]
 #![feature(custom_derive)]
 #![feature(fnbox)]
+#![feature(iter_arith)]
 #![feature(mpsc_select)]
 #![feature(nonzero)]
 #![feature(on_unimplemented)]

components/script/textinput.rs

@@ -42,6 +42,9 @@ pub struct TextInput<T: ClipboardProvider> {
     multiline: bool,
     #[ignore_heap_size_of = "Can't easily measure this generic type"]
     clipboard_provider: T,
+    /// The maximum number of UTF-16 code units this text input is allowed to hold.
+    ///
+    /// https://html.spec.whatwg.org/multipage/#attr-fe-maxlength
     pub max_length: Option<usize>
 }
 
@@ -107,6 +110,32 @@ fn is_printable_key(key: Key) -> bool {
     }
 }
 
+/// The length in bytes of the first n characters in a UTF-8 string.
+///
+/// If the string has fewer than n characters, returns the length of the whole string.
+fn len_of_first_n_chars(text: &str, n: usize) -> usize {
+    match text.char_indices().take(n).last() {
+        Some((index, ch)) => index + ch.len_utf8(),
+        None => 0
+    }
+}
+
+/// The length in bytes of the first n code units a string when encoded in UTF-16.
+///
+/// If the string is fewer than n code units, returns the length of the whole string.
+fn len_of_first_n_code_units(text: &str, n: usize) -> usize {
+    let mut utf8_len = 0;
+    let mut utf16_len = 0;
+    for c in text.chars() {
+        utf16_len += c.len_utf16();
+        if utf16_len > n {
+            break;
+        }
+        utf8_len += c.len_utf8();
+    }
+    utf8_len
+}
+
 impl<T: ClipboardProvider> TextInput<T> {
     /// Instantiate a new text input control
     pub fn new(lines: Lines, initial: DOMString, clipboard_provider: T, max_length: Option<usize>) -> TextInput<T> {
@@ -155,6 +184,9 @@ impl<T: ClipboardProvider> TextInput<T> {
         })
     }
 
+    /// Return the selection range as UTF-8 byte offsets from the start of the content.
+    ///
+    /// If there is no selection, returns an empty range at the insertion point.
     pub fn get_absolute_selection_range(&self) -> Range<usize> {
         match self.get_sorted_selection() {
             Some((begin, _end)) =>
@@ -165,44 +197,51 @@ impl<T: ClipboardProvider> TextInput<T> {
     }
 
     pub fn get_selection_text(&self) -> Option<String> {
-        self.get_sorted_selection().map(|(begin, end)| {
+        let text = self.fold_selection_slices(String::new(), |s, slice| s.push_str(slice));
-            if begin.line != end.line {
+        if text.is_empty() {
-                let mut s = String::new();
+            return None
-                s.push_str(&self.lines[begin.line][begin.index..]);
+        }
-                for (_, line) in self.lines.iter().enumerate().filter(|&(i, _)| begin.line < i && i < end.line) {
+        Some(text)
-                    s.push_str("\n");
-                    s.push_str(line);
-                }
-                s.push_str("\n");
-                s.push_str(&self.lines[end.line][..end.index]);
-                s
-            } else {
-                self.lines[begin.line][begin.index..end.index].to_owned()
-            }
-        })
     }
 
+    /// The length of the selected text in UTF-8 bytes.
     fn selection_len(&self) -> usize {
-        if let Some((begin, end)) = self.get_sorted_selection() {
+        self.fold_selection_slices(0, |len, slice| *len += slice.len())
-            let prefix = &self.lines[begin.line][0..begin.index];
+    }
-            let suffix = &self.lines[end.line][end.index..];
-            let lines_prefix = &self.lines[..begin.line];
-            let lines_suffix = &self.lines[end.line + 1..];
 
-            self.len() - (prefix.chars().count() +
+    /// The length of the selected text in UTF-16 code units.
-                          suffix.chars().count() +
+    fn selection_utf16_len(&self) -> usize {
-                          lines_prefix.iter().fold(0, |m, i| m + i.chars().count() + 1) +
+        self.fold_selection_slices(0usize,
-                          lines_suffix.iter().fold(0, |m, i| m + i.chars().count() + 1))
+            |len, slice| *len += slice.chars().map(char::len_utf16).sum())
-        } else {
+    }
-            0
+
+    /// Run the callback on a series of slices that, concatenated, make up the selected text.
+    ///
+    /// The accumulator `acc` can be mutated by the callback, and will be returned at the end.
+    fn fold_selection_slices<B, F: FnMut(&mut B, &str)>(&self, mut acc: B, mut f: F) -> B {
+        match self.get_sorted_selection() {
+            Some((begin, end)) if begin.line == end.line => {
+                f(&mut acc, &self.lines[begin.line][begin.index..end.index])
+            }
+            Some((begin, end)) => {
+                f(&mut acc, &self.lines[begin.line][begin.index..]);
+                for line in &self.lines[begin.line + 1 .. end.line] {
+                    f(&mut acc, "\n");
+                    f(&mut acc, line);
+                }
+                f(&mut acc, "\n");
+                f(&mut acc, &self.lines[end.line][..end.index])
+            }
+            None => {}
         }
+        acc
     }
 
     pub fn replace_selection(&mut self, insert: DOMString) {
         if let Some((begin, end)) = self.get_sorted_selection() {
             let allowed_to_insert_count = if let Some(max_length) = self.max_length {
-                let len_after_selection_replaced = self.len() - self.selection_len();
+                let len_after_selection_replaced = self.utf16_len() - self.selection_utf16_len();
-                if len_after_selection_replaced > max_length {
+                if len_after_selection_replaced >= max_length {
                     // If, after deleting the selection, the len is still greater than the max
                     // length, then don't delete/insert anything
                     return
@@ -213,8 +252,8 @@ impl<T: ClipboardProvider> TextInput<T> {
                 usize::MAX
             };
 
-            let last_char_to_insert = min(allowed_to_insert_count, insert.chars().count());
+            let last_char_index = len_of_first_n_code_units(&*insert, allowed_to_insert_count);
-            let chars_to_insert = (&insert[0 .. last_char_to_insert]).to_owned();
+            let chars_to_insert = &insert[..last_char_index];
 
             self.clear_selection();
 
@@ -225,7 +264,7 @@ impl<T: ClipboardProvider> TextInput<T> {
                 let lines_suffix = &self.lines[end.line + 1..];
 
                 let mut insert_lines = if self.multiline {
-                    chars_to_insert.split('\n').map(|s| DOMString::from(s.to_owned())).collect()
+                    chars_to_insert.split('\n').map(|s| DOMString::from(s)).collect()
                 } else {
                     vec!(DOMString::from(chars_to_insert))
                 };
@@ -288,11 +327,14 @@ impl<T: ClipboardProvider> TextInput<T> {
             return;
         }
 
+
+        let col = self.lines[self.edit_point.line][..self.edit_point.index].chars().count();
+
         self.edit_point.line = target_line as usize;
-        self.edit_point.index = min(self.current_line_length(), self.edit_point.index);
+        self.edit_point.index = len_of_first_n_chars(&self.lines[self.edit_point.line], col);
     }
 
-    /// Adjust the editing point position by a given number of columns. If the adjustment
+    /// Adjust the editing point position by a given number of bytes. If the adjustment
     /// requested is larger than is available in the current line, the editing point is
     /// adjusted vertically and the process repeats with the remaining adjustment requested.
     pub fn adjust_horizontal(&mut self, adjust: isize, select: Selection) {
@@ -329,7 +371,7 @@ impl<T: ClipboardProvider> TextInput<T> {
         self.perform_horizontal_adjustment(adjust, select);
     }
 
-    // Return whether to cancel the caret move
+    /// Return whether to cancel the caret move
     fn adjust_selection_for_horizontal_change(&mut self, adjust: Direction, select: Selection)
                                               -> bool {
         if select == Selection::Selected {
@@ -479,9 +521,24 @@ impl<T: ClipboardProvider> TextInput<T> {
         }
     }
 
+    /// The length of the content in bytes.
     pub fn len(&self) -> usize {
         self.lines.iter().fold(0, |m, l| {
-            m + l.len() + 1
+            m + l.len() + 1 // + 1 for the '\n'
+        }) - 1
+    }
+
+    /// The length of the content in bytes.
+    pub fn utf16_len(&self) -> usize {
+        self.lines.iter().fold(0, |m, l| {
+            m + l.chars().map(char::len_utf16).sum::<usize>() + 1 // + 1 for the '\n'
+        }) - 1
+    }
+
+    /// The length of the content in chars.
+    pub fn char_count(&self) -> usize {
+        self.lines.iter().fold(0, |m, l| {
+            m + l.chars().count() + 1 // + 1 for the '\n'
         }) - 1
     }
 
@@ -510,10 +567,12 @@ impl<T: ClipboardProvider> TextInput<T> {
         self.selection_begin = None;
     }
 
+    /// Get the insertion point as a byte offset from the start of the content.
     pub fn get_absolute_insertion_point(&self) -> usize {
         self.get_absolute_point_for_text_point(&self.edit_point)
     }
 
+    /// Convert a TextPoint into a byte offset from the start of the content.
     pub fn get_absolute_point_for_text_point(&self, text_point: &TextPoint) -> usize {
         self.lines.iter().enumerate().fold(0, |acc, (i, val)| {
             if i < text_point.line {
@@ -524,6 +583,7 @@ impl<T: ClipboardProvider> TextInput<T> {
         }) + text_point.index
     }
 
+    /// Convert a byte offset from the start of the content into a TextPoint.
     pub fn get_text_point_for_absolute_point(&self, abs_point: usize) -> TextPoint {
         let mut index = abs_point;
         let mut line = 0;

tests/unit/script/textinput.rs

@@ -117,6 +117,55 @@ fn test_single_line_textinput_with_max_length_doesnt_allow_appending_characters_
     assert_eq!(textinput.get_content(), "atooe");
 }
 
+#[test]
+fn test_single_line_textinput_with_max_length_multibyte() {
+    let mut textinput = TextInput::new(
+        Lines::Single,
+        DOMString::from(""),
+        DummyClipboardContext::new(""),
+        Some(2)
+    );
+
+    textinput.insert_char('á');
+    assert_eq!(textinput.get_content(), "á");
+    textinput.insert_char('é');
+    assert_eq!(textinput.get_content(), "áé");
+    textinput.insert_char('i');
+    assert_eq!(textinput.get_content(), "áé");
+}
+
+#[test]
+fn test_single_line_textinput_with_max_length_multi_code_unit() {
+    let mut textinput = TextInput::new(
+        Lines::Single,
+        DOMString::from(""),
+        DummyClipboardContext::new(""),
+        Some(3)
+    );
+
+    textinput.insert_char('\u{10437}');
+    assert_eq!(textinput.get_content(), "\u{10437}");
+    textinput.insert_char('\u{10437}');
+    assert_eq!(textinput.get_content(), "\u{10437}");
+    textinput.insert_char('x');
+    assert_eq!(textinput.get_content(), "\u{10437}x");
+    textinput.insert_char('x');
+    assert_eq!(textinput.get_content(), "\u{10437}x");
+}
+
+#[test]
+fn test_single_line_textinput_with_max_length_inside_char() {
+    let mut textinput = TextInput::new(
+        Lines::Single,
+        DOMString::from("\u{10437}"),
+        DummyClipboardContext::new(""),
+        Some(1)
+    );
+
+    textinput.insert_char('x');
+    assert_eq!(textinput.get_content(), "\u{10437}");
+}
+
 #[test]
 fn test_single_line_textinput_with_max_length_doesnt_allow_appending_characters_after_max_length_is_reached() {
     let mut textinput = TextInput::new(
@@ -206,6 +255,15 @@ fn test_textinput_replace_selection() {
     assert_eq!(textinput.get_content(), "abxyzefg");
 }
 
+#[test]
+fn test_textinput_replace_selection_multibyte_char() {
+    let mut textinput = text_input(Lines::Single, "é");
+    textinput.adjust_horizontal_by_one(Direction::Forward, Selection::Selected);
+
+    textinput.replace_selection(DOMString::from("e"));
+    assert_eq!(textinput.get_content(), "e");
+}
+
 #[test]
 fn test_textinput_current_line_length() {
     let mut textinput = text_input(Lines::Multiple, "abc\nde\nf");
@@ -235,6 +293,19 @@ fn test_textinput_adjust_vertical() {
     assert_eq!(textinput.edit_point.index, 1);
 }
 
+#[test]
+fn test_textinput_adjust_vertical_multibyte() {
+    let mut textinput = text_input(Lines::Multiple, "áé\nae");
+
+    textinput.adjust_horizontal_by_one(Direction::Forward, Selection::NotSelected);
+    assert_eq!(textinput.edit_point.line, 0);
+    assert_eq!(textinput.edit_point.index, 2);
+
+    textinput.adjust_vertical(1, Selection::NotSelected);
+    assert_eq!(textinput.edit_point.line, 1);
+    assert_eq!(textinput.edit_point.index, 1);
+}
+
 #[test]
 fn test_textinput_adjust_horizontal() {
     let mut textinput = text_input(Lines::Multiple, "abc\nde\nf");