Auto merge of #19272 - jonleighton:issue-19171-2, r=jdm

Further changes in relation to #19171

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19272)
<!-- Reviewable:end -->

components/script/dom/htmlinputelement.rs

@@ -31,6 +31,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::validation::Validatable;
 use dom::validitystate::ValidationFlags;
 use dom::virtualmethods::VirtualMethods;
@@ -292,6 +293,12 @@ impl LayoutHTMLInputElementHelpers for LayoutDom<HTMLInputElement> {
     }
 }
 
+impl TextControl for HTMLInputElement {
+    fn textinput(&self) -> &DomRefCell<TextInput<ScriptToConstellationChan>> {
+        &self.textinput
+    }
+}
+
 impl HTMLInputElementMethods for HTMLInputElement {
     // https://html.spec.whatwg.org/multipage/#dom-input-accept
     make_getter!(Accept, "accept");
@@ -567,42 +574,39 @@ impl HTMLInputElementMethods for HTMLInputElement {
         }
     }
 
-    // https://html.spec.whatwg.org/multipage/#dom-input-selectionstart
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
     fn SelectionStart(&self) -> u32 {
-        self.textinput.borrow().get_selection_start()
+        self.dom_selection_start()
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
     fn SetSelectionStart(&self, start: u32) {
-        self.set_selection_range(start, self.SelectionEnd(), self.selection_direction());
+        self.set_dom_selection_start(start);
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
     fn SelectionEnd(&self) -> u32 {
-        self.textinput.borrow().get_absolute_insertion_point() as u32
+        self.dom_selection_end()
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
     fn SetSelectionEnd(&self, end: u32) {
-        self.set_selection_range(self.SelectionStart(), end, self.selection_direction());
+        self.set_dom_selection_end(end)
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
     fn SelectionDirection(&self) -> DOMString {
-        DOMString::from(self.textinput.borrow().selection_direction)
+        self.dom_selection_direction()
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
     fn SetSelectionDirection(&self, direction: DOMString) {
-        self.textinput.borrow_mut().selection_direction = SelectionDirection::from(direction);
+        self.set_dom_selection_direction(direction);
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-textarea/input-setselectionrange
     fn SetSelectionRange(&self, start: u32, end: u32, direction: Option<DOMString>) {
-        // Step 4
+        self.set_dom_selection_range(start, end, direction);
-        let direction = direction.map_or(SelectionDirection::None, |d| SelectionDirection::from(d));
-
-        self.set_selection_range(start, end, direction);
     }
 
     // Select the files based on filepaths passed in,
@@ -875,30 +879,6 @@ impl HTMLInputElement {
             _ => ()
         }
     }
-
-    fn selection_direction(&self) -> SelectionDirection {
-        self.textinput.borrow().selection_direction
-    }
-
-    // https://html.spec.whatwg.org/multipage/#set-the-selection-range
-    fn set_selection_range(&self, start: u32, end: u32, direction: SelectionDirection) {
-        // Step 5
-        self.textinput.borrow_mut().selection_direction = direction;
-
-        // Step 3
-        self.textinput.borrow_mut().set_selection_range(start, end);
-
-        // Step 6
-        let window = window_from_node(self);
-        let _ = window.user_interaction_task_source().queue_event(
-            &self.upcast(),
-            atom!("select"),
-            EventBubbles::Bubbles,
-            EventCancelable::NotCancelable,
-            &window);
-
-        self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
-    }
 }
 
 impl VirtualMethods for HTMLInputElement {

components/script/dom/htmltextareaelement.rs

@@ -23,6 +23,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::validation::Validatable;
 use dom::virtualmethods::VirtualMethods;
 use dom_struct::dom_struct;
@@ -140,6 +141,12 @@ impl HTMLTextAreaElement {
     }
 }
 
+impl TextControl for HTMLTextAreaElement {
+    fn textinput(&self) -> &DomRefCell<TextInput<ScriptToConstellationChan>> {
+        &self.textinput
+    }
+}
+
 impl HTMLTextAreaElementMethods for HTMLTextAreaElement {
     // TODO A few of these attributes have default values and additional
     // constraints
@@ -237,53 +244,39 @@ impl HTMLTextAreaElementMethods for HTMLTextAreaElement {
         self.upcast::<HTMLElement>().labels()
     }
 
-    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
-    fn SetSelectionDirection(&self, direction: DOMString) {
+    fn SelectionStart(&self) -> u32 {
-        self.textinput.borrow_mut().selection_direction = SelectionDirection::from(direction);
+        self.dom_selection_start()
-    }
-
-    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
-    fn SelectionDirection(&self) -> DOMString {
-        DOMString::from(self.textinput.borrow().selection_direction)
-    }
-
-    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
-    fn SetSelectionEnd(&self, end: u32) {
-        let selection_start = self.SelectionStart();
-        self.textinput.borrow_mut().set_selection_range(selection_start, end);
-        self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
-    }
-
-    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
-    fn SelectionEnd(&self) -> u32 {
-        self.textinput.borrow().get_absolute_insertion_point() as u32
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
     fn SetSelectionStart(&self, start: u32) {
-        let selection_end = self.SelectionEnd();
+        self.set_dom_selection_start(start);
-        self.textinput.borrow_mut().set_selection_range(start, selection_end);
-        self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
     }
 
-    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
-    fn SelectionStart(&self) -> u32 {
+    fn SelectionEnd(&self) -> u32 {
-        self.textinput.borrow().get_selection_start()
+        self.dom_selection_end()
+    }
+
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
+    fn SetSelectionEnd(&self, end: u32) {
+        self.set_dom_selection_end(end);
+    }
+
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
+    fn SelectionDirection(&self) -> DOMString {
+        self.dom_selection_direction()
+    }
+
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
+    fn SetSelectionDirection(&self, direction: DOMString) {
+        self.set_dom_selection_direction(direction);
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-textarea/input-setselectionrange
     fn SetSelectionRange(&self, start: u32, end: u32, direction: Option<DOMString>) {
-        let direction = direction.map_or(SelectionDirection::None, |d| SelectionDirection::from(d));
+        self.set_dom_selection_range(start, end, direction);
-        self.textinput.borrow_mut().selection_direction = direction;
-        self.textinput.borrow_mut().set_selection_range(start, end);
-        let window = window_from_node(self);
-        let _ = window.user_interaction_task_source().queue_event(
-            &self.upcast(),
-            atom!("select"),
-            EventBubbles::Bubbles,
-            EventCancelable::NotCancelable,
-            &window);
-        self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
     }
 }
 

components/script/dom/mod.rs

@@ -439,6 +439,7 @@ pub mod testrunner;
 pub mod testworklet;
 pub mod testworkletglobalscope;
 pub mod text;
+pub mod textcontrol;
 pub mod textdecoder;
 pub mod textencoder;
 pub mod touch;

components/script/dom/textcontrol.rs

@@ -0,0 +1,78 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+use dom::bindings::cell::DomRefCell;
+use dom::bindings::conversions::DerivedFrom;
+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};
+
+pub trait TextControl: DerivedFrom<EventTarget> + DerivedFrom<Node> {
+    fn textinput(&self) -> &DomRefCell<TextInput<ScriptToConstellationChan>>;
+
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
+    fn dom_selection_start(&self) -> u32 {
+        self.textinput().borrow().get_selection_start()
+    }
+
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionstart
+    fn set_dom_selection_start(&self, start: u32) {
+        self.set_selection_range(start, self.dom_selection_end(), self.selection_direction());
+    }
+
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
+    fn dom_selection_end(&self) -> u32 {
+        self.textinput().borrow().get_absolute_insertion_point() as u32
+    }
+
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectionend
+    fn set_dom_selection_end(&self, end: u32) {
+        self.set_selection_range(self.dom_selection_start(), end, self.selection_direction());
+    }
+
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
+    fn dom_selection_direction(&self) -> DOMString {
+        DOMString::from(self.selection_direction())
+    }
+
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-selectiondirection
+    fn set_dom_selection_direction(&self, direction: DOMString) {
+        self.textinput().borrow_mut().selection_direction = SelectionDirection::from(direction);
+    }
+
+    // https://html.spec.whatwg.org/multipage/#dom-textarea/input-setselectionrange
+    fn set_dom_selection_range(&self, start: u32, end: u32, direction: Option<DOMString>) {
+        // Step 4
+        let direction = direction.map_or(SelectionDirection::None, |d| SelectionDirection::from(d));
+
+        self.set_selection_range(start, end, direction);
+    }
+
+    fn selection_direction(&self) -> SelectionDirection {
+        self.textinput().borrow().selection_direction
+    }
+
+    // https://html.spec.whatwg.org/multipage/#set-the-selection-range
+    fn set_selection_range(&self, start: u32, end: u32, direction: SelectionDirection) {
+        // Step 5
+        self.textinput().borrow_mut().selection_direction = direction;
+
+        // Step 3
+        self.textinput().borrow_mut().set_selection_range(start, end);
+
+        // Step 6
+        let window = window_from_node(self);
+        let _ = window.user_interaction_task_source().queue_event(
+            &self.upcast::<EventTarget>(),
+            atom!("select"),
+            EventBubbles::Bubbles,
+            EventCancelable::NotCancelable,
+            &window);
+
+        self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
+    }
+}

tests/wpt/metadata/MANIFEST.json

@@ -542600,7 +542600,7 @@
    "testharness"
   ],
   "html/semantics/forms/textfieldselection/selection-start-end.html": [
-   "e38a79075e27780327f49e7ae9cadd2558165eac",
+   "3fd1c942f7ac3ed3097bbd1ec89db15fb0805476",
    "testharness"
   ],
   "html/semantics/forms/textfieldselection/selection-value-interactions.html": [

tests/wpt/metadata/html/semantics/forms/textfieldselection/selection-start-end.html.ini

@@ -1,11 +1,5 @@
 [selection-start-end.html]
   type: testharness
-  [onselect should fire when selectionStart is changed]
-    expected: FAIL
-
-  [onselect should fire when selectionEnd is changed]
-    expected: FAIL
-
   [Setting selectionStart to a value larger than selectionEnd should increase selectionEnd]
     expected: FAIL
 
@@ -15,3 +9,27 @@
   [selectionStart edge-case values]
     expected: FAIL
 
+  [Initial .value set on textarea-appended should set selectionStart to end of value]
+    expected: FAIL
+
+  [Initial .value set on textarea-not-appended should set selectionStart to end of value]
+    expected: FAIL
+
+  [Initial .value set on textarea-appended-prefocused should set selectionStart to end of value]
+    expected: FAIL
+
+  [Initial .value set on textarea-not-appended-prefocused should set selectionStart to end of value]
+    expected: FAIL
+
+  [Initial .value set on textarea-appended should set selectionEnd to end of value]
+    expected: FAIL
+
+  [Initial .value set on textarea-not-appended should set selectionEnd to end of value]
+    expected: FAIL
+
+  [Initial .value set on textarea-appended-prefocused should set selectionEnd to end of value]
+    expected: FAIL
+
+  [Initial .value set on textarea-not-appended-prefocused should set selectionEnd to end of value]
+    expected: FAIL
+

tests/wpt/web-platform-tests/html/semantics/forms/textfieldselection/selection-start-end.html

@@ -59,10 +59,16 @@
     assert_equals(testValue.length, 10);
   }, "Sanity check for testValue length; if this fails, variou absolute offsets in the test below need to be adjusted to be less than testValue.length");
 
+  for (let prop of ["selectionStart", "selectionEnd"]) {
+    for (let el of createTestElements(testValue)) {
+      test(function() {
+        assert_equals(el.selectionStart, testValue.length);
+      }, `Initial .value set on ${el.id} should set ${prop} to end of value`);
+    }
+  }
+
   test(function() {
     for (let el of createTestElements(testValue)) {
-      assert_equals(el.selectionStart, testValue.length,
-                    `Initial .value set on ${el.id} should set selectionStart to end of value`);
       var t = async_test(`onselect should fire when selectionStart is changed on ${el.id}`);
       el.onselect = t.step_func_done(function(e) {
         assert_equals(e.type, "select");
@@ -74,8 +80,6 @@
 
   test(function() {
     for (let el of createTestElements(testValue)) {
-      assert_equals(el.selectionEnd, testValue.length,
-                    `Initial .value set on ${el.id} should set selectionEnd to end of value`);
       var t = async_test(`onselect should fire when selectionEnd is changed on ${el.id}`);
       el.onselect = t.step_func_done(function(e) {
         assert_equals(e.type, "select");
@@ -87,8 +91,6 @@
 
   test(function() {
     for (let el of createTestElements(testValue)) {
-      assert_equals(el.selectionStart, testValue.length,
-                    `Initial .value set on ${el.id} should set selectionStart to end of value`);
       el.selectionStart = 0;
       el.selectionEnd = 5;
       el.selectionStart = 8;
@@ -100,10 +102,6 @@
 
   test(function() {
     for (let el of createTestElements(testValue)) {
-      assert_equals(el.selectionStart, testValue.length,
-                    `Initial .value set on ${el.id} should set selectionStart to end of value`);
-      assert_equals(el.selectionEnd, testValue.length,
-                    `Initial .value set on ${el.id} should set selectionEnd to end of value`);
       el.selectionStart = 8;
       el.selectionEnd = 5;
       assert_equals(el.selectionStart, 5, `selectionStart on ${el.id}`);