TextInput::max_length should be in code units, not bytes

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>
 }
 
@@ -117,6 +120,22 @@ fn len_of_first_n_chars(text: &str, n: usize) -> usize {
     }
 }
 
+/// 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> {
@@ -178,44 +197,51 @@ impl<T: ClipboardProvider> TextInput<T> {
     }
 
     pub fn get_selection_text(&self) -> Option<String> {
-        self.get_sorted_selection().map(|(begin, end)| {
-            if begin.line != end.line {
-                let mut s = String::new();
-                s.push_str(&self.lines[begin.line][begin.index..]);
-                for (_, line) in self.lines.iter().enumerate().filter(|&(i, _)| begin.line < i && i < end.line) {
-                    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()
-            }
-        })
+        let text = self.fold_selection_slices(String::new(), |s, slice| s.push_str(slice));
+        if text.is_empty() {
+            return None
+        }
+        Some(text)
     }
 
+    /// The length of the selected text in UTF-8 bytes.
     fn selection_len(&self) -> usize {
-        if let Some((begin, end)) = self.get_sorted_selection() {
-            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.fold_selection_slices(0, |len, slice| *len += slice.len())
+    }
 
-            self.len() - (prefix.chars().count() +
-                          suffix.chars().count() +
-                          lines_prefix.iter().fold(0, |m, i| m + i.chars().count() + 1) +
-                          lines_suffix.iter().fold(0, |m, i| m + i.chars().count() + 1))
-        } else {
-            0
+    /// The length of the selected text in UTF-16 code units.
+    fn selection_utf16_len(&self) -> usize {
+        self.fold_selection_slices(0usize,
+            |len, slice| *len += slice.chars().map(char::len_utf16).sum())
+    }
+
+    /// 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();
-                if len_after_selection_replaced > max_length {
+                let len_after_selection_replaced = self.utf16_len() - self.selection_utf16_len();
+                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
@@ -226,7 +252,7 @@ impl<T: ClipboardProvider> TextInput<T> {
                 usize::MAX
             };
 
-            let last_char_index = len_of_first_n_chars(&*insert, allowed_to_insert_count);
+            let last_char_index = len_of_first_n_code_units(&*insert, allowed_to_insert_count);
             let chars_to_insert = &insert[..last_char_index];
 
             self.clear_selection();
@@ -498,7 +524,21 @@ 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
     }
 

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(