servo: Merge #13315 - Implement minlength attribute for text inputs (from Phrohdoh:textinput-minlength-13313); r=ConnorGBrewster
authorTaryn Hill <Phrohdoh@gmail.com>
Wed, 21 Sep 2016 08:49:14 -0500
changeset 339719 f117f24b42e149a6b778cd3f068db5496896b692
parent 339718 a33c53c26dc7700f9be66c5c1b6f640e15e3ee43
child 339720 40b26c9d9c89a0d0301cc685ed82e37b8035ecdf
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersConnorGBrewster
servo: Merge #13315 - Implement minlength attribute for text inputs (from Phrohdoh:textinput-minlength-13313); r=ConnorGBrewster <!-- Please describe your changes on the following line: --> **This is not ready to be merged: I need help writing tests as I am not familiar with the methods used in the `maxlength` tests (introduced in tests/unit/script/textinput.rs).** I also need to write the `minlength-manual` test. Closes #13313 This depends on #13314 (and includes the commit from it so will need to be rebased once that patch lands). I am just looking for a quick review to make sure I am on the right path. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #13313 <!-- Either: --> - [X] There ~~are~~ *will be* tests for these changes <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: c0bcd6fa5ed183e9b4a2f6ead4926773dfb637f1
servo/components/script/dom/htmlinputelement.rs
servo/components/script/dom/htmltextareaelement.rs
servo/components/script/dom/webidls/HTMLInputElement.webidl
servo/components/script/textinput.rs
servo/tests/unit/script/textinput.rs
--- a/servo/components/script/dom/htmlinputelement.rs
+++ b/servo/components/script/dom/htmlinputelement.rs
@@ -80,16 +80,17 @@ enum ValueMode {
 pub struct HTMLInputElement {
     htmlelement: HTMLElement,
     input_type: Cell<InputType>,
     checked_changed: Cell<bool>,
     placeholder: DOMRefCell<DOMString>,
     value_changed: Cell<bool>,
     size: Cell<u32>,
     maxlength: Cell<i32>,
+    minlength: Cell<i32>,
     #[ignore_heap_size_of = "#7193"]
     textinput: DOMRefCell<TextInput<IpcSender<ConstellationMsg>>>,
     activation_state: DOMRefCell<InputActivationState>,
     // https://html.spec.whatwg.org/multipage/#concept-input-value-dirty-flag
     value_dirty: Cell<bool>,
 
     filelist: MutNullableHeap<JS<FileList>>,
 }
@@ -118,31 +119,38 @@ impl InputActivationState {
             was_mutable: false,
             old_type: InputType::InputText
         }
     }
 }
 
 static DEFAULT_INPUT_SIZE: u32 = 20;
 static DEFAULT_MAX_LENGTH: i32 = -1;
+static DEFAULT_MIN_LENGTH: i32 = -1;
 
 impl HTMLInputElement {
     fn new_inherited(local_name: Atom, prefix: Option<DOMString>, document: &Document) -> HTMLInputElement {
         let chan = document.window().constellation_chan().clone();
         HTMLInputElement {
             htmlelement:
                 HTMLElement::new_inherited_with_state(IN_ENABLED_STATE | IN_READ_WRITE_STATE,
                                                       local_name, prefix, document),
             input_type: Cell::new(InputType::InputText),
             placeholder: DOMRefCell::new(DOMString::new()),
             checked_changed: Cell::new(false),
             value_changed: Cell::new(false),
             maxlength: Cell::new(DEFAULT_MAX_LENGTH),
+            minlength: Cell::new(DEFAULT_MIN_LENGTH),
             size: Cell::new(DEFAULT_INPUT_SIZE),
-            textinput: DOMRefCell::new(TextInput::new(Single, DOMString::new(), chan, None, SelectionDirection::None)),
+            textinput: DOMRefCell::new(TextInput::new(Single,
+                                                      DOMString::new(),
+                                                      chan,
+                                                      None,
+                                                      None,
+                                                      SelectionDirection::None)),
             activation_state: DOMRefCell::new(InputActivationState::new()),
             value_dirty: Cell::new(false),
             filelist: MutNullableHeap::new(None),
         }
     }
 
     #[allow(unrooted_must_root)]
     pub fn new(local_name: Atom,
@@ -474,16 +482,22 @@ impl HTMLInputElementMethods for HTMLInp
     make_setter!(SetMax, "max");
 
     // https://html.spec.whatwg.org/multipage/#dom-input-maxlength
     make_int_getter!(MaxLength, "maxlength", DEFAULT_MAX_LENGTH);
 
     // https://html.spec.whatwg.org/multipage/#dom-input-maxlength
     make_limited_int_setter!(SetMaxLength, "maxlength", DEFAULT_MAX_LENGTH);
 
+    // https://html.spec.whatwg.org/multipage/#dom-input-minlength
+    make_int_getter!(MinLength, "minlength", DEFAULT_MIN_LENGTH);
+
+    // https://html.spec.whatwg.org/multipage/#dom-input-minlength
+    make_limited_int_setter!(SetMinLength, "minlength", DEFAULT_MIN_LENGTH);
+
     // https://html.spec.whatwg.org/multipage/#dom-input-min
     make_getter!(Min, "min");
 
     // https://html.spec.whatwg.org/multipage/#dom-input-min
     make_setter!(SetMin, "min");
 
     // https://html.spec.whatwg.org/multipage/#dom-input-multiple
     make_bool_getter!(Multiple, "multiple");
@@ -988,17 +1002,29 @@ impl VirtualMethods for HTMLInputElement
                         if value < 0 {
                             self.textinput.borrow_mut().max_length = None
                         } else {
                             self.textinput.borrow_mut().max_length = Some(value as usize)
                         }
                     },
                     _ => panic!("Expected an AttrValue::Int"),
                 }
-            }
+            },
+            &atom!("minlength") => {
+                match *attr.value() {
+                    AttrValue::Int(_, value) => {
+                        if value < 0 {
+                            self.textinput.borrow_mut().min_length = None
+                        } else {
+                            self.textinput.borrow_mut().min_length = Some(value as usize)
+                        }
+                    },
+                    _ => panic!("Expected an AttrValue::Int"),
+                }
+            },
             &atom!("placeholder") => {
                 {
                     let mut placeholder = self.placeholder.borrow_mut();
                     placeholder.clear();
                     if let AttributeMutation::Set(_) = mutation {
                         placeholder.extend(
                             attr.value().chars().filter(|&c| c != '\n' && c != '\r'));
                     }
@@ -1022,16 +1048,17 @@ impl VirtualMethods for HTMLInputElement
 
     fn parse_plain_attribute(&self, name: &Atom, value: DOMString) -> AttrValue {
         match name {
             &atom!("accept") => AttrValue::from_comma_separated_tokenlist(value.into()),
             &atom!("name") => AttrValue::from_atomic(value.into()),
             &atom!("size") => AttrValue::from_limited_u32(value.into(), DEFAULT_INPUT_SIZE),
             &atom!("type") => AttrValue::from_atomic(value.into()),
             &atom!("maxlength") => AttrValue::from_limited_i32(value.into(), DEFAULT_MAX_LENGTH),
+            &atom!("minlength") => AttrValue::from_limited_i32(value.into(), DEFAULT_MIN_LENGTH),
             _ => self.super_type().unwrap().parse_plain_attribute(name, value),
         }
     }
 
     fn bind_to_tree(&self, tree_in_doc: bool) {
         if let Some(ref s) = self.super_type() {
             s.bind_to_tree(tree_in_doc);
         }
--- a/servo/components/script/dom/htmltextareaelement.rs
+++ b/servo/components/script/dom/htmltextareaelement.rs
@@ -100,17 +100,17 @@ impl HTMLTextAreaElement {
                      prefix: Option<DOMString>,
                      document: &Document) -> HTMLTextAreaElement {
         let chan = document.window().constellation_chan().clone();
         HTMLTextAreaElement {
             htmlelement:
                 HTMLElement::new_inherited_with_state(IN_ENABLED_STATE | IN_READ_WRITE_STATE,
                                                       local_name, prefix, document),
             textinput: DOMRefCell::new(TextInput::new(
-                    Lines::Multiple, DOMString::new(), chan, None, SelectionDirection::None)),
+                    Lines::Multiple, DOMString::new(), chan, None, None, SelectionDirection::None)),
             value_changed: Cell::new(false),
         }
     }
 
     #[allow(unrooted_must_root)]
     pub fn new(local_name: Atom,
                prefix: Option<DOMString>,
                document: &Document) -> Root<HTMLTextAreaElement> {
--- a/servo/components/script/dom/webidls/HTMLInputElement.webidl
+++ b/servo/components/script/dom/webidls/HTMLInputElement.webidl
@@ -22,17 +22,18 @@ interface HTMLInputElement : HTMLElement
   //         attribute unsigned long height;
              attribute boolean indeterminate;
   //         attribute DOMString inputMode;
   //readonly attribute HTMLElement? list;
            attribute DOMString max;
           [SetterThrows]
           attribute long maxLength;
            attribute DOMString min;
-  //         attribute long minLength;
+          [SetterThrows]
+          attribute long minLength;
            attribute boolean multiple;
            attribute DOMString name;
            attribute DOMString pattern;
            attribute DOMString placeholder;
            attribute boolean readOnly;
            attribute boolean required;
              [SetterThrows]
              attribute unsigned long size;
--- a/servo/components/script/textinput.rs
+++ b/servo/components/script/textinput.rs
@@ -68,16 +68,17 @@ pub struct TextInput<T: ClipboardProvide
     /// Is this a multiline input?
     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>,
+    pub min_length: Option<usize>,
     pub selection_direction: SelectionDirection,
 }
 
 /// Resulting action to be taken by the owner of a text input that is handling an event.
 pub enum KeyReaction {
     TriggerDefaultAction,
     DispatchInput,
     RedrawSelection,
@@ -145,24 +146,26 @@ fn len_of_first_n_code_units(text: &str,
     }
     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>,
+               min_length: Option<usize>,
                selection_direction: SelectionDirection) -> TextInput<T> {
         let mut i = TextInput {
             lines: vec!(),
             edit_point: Default::default(),
             selection_begin: None,
             multiline: lines == Lines::Multiple,
             clipboard_provider: clipboard_provider,
             max_length: max_length,
+            min_length: min_length,
             selection_direction: selection_direction,
         };
         i.set_content(initial);
         i
     }
 
     /// Remove a character at the current editing point
     pub fn delete_char(&mut self, dir: Direction) {
--- a/servo/tests/unit/script/textinput.rs
+++ b/servo/tests/unit/script/textinput.rs
@@ -12,36 +12,42 @@ use msg::constellation_msg::{Key, KeyMod
 use msg::constellation_msg::CONTROL;
 #[cfg(target_os = "macos")]
 use msg::constellation_msg::SUPER;
 use script::clipboard_provider::DummyClipboardContext;
 use script::dom::bindings::str::DOMString;
 use script::textinput::{TextInput, TextPoint, Selection, Lines, Direction, SelectionDirection};
 
 fn text_input(lines: Lines, s: &str) -> TextInput<DummyClipboardContext> {
-    TextInput::new(lines, DOMString::from(s), DummyClipboardContext::new(""), None, SelectionDirection::None)
+    TextInput::new(lines,
+                   DOMString::from(s),
+                   DummyClipboardContext::new(""),
+                   None,
+                   None,
+                   SelectionDirection::None)
 }
 
 #[test]
 fn test_set_content_ignores_max_length() {
     let mut textinput = TextInput::new(
-        Lines::Single, DOMString::from(""), DummyClipboardContext::new(""), Some(1), SelectionDirection::None
+        Lines::Single, DOMString::from(""), DummyClipboardContext::new(""), Some(1), None, SelectionDirection::None
     );
 
     textinput.set_content(DOMString::from("mozilla rocks"));
     assert_eq!(textinput.get_content(), DOMString::from("mozilla rocks"));
 }
 
 #[test]
 fn test_textinput_when_inserting_multiple_lines_over_a_selection_respects_max_length() {
     let mut textinput = TextInput::new(
         Lines::Multiple,
         DOMString::from("hello\nworld"),
         DummyClipboardContext::new(""),
         Some(17),
+        None,
         SelectionDirection::None,
     );
 
     textinput.edit_point = TextPoint { line: 0, index: 1 };
     textinput.adjust_horizontal(3, Selection::Selected);
     textinput.adjust_vertical(1, Selection::Selected);
 
     // Selection is now "hello\n
@@ -56,16 +62,17 @@ fn test_textinput_when_inserting_multipl
 
 #[test]
 fn test_textinput_when_inserting_multiple_lines_still_respects_max_length() {
     let mut textinput = TextInput::new(
         Lines::Multiple,
         DOMString::from("hello\nworld"),
         DummyClipboardContext::new(""),
         Some(17),
+        None,
         SelectionDirection::None
     );
 
     textinput.edit_point = TextPoint { line: 1, index: 0 };
 
     textinput.insert_string("cruel\nterrible".to_string());
 
     assert_eq!(textinput.get_content(), "hello\ncruel\nworld");
@@ -73,46 +80,49 @@ fn test_textinput_when_inserting_multipl
 
 #[test]
 fn test_textinput_when_content_is_already_longer_than_max_length_and_theres_no_selection_dont_insert_anything() {
     let mut textinput = TextInput::new(
         Lines::Single,
         DOMString::from("abc"),
         DummyClipboardContext::new(""),
         Some(1),
+        None,
         SelectionDirection::None,
     );
 
     textinput.insert_char('a');
 
     assert_eq!(textinput.get_content(), "abc");
 }
 
 #[test]
 fn test_multi_line_textinput_with_maxlength_doesnt_allow_appending_characters_when_input_spans_lines() {
     let mut textinput = TextInput::new(
         Lines::Multiple,
         DOMString::from("abc\nd"),
         DummyClipboardContext::new(""),
         Some(5),
+        None,
         SelectionDirection::None,
     );
 
     textinput.insert_char('a');
 
     assert_eq!(textinput.get_content(), "abc\nd");
 }
 
 #[test]
 fn test_single_line_textinput_with_max_length_doesnt_allow_appending_characters_when_replacing_a_selection() {
     let mut textinput = TextInput::new(
         Lines::Single,
         DOMString::from("abcde"),
         DummyClipboardContext::new(""),
         Some(5),
+        None,
         SelectionDirection::None,
     );
 
     textinput.edit_point = TextPoint { line: 0, index: 1 };
     textinput.adjust_horizontal(3, Selection::Selected);
 
     // Selection is now "abcde"
     //                    ---
@@ -124,16 +134,17 @@ fn test_single_line_textinput_with_max_l
 
 #[test]
 fn test_single_line_textinput_with_max_length_multibyte() {
     let mut textinput = TextInput::new(
         Lines::Single,
         DOMString::from(""),
         DummyClipboardContext::new(""),
         Some(2),
+        None,
         SelectionDirection::None,
     );
 
     textinput.insert_char('á');
     assert_eq!(textinput.get_content(), "á");
     textinput.insert_char('é');
     assert_eq!(textinput.get_content(), "áé");
     textinput.insert_char('i');
@@ -142,16 +153,17 @@ fn test_single_line_textinput_with_max_l
 
 #[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),
+        None,
         SelectionDirection::None,
     );
 
     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');
@@ -162,30 +174,32 @@ fn test_single_line_textinput_with_max_l
 
 #[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),
+        None,
         SelectionDirection::None,
     );
 
     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(
         Lines::Single,
         DOMString::from("a"),
         DummyClipboardContext::new(""),
         Some(1),
+        None,
         SelectionDirection::None,
     );
 
     textinput.insert_char('b');
     assert_eq!(textinput.get_content(), "a");
 }
 
 #[test]
@@ -393,16 +407,17 @@ fn test_clipboard_paste() {
     const MODIFIERS: KeyModifiers = SUPER;
     #[cfg(not(target_os = "macos"))]
     const MODIFIERS: KeyModifiers = CONTROL;
 
     let mut textinput = TextInput::new(Lines::Single,
                                        DOMString::from("defg"),
                                        DummyClipboardContext::new("abc"),
                                        None,
+                                       None,
                                        SelectionDirection::None);
     assert_eq!(textinput.get_content(), "defg");
     assert_eq!(textinput.edit_point.index, 0);
     textinput.handle_keydown_aux(Some('v'), Key::V, MODIFIERS);
     assert_eq!(textinput.get_content(), "abcdefg");
 }
 
 #[test]