Skip to content

Commit

Permalink
Implement setRangeText API
Browse files Browse the repository at this point in the history
Spec: https://html.spec.whatwg.org/multipage/#dom-textarea/input-setrangetext

In order to do this, we need to define the SelectionMode enum in WebIDL:
https://html.spec.whatwg.org/multipage/#selectionmode

Since the enum is used by HTMLTextAreaElement and HTMLInputElement, it
doesn't seem to make sense to define it in the WebIDL file for one or
other of those.

However, we also can't create a stand-alone SelectionMode.webidl file,
because the current binding-generation code won't generate a "pub mod
SelectionMode;" line in mod.rs unless SelectionMode.webidl contains
either an interface or a namespace. (This logic happens in
components/script/dom/bindings/codegen/Configuration.py:35, in the
Configuration.__init__ method.)

I thought about changing the binding-generation code, but that seems
difficult. So I settled for placing the enum inside
HTMLFormElement.webidl, as that seems like a "neutral" location. We
could equally settle for putting it under HTMLTextAreaElement or
HTMLInputElement, it probably doesn't really matter.

The setRangeText algorithm set the "dirty value flag" on the
input/textarea. I made some clean-ups related to this:

1. HTMLTextAreaElement called its dirty value flag "value_changed"; I
   changed this to "value_dirty" to be consistent with the spec.

2. HTMLInputElement had a "value_changed" field and also a "value_dirty"
   field, which were each used in slightly different places (and
   sometimes in both places). I consolidated these into a single
   "value_dirty" field, which was necessary in order to make some of the
   tests pass.

TextControl::set_dom_range_text replaces part of the existing textinput
content with the replacement string (steps 9-10 of the algorithm). My
implementation changes the textinput's selection and then replaces the
selection. A downside of this approach is that we lose the original
selection state from before the call to setRangeText. Therefore, we have
to save the state into the original_selection_state variable so that we
can later pass it into TextControl::set_selection_range. This allows
TextControl::set_selection_range to correctly decide whether or not to
fire the select event.

An alternative approach would be to implement a method on TextInput
which allows a subtring of the content to be mutated, without touching
the current selection state. However, any such method would potentially
put the TextInput into an inconsistent state where the edit_point and/or
selection_origin is a TextPoint which doesn't exist in the content. It
would be up to the caller to subsequently make sure that the TextInput
gets put back into a valid state (which would actually happen, when
TextControl::set_selection_range is called).

I think TextInput's public API should not make it possible to put it
into an invalid state, as that would be a potential source of bugs.
That's why I didn't take this approach. (TextInput's public API does
currently make it possible to create an invalid state, but I'd like to
submit a follow-up patch to lock this down.)
  • Loading branch information
jonleighton committed Jan 26, 2018
1 parent e34f7c5 commit ce7bae8
Show file tree
Hide file tree
Showing 13 changed files with 198 additions and 740 deletions.
26 changes: 20 additions & 6 deletions components/script/dom/htmlinputelement.rs
Expand Up @@ -8,6 +8,7 @@ use dom::attr::Attr;
use dom::bindings::cell::DomRefCell;
use dom::bindings::codegen::Bindings::EventBinding::EventMethods;
use dom::bindings::codegen::Bindings::FileListBinding::FileListMethods;
use dom::bindings::codegen::Bindings::HTMLFormElementBinding::SelectionMode;
use dom::bindings::codegen::Bindings::HTMLInputElementBinding;
use dom::bindings::codegen::Bindings::HTMLInputElementBinding::HTMLInputElementMethods;
use dom::bindings::codegen::Bindings::KeyboardEventBinding::KeyboardEventMethods;
Expand Down Expand Up @@ -188,7 +189,6 @@ pub struct HTMLInputElement {
input_type: Cell<InputType>,
checked_changed: Cell<bool>,
placeholder: DomRefCell<DOMString>,
value_changed: Cell<bool>,
size: Cell<u32>,
maxlength: Cell<i32>,
minlength: Cell<i32>,
Expand Down Expand Up @@ -244,7 +244,6 @@ impl HTMLInputElement {
input_type: Cell::new(Default::default()),
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),
Expand Down Expand Up @@ -442,6 +441,10 @@ impl TextControl for HTMLInputElement {
}
}
}

fn set_dirty_value_flag(&self, value: bool) {
self.value_dirty.set(value)
}
}

impl HTMLInputElementMethods for HTMLInputElement {
Expand Down Expand Up @@ -581,7 +584,6 @@ impl HTMLInputElementMethods for HTMLInputElement {
}
}

self.value_changed.set(true);
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
Ok(())
}
Expand Down Expand Up @@ -751,6 +753,19 @@ impl HTMLInputElementMethods for HTMLInputElement {
self.set_dom_selection_range(start, end, direction)
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-setrangetext
fn SetRangeText(&self, replacement: DOMString) -> ErrorResult {
// defined in TextControl trait
self.set_dom_range_text(replacement, None, None, Default::default())
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-setrangetext
fn SetRangeText_(&self, replacement: DOMString, start: u32, end: u32,
selection_mode: SelectionMode) -> ErrorResult {
// defined in TextControl trait
self.set_dom_range_text(replacement, Some(start), Some(end), selection_mode)
}

// Select the files based on filepaths passed in,
// enabled by dom.htmlinputelement.select_files.enabled,
// used for test purpose.
Expand Down Expand Up @@ -931,7 +946,6 @@ impl HTMLInputElement {
self.SetValue(self.DefaultValue())
.expect("Failed to reset input value to default.");
self.value_dirty.set(false);
self.value_changed.set(false);
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
}

Expand Down Expand Up @@ -1213,7 +1227,7 @@ impl VirtualMethods for HTMLInputElement {

self.update_placeholder_shown_state();
},
&local_name!("value") if !self.value_changed.get() => {
&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));
Expand Down Expand Up @@ -1356,7 +1370,7 @@ impl VirtualMethods for HTMLInputElement {
keyevent.MetaKey());
},
DispatchInput => {
self.value_changed.set(true);
self.value_dirty.set(true);
self.update_placeholder_shown_state();
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
event.mark_as_handled();
Expand Down
32 changes: 25 additions & 7 deletions components/script/dom/htmltextareaelement.rs
Expand Up @@ -5,6 +5,7 @@
use dom::attr::Attr;
use dom::bindings::cell::DomRefCell;
use dom::bindings::codegen::Bindings::EventBinding::EventMethods;
use dom::bindings::codegen::Bindings::HTMLFormElementBinding::SelectionMode;
use dom::bindings::codegen::Bindings::HTMLTextAreaElementBinding;
use dom::bindings::codegen::Bindings::HTMLTextAreaElementBinding::HTMLTextAreaElementMethods;
use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
Expand Down Expand Up @@ -44,7 +45,7 @@ pub struct HTMLTextAreaElement {
textinput: DomRefCell<TextInput<ScriptToConstellationChan>>,
placeholder: DomRefCell<DOMString>,
// https://html.spec.whatwg.org/multipage/#concept-textarea-dirty
value_changed: Cell<bool>,
value_dirty: Cell<bool>,
form_owner: MutNullableDom<HTMLFormElement>,
}

Expand Down Expand Up @@ -122,7 +123,7 @@ impl HTMLTextAreaElement {
placeholder: DomRefCell::new(DOMString::new()),
textinput: DomRefCell::new(TextInput::new(
Lines::Multiple, DOMString::new(), chan, None, None, SelectionDirection::None)),
value_changed: Cell::new(false),
value_dirty: Cell::new(false),
form_owner: Default::default(),
}
}
Expand Down Expand Up @@ -156,6 +157,10 @@ impl TextControl for HTMLTextAreaElement {
fn has_selectable_text(&self) -> bool {
true
}

fn set_dirty_value_flag(&self, value: bool) {
self.value_dirty.set(value)
}
}

impl HTMLTextAreaElementMethods for HTMLTextAreaElement {
Expand Down Expand Up @@ -231,7 +236,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_changed.get() {
if !self.value_dirty.get() {
self.reset();
}
}
Expand All @@ -253,7 +258,7 @@ impl HTMLTextAreaElementMethods for HTMLTextAreaElement {
textinput.set_content(value);

// Step 3
self.value_changed.set(true);
self.value_dirty.set(true);

if old_value != textinput.get_content() {
// Step 4
Expand Down Expand Up @@ -309,14 +314,27 @@ impl HTMLTextAreaElementMethods for HTMLTextAreaElement {
fn SetSelectionRange(&self, start: u32, end: u32, direction: Option<DOMString>) -> ErrorResult {
self.set_dom_selection_range(start, end, direction)
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-setrangetext
fn SetRangeText(&self, replacement: DOMString) -> ErrorResult {
// defined in TextControl trait
self.set_dom_range_text(replacement, None, None, Default::default())
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-setrangetext
fn SetRangeText_(&self, replacement: DOMString, start: u32, end: u32,
selection_mode: SelectionMode) -> ErrorResult {
// defined in TextControl trait
self.set_dom_range_text(replacement, Some(start), Some(end), selection_mode)
}
}


impl HTMLTextAreaElement {
pub fn reset(&self) {
// https://html.spec.whatwg.org/multipage/#the-textarea-element:concept-form-reset-control
self.SetValue(self.DefaultValue());
self.value_changed.set(false);
self.value_dirty.set(false);
}
}

Expand Down Expand Up @@ -409,7 +427,7 @@ impl VirtualMethods for HTMLTextAreaElement {
if let Some(ref s) = self.super_type() {
s.children_changed(mutation);
}
if !self.value_changed.get() {
if !self.value_dirty.get() {
self.reset();
}
}
Expand All @@ -432,7 +450,7 @@ impl VirtualMethods for HTMLTextAreaElement {
match action {
KeyReaction::TriggerDefaultAction => (),
KeyReaction::DispatchInput => {
self.value_changed.set(true);
self.value_dirty.set(true);
self.update_placeholder_shown_state();
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
event.mark_as_handled();
Expand Down
129 changes: 121 additions & 8 deletions components/script/dom/textcontrol.rs
Expand Up @@ -3,19 +3,21 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use dom::bindings::cell::DomRefCell;
use dom::bindings::codegen::Bindings::HTMLFormElementBinding::SelectionMode;
use dom::bindings::conversions::DerivedFrom;
use dom::bindings::error::{Error, ErrorResult};
use dom::bindings::str::DOMString;
use dom::event::{EventBubbles, EventCancelable};
use dom::eventtarget::EventTarget;
use dom::node::{Node, NodeDamage, window_from_node};
use script_traits::ScriptToConstellationChan;
use textinput::{SelectionDirection, TextInput};
use textinput::{SelectionDirection, SelectionState, TextInput};

pub trait TextControl: DerivedFrom<EventTarget> + DerivedFrom<Node> {
fn textinput(&self) -> &DomRefCell<TextInput<ScriptToConstellationChan>>;
fn selection_api_applies(&self) -> bool;
fn has_selectable_text(&self) -> bool;
fn set_dirty_value_flag(&self, value: bool);

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-select
fn dom_select(&self) {
Expand All @@ -25,7 +27,7 @@ pub trait TextControl: DerivedFrom<EventTarget> + DerivedFrom<Node> {
}

// Step 2
self.set_selection_range(Some(0), Some(u32::max_value()), None);
self.set_selection_range(Some(0), Some(u32::max_value()), None, None);
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
Expand Down Expand Up @@ -57,7 +59,7 @@ pub trait TextControl: DerivedFrom<EventTarget> + DerivedFrom<Node> {
}

// Step 4
self.set_selection_range(start, Some(end), Some(self.selection_direction()));
self.set_selection_range(start, Some(end), Some(self.selection_direction()), None);
Ok(())
}

Expand All @@ -80,7 +82,7 @@ pub trait TextControl: DerivedFrom<EventTarget> + DerivedFrom<Node> {
}

// Step 2
self.set_selection_range(Some(self.selection_start()), end, Some(self.selection_direction()));
self.set_selection_range(Some(self.selection_start()), end, Some(self.selection_direction()), None);
Ok(())
}

Expand All @@ -105,7 +107,8 @@ pub trait TextControl: DerivedFrom<EventTarget> + DerivedFrom<Node> {
self.set_selection_range(
Some(self.selection_start()),
Some(self.selection_end()),
direction.map(|d| SelectionDirection::from(d))
direction.map(|d| SelectionDirection::from(d)),
None
);
Ok(())
}
Expand All @@ -118,7 +121,116 @@ pub trait TextControl: DerivedFrom<EventTarget> + DerivedFrom<Node> {
}

// Step 2
self.set_selection_range(Some(start), Some(end), direction.map(|d| SelectionDirection::from(d)));
self.set_selection_range(Some(start), Some(end), direction.map(|d| SelectionDirection::from(d)), None);
Ok(())
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-setrangetext
fn set_dom_range_text(&self, replacement: DOMString, start: Option<u32>, end: Option<u32>,
selection_mode: SelectionMode) -> ErrorResult {
// Step 1
if !self.selection_api_applies() {
return Err(Error::InvalidState);
}

// Step 2
self.set_dirty_value_flag(true);

// Step 3
let mut start = start.unwrap_or_else(|| self.selection_start());
let mut end = end.unwrap_or_else(|| self.selection_end());

// Step 4
if start > end {
return Err(Error::IndexSize);
}

// Save the original selection state to later pass to set_selection_range, because we will
// change the selection state in order to replace the text in the range.
let original_selection_state = self.textinput().borrow().selection_state();

let content_length = self.textinput().borrow().len() as u32;

// Step 5
if start > content_length {
start = content_length;
}

// Step 6
if end > content_length {
end = content_length;
}

// Step 7
let mut selection_start = self.selection_start();

// Step 8
let mut selection_end = self.selection_end();

// Step 11
// Must come before the textinput.replace_selection() call, as replacement gets moved in
// that call.
let new_length = replacement.len() as u32;

{
let mut textinput = self.textinput().borrow_mut();

// Steps 9-10
textinput.set_selection_range(start, end, SelectionDirection::None);
textinput.replace_selection(replacement);
}

// Step 12
let new_end = start + new_length;

// Step 13
match selection_mode {
SelectionMode::Select => {
selection_start = start;
selection_end = new_end;
},

SelectionMode::Start => {
selection_start = start;
selection_end = start;
},

SelectionMode::End => {
selection_start = new_end;
selection_end = new_end;
},

SelectionMode::Preserve => {
// Sub-step 1
let old_length = end - start;

// Sub-step 2
let delta = (new_length as isize) - (old_length as isize);

// Sub-step 3
if selection_start > end {
selection_start = ((selection_start as isize) + delta) as u32;
} else if selection_start > start {
selection_start = start;
}

// Sub-step 4
if selection_end > end {
selection_end = ((selection_end as isize) + delta) as u32;
} else if selection_end > start {
selection_end = new_end;
}
},
}

// Step 14
self.set_selection_range(
Some(selection_start),
Some(selection_end),
None,
Some(original_selection_state)
);

Ok(())
}

Expand All @@ -135,9 +247,10 @@ pub trait TextControl: DerivedFrom<EventTarget> + DerivedFrom<Node> {
}

// https://html.spec.whatwg.org/multipage/#set-the-selection-range
fn set_selection_range(&self, start: Option<u32>, end: Option<u32>, direction: Option<SelectionDirection>) {
fn set_selection_range(&self, start: Option<u32>, end: Option<u32>, direction: Option<SelectionDirection>,
original_selection_state: Option<SelectionState>) {
let mut textinput = self.textinput().borrow_mut();
let original_selection_state = textinput.selection_state();
let original_selection_state = original_selection_state.unwrap_or_else(|| textinput.selection_state());

// Step 1
let start = start.unwrap_or(0);
Expand Down
8 changes: 8 additions & 0 deletions components/script/dom/webidls/HTMLFormElement.webidl
Expand Up @@ -35,3 +35,11 @@ interface HTMLFormElement : HTMLElement {
//boolean checkValidity();
//boolean reportValidity();
};

// https://html.spec.whatwg.org/multipage/#selectionmode
enum SelectionMode {
"preserve", // default
"select",
"start",
"end"
};

0 comments on commit ce7bae8

Please sign in to comment.