From b2c1f89b932a72f9e0110c17adde33647e84c902 Mon Sep 17 00:00:00 2001 From: paavininanda Date: Tue, 6 Feb 2018 22:33:12 +0530 Subject: [PATCH] Correct default Selectionstart and SelectionEnd --- components/script/dom/htmlformelement.rs | 2 +- components/script/dom/htmlinputelement.rs | 75 ++++++++++--------- components/script/dom/htmltextareaelement.rs | 55 +++++++------- components/script/textinput.rs | 40 +++++----- tests/unit/script/textinput.rs | 10 ++- tests/wpt/metadata/MANIFEST.json | 10 +++ .../setSelectionRange.html.ini | 4 - .../textfieldselection/defaultSelection.html | 35 +++++++++ 8 files changed, 144 insertions(+), 87 deletions(-) delete mode 100644 tests/wpt/metadata/html/semantics/forms/textfieldselection/setSelectionRange.html.ini create mode 100644 tests/wpt/web-platform-tests/html/semantics/forms/textfieldselection/defaultSelection.html diff --git a/components/script/dom/htmlformelement.rs b/components/script/dom/htmlformelement.rs index 27a64d00cda..36ebb18db87 100755 --- a/components/script/dom/htmlformelement.rs +++ b/components/script/dom/htmlformelement.rs @@ -651,7 +651,7 @@ impl HTMLFormElement { child.downcast::().unwrap().reset(); } NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLTextAreaElement)) => { - child.downcast::().unwrap().reset(); + child.downcast::().unwrap().reset(true); } NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLOutputElement)) => { // Unimplemented diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 09309da7448..b0fbfb36f36 100755 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -552,36 +552,7 @@ impl HTMLInputElementMethods for HTMLInputElement { // https://html.spec.whatwg.org/multipage/#dom-input-value fn SetValue(&self, value: DOMString) -> ErrorResult { - match self.value_mode() { - ValueMode::Value => { - // Steps 1-2. - let old_value = mem::replace(self.textinput.borrow_mut().single_line_content_mut(), value); - // Step 3. - self.value_dirty.set(true); - // Step 4. - self.sanitize_value(); - // Step 5. - if *self.textinput.borrow().single_line_content() != old_value { - self.textinput.borrow_mut().clear_selection_to_limit(Direction::Forward); - } - } - ValueMode::Default | - ValueMode::DefaultOn => { - self.upcast::().set_string_attribute(&local_name!("value"), value); - } - ValueMode::Filename => { - if value.is_empty() { - let window = window_from_node(self); - let fl = FileList::new(&window, vec![]); - self.filelist.set(Some(&fl)); - } else { - return Err(Error::InvalidState); - } - } - } - - self.upcast::().dirty(NodeDamage::OtherNodeDamage); - Ok(()) + self.update_text_contents(value, true) } // https://html.spec.whatwg.org/multipage/#dom-input-defaultvalue @@ -937,7 +908,7 @@ impl HTMLInputElement { _ => () } - self.SetValue(self.DefaultValue()) + self.update_text_contents(self.DefaultValue(), true) .expect("Failed to reset input value to default."); self.value_dirty.set(false); self.upcast::().dirty(NodeDamage::OtherNodeDamage); @@ -1065,7 +1036,7 @@ impl HTMLInputElement { let content = textinput.single_line_content_mut(); content.make_ascii_lowercase(); } else { - textinput.set_content("#000000".into()); + textinput.set_content("#000000".into(), true); } } InputType::Time => { @@ -1103,6 +1074,41 @@ impl HTMLInputElement { fn selection(&self) -> TextControlSelection { TextControlSelection::new(&self, &self.textinput) } + + fn update_text_contents(&self, value: DOMString, update_text_cursor: bool) -> ErrorResult { + match self.value_mode() { + ValueMode::Value => { + // Steps 1-2. + let old_value = mem::replace(self.textinput.borrow_mut().single_line_content_mut(), value); + // Step 3. + self.value_dirty.set(true); + // Step 4. + if update_text_cursor { + self.sanitize_value(); + } + // Step 5. + if *self.textinput.borrow().single_line_content() != old_value { + self.textinput.borrow_mut().clear_selection_to_limit(Direction::Forward, update_text_cursor); + } + } + ValueMode::Default | + ValueMode::DefaultOn => { + self.upcast::().set_string_attribute(&local_name!("value"), value); + } + ValueMode::Filename => { + if value.is_empty() { + let window = window_from_node(self); + let fl = FileList::new(&window, vec![]); + self.filelist.set(Some(&fl)); + } else { + return Err(Error::InvalidState); + } + } + } + + self.upcast::().dirty(NodeDamage::OtherNodeDamage); + Ok(()) + } } impl VirtualMethods for HTMLInputElement { @@ -1112,7 +1118,6 @@ impl VirtualMethods for HTMLInputElement { fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) { self.super_type().unwrap().attribute_mutated(attr, mutation); - match attr.local_name() { &local_name!("disabled") => { let disabled_state = match mutation { @@ -1214,7 +1219,7 @@ impl VirtualMethods for HTMLInputElement { // Steps 7-9 if !previously_selectable && self.selection_api_applies() { - self.textinput.borrow_mut().clear_selection_to_limit(Direction::Backward); + self.textinput.borrow_mut().clear_selection_to_limit(Direction::Backward, true); } }, AttributeMutation::Removed => { @@ -1236,7 +1241,7 @@ impl VirtualMethods for HTMLInputElement { &local_name!("value") if !self.value_dirty.get() => { let value = mutation.new_value(attr).map(|value| (**value).to_owned()); self.textinput.borrow_mut().set_content( - value.map_or(DOMString::new(), DOMString::from)); + value.map_or(DOMString::new(), DOMString::from), true); self.sanitize_value(); self.update_placeholder_shown_state(); }, diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index 58e017de93a..a5038918517 100755 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -233,7 +233,7 @@ impl HTMLTextAreaElementMethods for HTMLTextAreaElement { // if the element's dirty value flag is false, then the element's // raw value must be set to the value of the element's textContent IDL attribute if !self.value_dirty.get() { - self.reset(); + self.reset(false); } } @@ -244,26 +244,7 @@ impl HTMLTextAreaElementMethods for HTMLTextAreaElement { // https://html.spec.whatwg.org/multipage/#dom-textarea-value fn SetValue(&self, value: DOMString) { - let mut textinput = self.textinput.borrow_mut(); - - // Step 1 - let old_value = textinput.get_content(); - let old_selection = textinput.selection_origin; - - // Step 2 - textinput.set_content(value); - - // Step 3 - self.value_dirty.set(true); - - if old_value != textinput.get_content() { - // Step 4 - textinput.clear_selection_to_limit(Direction::Forward); - } else { - textinput.selection_origin = old_selection; - } - - self.upcast::().dirty(NodeDamage::OtherNodeDamage); + self.update_text_contents(value, true); } // https://html.spec.whatwg.org/multipage/#dom-lfe-labels @@ -325,9 +306,9 @@ impl HTMLTextAreaElementMethods for HTMLTextAreaElement { impl HTMLTextAreaElement { - pub fn reset(&self) { + pub fn reset(&self, update_text_cursor: bool) { // https://html.spec.whatwg.org/multipage/#the-textarea-element:concept-form-reset-control - self.SetValue(self.DefaultValue()); + self.update_text_contents(self.DefaultValue(), update_text_cursor); self.value_dirty.set(false); } @@ -335,6 +316,30 @@ impl HTMLTextAreaElement { fn selection(&self) -> TextControlSelection { TextControlSelection::new(&self, &self.textinput) } + + // Helper function to check if text_cursor is to be updated or not + fn update_text_contents(&self, value: DOMString, update_text_cursor: bool) { + let mut textinput = self.textinput.borrow_mut(); + + // Step 1 + let old_value = textinput.get_content(); + let old_selection = textinput.selection_origin; + + // Step 2 + textinput.set_content(value, update_text_cursor); + + // Step 3 + self.value_dirty.set(true); + + if old_value != textinput.get_content() { + // Step 4 + textinput.clear_selection_to_limit(Direction::Forward, update_text_cursor); + } else { + textinput.selection_origin = old_selection; + } + + self.upcast::().dirty(NodeDamage::OtherNodeDamage); + } } @@ -427,7 +432,7 @@ impl VirtualMethods for HTMLTextAreaElement { s.children_changed(mutation); } if !self.value_dirty.get() { - self.reset(); + self.reset(false); } } @@ -478,7 +483,7 @@ impl VirtualMethods for HTMLTextAreaElement { self.super_type().unwrap().pop(); // https://html.spec.whatwg.org/multipage/#the-textarea-element:stack-of-open-elements - self.reset(); + self.reset(false); } } diff --git a/components/script/textinput.rs b/components/script/textinput.rs index 4d8479346f3..a4a5d5e8965 100644 --- a/components/script/textinput.rs +++ b/components/script/textinput.rs @@ -171,7 +171,7 @@ impl TextInput { min_length: min_length, selection_direction: selection_direction, }; - i.set_content(initial); + i.set_content(initial, false); i } @@ -411,9 +411,7 @@ impl TextInput { 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 = len_of_first_n_chars(&self.lines[self.edit_point.line], col); self.assert_ok_selection(); @@ -533,9 +531,9 @@ impl TextInput { } /// Remove the current selection and set the edit point to the end of the content. - pub fn clear_selection_to_limit(&mut self, direction: Direction) { + pub fn clear_selection_to_limit(&mut self, direction: Direction, update_text_cursor: bool) { self.clear_selection(); - self.adjust_horizontal_to_limit(direction, Selection::NotSelected); + self.adjust_horizontal_to_limit(direction, Selection::NotSelected, update_text_cursor); } pub fn adjust_horizontal_by_word(&mut self, direction: Direction, select: Selection) { @@ -627,18 +625,20 @@ impl TextInput { self.perform_horizontal_adjustment(shift, select); } - pub fn adjust_horizontal_to_limit(&mut self, direction: Direction, select: Selection) { + pub fn adjust_horizontal_to_limit(&mut self, direction: Direction, select: Selection, update_text_cursor: bool) { if self.adjust_selection_for_horizontal_change(direction, select) { return } - match direction { - Direction::Backward => { - self.edit_point.line = 0; - self.edit_point.index = 0; - }, - Direction::Forward => { - self.edit_point.line = &self.lines.len() - 1; - self.edit_point.index = (&self.lines[&self.lines.len() - 1]).len(); + if update_text_cursor { + match direction { + Direction::Backward => { + self.edit_point.line = 0; + self.edit_point.index = 0; + }, + Direction::Forward => { + self.edit_point.line = &self.lines.len() - 1; + self.edit_point.index = (&self.lines[&self.lines.len() - 1]).len(); + } } } } @@ -728,12 +728,12 @@ impl TextInput { }, #[cfg(target_os = "macos")] (None, Key::Up) if mods.contains(KeyModifiers::SUPER) => { - self.adjust_horizontal_to_limit(Direction::Backward, maybe_select); + self.adjust_horizontal_to_limit(Direction::Backward, maybe_select, true); KeyReaction::RedrawSelection }, #[cfg(target_os = "macos")] (None, Key::Down) if mods.contains(KeyModifiers::SUPER) => { - self.adjust_horizontal_to_limit(Direction::Forward, maybe_select); + self.adjust_horizontal_to_limit(Direction::Forward, maybe_select, true); KeyReaction::RedrawSelection }, (None, Key::Left) if mods.contains(KeyModifiers::ALT) => { @@ -840,7 +840,7 @@ impl TextInput { /// Set the current contents of the text input. If this is control supports multiple lines, /// any \n encountered will be stripped and force a new logical line. - pub fn set_content(&mut self, content: DOMString) { + pub fn set_content(&mut self, content: DOMString, update_text_cursor: bool) { self.lines = if self.multiline { // https://html.spec.whatwg.org/multipage/#textarea-line-break-normalisation-transformation content.replace("\r\n", "\n") @@ -850,8 +850,10 @@ impl TextInput { } else { vec!(content) }; - self.edit_point.line = min(self.edit_point.line, self.lines.len() - 1); - self.edit_point.index = min(self.edit_point.index, self.current_line_length()); + if update_text_cursor { + self.edit_point.line = min(self.edit_point.line, self.lines.len() - 1); + self.edit_point.index = min(self.edit_point.index, self.current_line_length()); + } self.selection_origin = None; self.assert_ok_selection(); } diff --git a/tests/unit/script/textinput.rs b/tests/unit/script/textinput.rs index dfc6970334d..0bdeec59cda 100644 --- a/tests/unit/script/textinput.rs +++ b/tests/unit/script/textinput.rs @@ -27,7 +27,7 @@ fn test_set_content_ignores_max_length() { Lines::Single, DOMString::from(""), DummyClipboardContext::new(""), Some(1), None, SelectionDirection::None ); - textinput.set_content(DOMString::from("mozilla rocks")); + textinput.set_content(DOMString::from("mozilla rocks"), true); assert_eq!(textinput.get_content(), DOMString::from("mozilla rocks")); } @@ -492,7 +492,7 @@ fn test_textinput_set_content() { let mut textinput = text_input(Lines::Multiple, "abc\nde\nf"); assert_eq!(textinput.get_content(), "abc\nde\nf"); - textinput.set_content(DOMString::from("abc\nf")); + textinput.set_content(DOMString::from("abc\nf"), true); assert_eq!(textinput.get_content(), "abc\nf"); assert_eq!(textinput.edit_point.line, 0); @@ -500,7 +500,7 @@ fn test_textinput_set_content() { textinput.adjust_horizontal(3, Selection::Selected); assert_eq!(textinput.edit_point.line, 0); assert_eq!(textinput.edit_point.index, 3); - textinput.set_content(DOMString::from("de")); + textinput.set_content(DOMString::from("de"), true); assert_eq!(textinput.get_content(), "de"); assert_eq!(textinput.edit_point.line, 0); assert_eq!(textinput.edit_point.index, 2); @@ -637,6 +637,10 @@ fn test_textinput_unicode_handling() { fn test_selection_bounds() { let mut textinput = text_input(Lines::Single, "abcdef"); + assert_eq!(TextPoint { line: 0, index: 0 }, textinput.selection_origin_or_edit_point()); + assert_eq!(TextPoint { line: 0, index: 0 }, textinput.selection_start()); + assert_eq!(TextPoint { line: 0, index: 0 }, textinput.selection_end()); + textinput.set_selection_range(2, 5, SelectionDirection::Forward); assert_eq!(TextPoint { line: 0, index: 2 }, textinput.selection_origin_or_edit_point()); assert_eq!(TextPoint { line: 0, index: 2 }, textinput.selection_start()); diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json index d492bd7b84e..15c6719348d 100644 --- a/tests/wpt/metadata/MANIFEST.json +++ b/tests/wpt/metadata/MANIFEST.json @@ -328172,6 +328172,12 @@ {} ] ], + "html/semantics/forms/textfieldselection/defaultSelection.html": [ + [ + "/html/semantics/forms/textfieldselection/defaultSelection.html", + {} + ] + ], "html/semantics/forms/textfieldselection/select-event.html": [ [ "/html/semantics/forms/textfieldselection/select-event.html", @@ -558275,6 +558281,10 @@ "da39a3ee5e6b4b0d3255bfef95601890afd80709", "support" ], + "html/semantics/forms/textfieldselection/defaultSelection.html": [ + "c9568da864127d49974b970809312c953fb347b1", + "testharness" + ], "html/semantics/forms/textfieldselection/original-id.json": [ "14a6f0077d512837c24931a2a9723e11d1d7a1c7", "support" diff --git a/tests/wpt/metadata/html/semantics/forms/textfieldselection/setSelectionRange.html.ini b/tests/wpt/metadata/html/semantics/forms/textfieldselection/setSelectionRange.html.ini deleted file mode 100644 index e5a3a19ddac..00000000000 --- a/tests/wpt/metadata/html/semantics/forms/textfieldselection/setSelectionRange.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[setSelectionRange.html] - [setSelectionRange on line boundaries] - expected: FAIL - diff --git a/tests/wpt/web-platform-tests/html/semantics/forms/textfieldselection/defaultSelection.html b/tests/wpt/web-platform-tests/html/semantics/forms/textfieldselection/defaultSelection.html new file mode 100644 index 00000000000..eb9bb40d767 --- /dev/null +++ b/tests/wpt/web-platform-tests/html/semantics/forms/textfieldselection/defaultSelection.html @@ -0,0 +1,35 @@ + + + + + + + + +