From 648bfbeb022d02fca3fb6da14a6247390d0f070b Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Sat, 9 Dec 2017 21:55:01 +0100 Subject: [PATCH] Fix clearing the selection when value is changed The implementation of adjust_horizontal_to_limit() is written with UI in mind. As such, when there's a selection and we "adjust horizontal", the selection will be cleared and the cursor will and up at the start/end of the previous selection. This is what happens when you have a selection and you press an arrow key on your keyboard, but it isn't the behaviour we want when programmatically changing the value. Instead, we need to first clear the selection, and then move the cursor to the end. (We also need to reset the selection direction when clearing the selection.) --- components/script/dom/htmlinputelement.rs | 5 ++--- components/script/dom/htmltextareaelement.rs | 4 ++-- components/script/textinput.rs | 7 +++++++ .../selection-after-content-change.html.ini | 20 ------------------- 4 files changed, 11 insertions(+), 25 deletions(-) delete mode 100644 tests/wpt/metadata/html/semantics/forms/textfieldselection/selection-after-content-change.html.ini diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 2e26d7fe53f0..cd476a47f2b5 100755 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -52,7 +52,7 @@ use std::ops::Range; use style::attr::AttrValue; use style::element_state::ElementState; use style::str::split_commas; -use textinput::{Direction, Selection, SelectionDirection, TextInput}; +use textinput::{SelectionDirection, TextInput}; use textinput::KeyReaction::{DispatchInput, Nothing, RedrawSelection, TriggerDefaultAction}; use textinput::Lines::Single; @@ -563,8 +563,7 @@ impl HTMLInputElementMethods for HTMLInputElement { self.sanitize_value(); // Step 5. if *self.textinput.borrow().single_line_content() != old_value { - self.textinput.borrow_mut() - .adjust_horizontal_to_limit(Direction::Forward, Selection::NotSelected); + self.textinput.borrow_mut().clear_selection_to_limit(); } } ValueMode::Default | diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index a2b52ce3087e..b2e78e083c0c 100755 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -35,7 +35,7 @@ use std::default::Default; use std::ops::Range; use style::attr::AttrValue; use style::element_state::ElementState; -use textinput::{Direction, KeyReaction, Lines, Selection, SelectionDirection, TextInput}; +use textinput::{KeyReaction, Lines, SelectionDirection, TextInput}; #[dom_struct] pub struct HTMLTextAreaElement { @@ -257,7 +257,7 @@ impl HTMLTextAreaElementMethods for HTMLTextAreaElement { if old_value != textinput.get_content() { // Step 4 - textinput.adjust_horizontal_to_limit(Direction::Forward, Selection::NotSelected); + textinput.clear_selection_to_limit(); } else { textinput.selection_origin = old_selection; } diff --git a/components/script/textinput.rs b/components/script/textinput.rs index b4e39eab3f8e..d703ccee9879 100644 --- a/components/script/textinput.rs +++ b/components/script/textinput.rs @@ -513,6 +513,13 @@ impl TextInput { /// Remove the current selection. pub fn clear_selection(&mut self) { self.selection_origin = None; + self.selection_direction = SelectionDirection::None; + } + + /// Remove the current selection and set the edit point to the end of the content. + pub fn clear_selection_to_limit(&mut self) { + self.clear_selection(); + self.adjust_horizontal_to_limit(Direction::Forward, Selection::NotSelected); } pub fn adjust_horizontal_by_word(&mut self, direction: Direction, select: Selection) { diff --git a/tests/wpt/metadata/html/semantics/forms/textfieldselection/selection-after-content-change.html.ini b/tests/wpt/metadata/html/semantics/forms/textfieldselection/selection-after-content-change.html.ini deleted file mode 100644 index 9eaf5d33c8b5..000000000000 --- a/tests/wpt/metadata/html/semantics/forms/textfieldselection/selection-after-content-change.html.ini +++ /dev/null @@ -1,20 +0,0 @@ -[selection-after-content-change.html] - type: testharness - [input out of document: selection must change when setting a different value] - expected: FAIL - - [input in document: selection must change when setting a different value] - expected: FAIL - - [input in document, with focus: selection must change when setting a different value] - expected: FAIL - - [textarea out of document: selection must change when setting a different value] - expected: FAIL - - [textarea in document: selection must change when setting a different value] - expected: FAIL - - [textarea in document, with focus: selection must change when setting a different value] - expected: FAIL -