Skip to content

Commit

Permalink
Refactor implementation of TextControl
Browse files Browse the repository at this point in the history 
The intention here is to make the flow more explicit. I.e. rather than
calling `self.dom_select()` and relying on the programmer to
know/realise that this method is provided by a trait, we call
`self.selection().dom_select()` and the programmer can inspect the
definition of `self.selection()` to follow the code.

This came out of a discussion with KiChjang here:

#19544 (comment)

Note that I tried to make "selection" be a member field of
HTML{Input,TextArea}Element but it opened up a whole can of worms with
lifetimes, so it seemed simpler to not do that since it is not
essential for this code to work.
  • Loading branch information
@jonleighton
jonleighton committed Jan 30, 2018
1 parent 76b4e5c commit 0461681
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 95 deletions.
35 changes: 17 additions & 18 deletions components/script/dom/htmlinputelement.rs
View file
Expand Up @@ -32,7 +32,7 @@ use dom::mouseevent::MouseEvent;
use dom::node::{Node, NodeDamage, UnbindContext};
use dom::node::{document_from_node, window_from_node};
use dom::nodelist::NodeList;
use dom::textcontrol::TextControl;
use dom::textcontrol::{TextControlElement, TextControlSelection};
use dom::validation::Validatable;
use dom::validitystate::ValidationFlags;
use dom::virtualmethods::VirtualMethods;
Expand Down Expand Up @@ -400,11 +400,7 @@ impl LayoutHTMLInputElementHelpers for LayoutDom<HTMLInputElement> {
}
}

impl TextControl for HTMLInputElement {
fn textinput(&self) -> &DomRefCell<TextInput<ScriptToConstellationChan>> {
&self.textinput
}

impl TextControlElement for HTMLInputElement {
// https://html.spec.whatwg.org/multipage/#concept-input-apply
fn selection_api_applies(&self) -> bool {
match self.input_type() {
Expand Down Expand Up @@ -715,55 +711,53 @@ impl HTMLInputElementMethods for HTMLInputElement {

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-select
fn Select(&self) {
self.dom_select(); // defined in TextControl trait
self.selection().dom_select();
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
fn GetSelectionStart(&self) -> Option<u32> {
self.get_dom_selection_start()
self.selection().dom_start()
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
fn SetSelectionStart(&self, start: Option<u32>) -> ErrorResult {
self.set_dom_selection_start(start)
self.selection().set_dom_start(start)
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
fn GetSelectionEnd(&self) -> Option<u32> {
self.get_dom_selection_end()
self.selection().dom_end()
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
fn SetSelectionEnd(&self, end: Option<u32>) -> ErrorResult {
self.set_dom_selection_end(end)
self.selection().set_dom_end(end)
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
fn GetSelectionDirection(&self) -> Option<DOMString> {
self.get_dom_selection_direction()
self.selection().dom_direction()
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
fn SetSelectionDirection(&self, direction: Option<DOMString>) -> ErrorResult {
self.set_dom_selection_direction(direction)
self.selection().set_dom_direction(direction)
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-setselectionrange
fn SetSelectionRange(&self, start: u32, end: u32, direction: Option<DOMString>) -> ErrorResult {
self.set_dom_selection_range(start, end, direction)
self.selection().set_dom_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())
self.selection().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)
self.selection().set_dom_range_text(replacement, Some(start), Some(end), selection_mode)
}

// Select the files based on filepaths passed in,
Expand Down Expand Up @@ -1104,6 +1098,11 @@ impl HTMLInputElement {
_ => ()
}
}

#[allow(unrooted_must_root)]
fn selection(&self) -> TextControlSelection<Self> {
TextControlSelection::new(&self, &self.textinput)
}
}

impl VirtualMethods for HTMLInputElement {
Expand Down
35 changes: 17 additions & 18 deletions components/script/dom/htmltextareaelement.rs
View file
Expand Up @@ -25,7 +25,7 @@ use dom::keyboardevent::KeyboardEvent;
use dom::node::{ChildrenMutation, Node, NodeDamage, UnbindContext};
use dom::node::{document_from_node, window_from_node};
use dom::nodelist::NodeList;
use dom::textcontrol::TextControl;
use dom::textcontrol::{TextControlElement, TextControlSelection};
use dom::validation::Validatable;
use dom::virtualmethods::VirtualMethods;
use dom_struct::dom_struct;
Expand Down Expand Up @@ -145,11 +145,7 @@ impl HTMLTextAreaElement {
}
}

impl TextControl for HTMLTextAreaElement {
fn textinput(&self) -> &DomRefCell<TextInput<ScriptToConstellationChan>> {
&self.textinput
}

impl TextControlElement for HTMLTextAreaElement {
fn selection_api_applies(&self) -> bool {
true
}
Expand Down Expand Up @@ -277,55 +273,53 @@ impl HTMLTextAreaElementMethods for HTMLTextAreaElement {

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-select
fn Select(&self) {
self.dom_select(); // defined in TextControl trait
self.selection().dom_select();
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
fn GetSelectionStart(&self) -> Option<u32> {
self.get_dom_selection_start()
self.selection().dom_start()
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
fn SetSelectionStart(&self, start: Option<u32>) -> ErrorResult {
self.set_dom_selection_start(start)
self.selection().set_dom_start(start)
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
fn GetSelectionEnd(&self) -> Option<u32> {
self.get_dom_selection_end()
self.selection().dom_end()
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
fn SetSelectionEnd(&self, end: Option<u32>) -> ErrorResult {
self.set_dom_selection_end(end)
self.selection().set_dom_end(end)
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
fn GetSelectionDirection(&self) -> Option<DOMString> {
self.get_dom_selection_direction()
self.selection().dom_direction()
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
fn SetSelectionDirection(&self, direction: Option<DOMString>) -> ErrorResult {
self.set_dom_selection_direction(direction)
self.selection().set_dom_direction(direction)
}

// https://html.spec.whatwg.org/multipage/#dom-textarea/input-setselectionrange
fn SetSelectionRange(&self, start: u32, end: u32, direction: Option<DOMString>) -> ErrorResult {
self.set_dom_selection_range(start, end, direction)
self.selection().set_dom_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())
self.selection().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)
self.selection().set_dom_range_text(replacement, Some(start), Some(end), selection_mode)
}
}

Expand All @@ -336,6 +330,11 @@ impl HTMLTextAreaElement {
self.SetValue(self.DefaultValue());
self.value_dirty.set(false);
}

#[allow(unrooted_must_root)]
fn selection(&self) -> TextControlSelection<Self> {
TextControlSelection::new(&self, &self.textinput)
}
}


Expand Down

0 comments on commit 0461681

Please sign in to comment.