servo: Merge #5800 - Made the clipboard-related functionality in TextInput more testable. Add (from aweinstock314:x11-clipboard); r=jdm

...ed test_clipboard_paste to the "test-unit" suite.

Source-Repo: https://github.com/servo/servo
Source-Revision: 5cb1356e9e3dbde9fd841c9aa2d21ea39c5eda18

servo/components/script/clipboard_provider.rs

@@ -0,0 +1,48 @@
+/* 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 msg::constellation_msg::ConstellationChan;
+use msg::constellation_msg::Msg as ConstellationMsg;
+
+use collections::borrow::ToOwned;
+use std::sync::mpsc::channel;
+
+pub trait ClipboardProvider {
+    // blocking method to get the clipboard contents
+    fn get_clipboard_contents(&mut self) -> String;
+    // blocking method to set the clipboard contents
+    fn set_clipboard_contents(&mut self, &str);
+}
+
+impl ClipboardProvider for ConstellationChan {
+    fn get_clipboard_contents(&mut self) -> String {
+        let (tx, rx) = channel();
+        self.0.send(ConstellationMsg::GetClipboardContents(tx)).unwrap();
+        rx.recv().unwrap()
+    }
+    fn set_clipboard_contents(&mut self, _: &str) {
+        panic!("not yet implemented");
+    }
+}
+
+pub struct DummyClipboardContext {
+    content: String
+}
+
+impl DummyClipboardContext {
+    pub fn new(s: &str) -> DummyClipboardContext {
+        DummyClipboardContext {
+            content: s.to_owned()
+        }
+    }
+}
+
+impl ClipboardProvider for DummyClipboardContext {
+    fn get_clipboard_contents(&mut self) -> String {
+        self.content.clone()
+    }
+    fn set_clipboard_contents(&mut self, s: &str) {
+        self.content = s.to_owned();
+    }
+}

servo/components/script/dom/document.rs

@@ -701,8 +701,8 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> {
         let props = KeyboardEvent::key_properties(key, modifiers);
 
         let keyevent = KeyboardEvent::new(window.r(), ev_type, true, true,
-                                          Some(window.r()), 0,
-                                          props.key.to_owned(), props.code.to_owned(),
+                                          Some(window.r()), 0, Some(key),
+                                          props.key_string.to_owned(), props.code.to_owned(),
                                           props.location, is_repeating, is_composing,
                                           ctrl, alt, shift, meta,
                                           None, props.key_code).root();
@@ -714,8 +714,8 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> {
         if state != KeyState::Released && props.is_printable() && !prevented {
             // https://dvcs.w3.org/hg/dom3events/raw-file/tip/html/DOM3-Events.html#keypress-event-order
             let event = KeyboardEvent::new(window.r(), "keypress".to_owned(),
-                                           true, true, Some(window.r()),
-                                           0, props.key.to_owned(), props.code.to_owned(),
+                                           true, true, Some(window.r()), 0, Some(key),
+                                            props.key_string.to_owned(), props.code.to_owned(),
                                            props.location, is_repeating, is_composing,
                                            ctrl, alt, shift, meta,
                                            props.char_code, 0).root();

servo/components/script/dom/htmlinputelement.rs

@@ -35,6 +35,7 @@ use dom::window::WindowHelpers;
 use textinput::TextInput;
 use textinput::KeyReaction::{TriggerDefaultAction, DispatchInput, Nothing};
 use textinput::Lines::Single;
+use msg::constellation_msg::ConstellationChan;
 
 use util::str::DOMString;
 use string_cache::Atom;
@@ -72,7 +73,7 @@ pub struct HTMLInputElement {
     indeterminate: Cell<bool>,
     value_changed: Cell<bool>,
     size: Cell<u32>,
-    textinput: DOMRefCell<TextInput>,
+    textinput: DOMRefCell<TextInput<ConstellationChan>>,
     activation_state: DOMRefCell<InputActivationState>,
 }
 
@@ -122,7 +123,7 @@ impl HTMLInputElement {
             checked_changed: Cell::new(false),
             value_changed: Cell::new(false),
             size: Cell::new(DEFAULT_INPUT_SIZE),
-            textinput: DOMRefCell::new(TextInput::new(Single, "".to_owned(), Some(chan))),
+            textinput: DOMRefCell::new(TextInput::new(Single, "".to_owned(), chan)),
             activation_state: DOMRefCell::new(InputActivationState::new())
         }
     }

servo/components/script/dom/htmltextareaelement.rs

@@ -30,6 +30,7 @@ use textinput::{TextInput, Lines, KeyReaction};
 use dom::virtualmethods::VirtualMethods;
 use dom::window::WindowHelpers;
 use script_task::{ScriptMsg, Runnable};
+use msg::constellation_msg::ConstellationChan;
 
 use util::str::DOMString;
 use string_cache::Atom;
@@ -40,7 +41,7 @@ use std::cell::Cell;
 #[dom_struct]
 pub struct HTMLTextAreaElement {
     htmlelement: HTMLElement,
-    textinput: DOMRefCell<TextInput>,
+    textinput: DOMRefCell<TextInput<ConstellationChan>>,
     cols: Cell<u32>,
     rows: Cell<u32>,
     // https://html.spec.whatwg.org/multipage/#concept-textarea-dirty
@@ -95,7 +96,7 @@ impl HTMLTextAreaElement {
         let chan = document.window().root().r().constellation_chan();
         HTMLTextAreaElement {
             htmlelement: HTMLElement::new_inherited(HTMLElementTypeId::HTMLTextAreaElement, localName, prefix, document),
-            textinput: DOMRefCell::new(TextInput::new(Lines::Multiple, "".to_owned(), Some(chan))),
+            textinput: DOMRefCell::new(TextInput::new(Lines::Multiple, "".to_owned(), chan)),
             cols: Cell::new(DEFAULT_COLS),
             rows: Cell::new(DEFAULT_ROWS),
             value_changed: Cell::new(false),

servo/components/script/lib.rs

@@ -66,6 +66,7 @@ pub mod page;
 pub mod script_task;
 mod timers;
 pub mod textinput;
+pub mod clipboard_provider;
 mod devtools;
 mod horribly_inefficient_timers;
 mod webdriver_handlers;

servo/components/script/textinput.rs

@@ -4,17 +4,17 @@
 
 //! Common handling of keyboard input and state management for text input controls
 
-use dom::bindings::codegen::Bindings::KeyboardEventBinding::KeyboardEventMethods;
+use clipboard_provider::ClipboardProvider;
 use dom::bindings::js::JSRef;
-use msg::constellation_msg::ConstellationChan;
-use msg::constellation_msg::Msg as ConstellationMsg;
-use dom::keyboardevent::KeyboardEvent;
+use dom::keyboardevent::{KeyboardEvent, KeyboardEventHelpers, key_value};
+use msg::constellation_msg::{SHIFT, CONTROL, ALT, SUPER};
+use msg::constellation_msg::{Key, KeyModifiers};
 use util::str::DOMString;
 
 use std::borrow::ToOwned;
 use std::cmp::{min, max};
 use std::default::Default;
-use std::sync::mpsc::channel;
+
 
 #[derive(Copy, Clone, PartialEq)]
 pub enum Selection {
@@ -33,7 +33,7 @@ pub struct TextPoint {
 
 /// Encapsulated state for handling keyboard input in a single or multiline text input control.
 #[jstraceable]
-pub struct TextInput {
+pub struct TextInput<T: ClipboardProvider> {
     /// Current text input content, split across lines without trailing '\n'
     lines: Vec<DOMString>,
     /// Current cursor input point
@@ -42,7 +42,7 @@ pub struct TextInput {
     selection_begin: Option<TextPoint>,
     /// Is this a multiline input?
     multiline: bool,
-    constellation_channel: Option<ConstellationChan>
+    clipboard_provider: T,
 }
 
 /// Resulting action to be taken by the owner of a text input that is handling an event.
@@ -79,24 +79,42 @@ pub enum DeleteDir {
 /// Was the keyboard event accompanied by the standard control modifier,
 /// i.e. cmd on Mac OS or ctrl on other platforms.
 #[cfg(target_os="macos")]
-fn is_control_key(event: JSRef<KeyboardEvent>) -> bool {
-    event.MetaKey() && !event.CtrlKey() && !event.AltKey()
+fn is_control_key(mods: KeyModifiers) -> bool {
+    mods.contains(SUPER) && !mods.contains(CONTROL | ALT)
 }
 
 #[cfg(not(target_os="macos"))]
-fn is_control_key(event: JSRef<KeyboardEvent>) -> bool {
-    event.CtrlKey() && !event.MetaKey() && !event.AltKey()
+fn is_control_key(mods: KeyModifiers) -> bool {
+    mods.contains(CONTROL) && !mods.contains(SUPER | ALT)
 }
 
-impl TextInput {
+fn is_printable_key(key: Key) -> bool {
+    match key {
+        Key::Space | Key::Apostrophe | Key::Comma | Key::Minus |
+        Key::Period | Key::Slash | Key::GraveAccent | Key::Num0 |
+        Key::Num1 | Key::Num2 | Key::Num3 | Key::Num4 | Key::Num5 |
+        Key::Num6 | Key::Num7 | Key::Num8 | Key::Num9 | Key::Semicolon |
+        Key::Equal | Key::A | Key::B | Key::C | Key::D | Key::E | Key::F |
+        Key::G | Key::H | Key::I | Key::J | Key::K | Key::L | Key::M | Key::N |
+        Key::O | Key::P | Key::Q | Key::R | Key::S | Key::T | Key::U | Key::V |
+        Key::W | Key::X | Key::Y | Key::Z | Key::LeftBracket | Key::Backslash |
+        Key::RightBracket | Key::Kp0 | Key::Kp1 | Key::Kp2 | Key::Kp3 |
+        Key::Kp4 | Key::Kp5 | Key::Kp6 | Key::Kp7 | Key::Kp8 | Key::Kp9 |
+        Key::KpDecimal | Key::KpDivide | Key::KpMultiply | Key::KpSubtract |
+        Key::KpAdd | Key::KpEqual => true,
+        _ => false,
+    }
+}
+
+impl<T: ClipboardProvider> TextInput<T> {
     /// Instantiate a new text input control
-    pub fn new(lines: Lines, initial: DOMString, cc: Option<ConstellationChan>) -> TextInput {
+    pub fn new(lines: Lines, initial: DOMString, clipboard_provider: T) -> TextInput<T> {
         let mut i = TextInput {
             lines: vec!(),
             edit_point: Default::default(),
             selection_begin: None,
             multiline: lines == Lines::Multiple,
-            constellation_channel: cc,
+            clipboard_provider: clipboard_provider
         };
         i.set_content(initial);
         i
@@ -283,81 +301,74 @@ impl TextInput {
 
     /// Process a given `KeyboardEvent` and return an action for the caller to execute.
     pub fn handle_keydown(&mut self, event: JSRef<KeyboardEvent>) -> KeyReaction {
-        //A simple way to convert an event to a selection
-        fn maybe_select(event: JSRef<KeyboardEvent>) -> Selection {
-            if event.ShiftKey() {
-                return Selection::Selected
-            }
-            return Selection::NotSelected
+        if let Some(key) = event.get_key() {
+            self.handle_keydown_aux(key, event.get_key_modifiers())
+        } else {
+            KeyReaction::Nothing
         }
-        match &*event.Key() {
-           "a" if is_control_key(event) => {
+    }
+    pub fn handle_keydown_aux(&mut self, key: Key, mods: KeyModifiers) -> KeyReaction {
+        let maybe_select = if mods.contains(SHIFT) { Selection::Selected } else { Selection::NotSelected };
+        match key {
+           Key::A if is_control_key(mods) => {
                 self.select_all();
                 KeyReaction::Nothing
             },
-            "v" if is_control_key(event) => {
-                let (tx, rx) = channel();
-                let mut contents = None;
-                if let Some(ref cc) = self.constellation_channel {
-                    cc.0.send(ConstellationMsg::GetClipboardContents(tx)).unwrap();
-                    contents = Some(rx.recv().unwrap());
-                }
-                if let Some(contents) = contents {
-                    self.insert_string(&contents);
-                }
+           Key::V if is_control_key(mods) => {
+                let contents = self.clipboard_provider.get_clipboard_contents();
+                self.insert_string(&contents);
                 KeyReaction::DispatchInput
             },
-            // printable characters have single-character key values
-            c if c.len() == 1 => {
-                self.insert_char(c.chars().next().unwrap());
+            _ if is_printable_key(key) => {
+                self.insert_string(key_value(key, mods));
                 KeyReaction::DispatchInput
             }
-            "Space" => {
+            Key::Space => {
                 self.insert_char(' ');
                 KeyReaction::DispatchInput
             }
-            "Delete" => {
+            Key::Delete => {
                 self.delete_char(DeleteDir::Forward);
                 KeyReaction::DispatchInput
             }
-            "Backspace" => {
+            Key::Backspace => {
                 self.delete_char(DeleteDir::Backward);
                 KeyReaction::DispatchInput
             }
-            "ArrowLeft" => {
-                self.adjust_horizontal(-1, maybe_select(event));
+            Key::Left => {
+                self.adjust_horizontal(-1, maybe_select);
                 KeyReaction::Nothing
             }
-            "ArrowRight" => {
-                self.adjust_horizontal(1, maybe_select(event));
+            Key::Right => {
+                self.adjust_horizontal(1, maybe_select);
                 KeyReaction::Nothing
             }
-            "ArrowUp" => {
-                self.adjust_vertical(-1, maybe_select(event));
+            Key::Up => {
+                self.adjust_vertical(-1, maybe_select);
                 KeyReaction::Nothing
             }
-            "ArrowDown" => {
-                self.adjust_vertical(1, maybe_select(event));
+            Key::Down => {
+                self.adjust_vertical(1, maybe_select);
                 KeyReaction::Nothing
             }
-            "Enter" => self.handle_return(),
-            "Home" => {
+            Key::Enter | Key::KpEnter => self.handle_return(),
+            Key::Home => {
                 self.edit_point.index = 0;
                 KeyReaction::Nothing
             }
-            "End" => {
+            Key::End => {
                 self.edit_point.index = self.current_line_length();
                 KeyReaction::Nothing
             }
-            "PageUp" => {
-                self.adjust_vertical(-28, maybe_select(event));
+            Key::PageUp => {
+                self.adjust_vertical(-28, maybe_select);
                 KeyReaction::Nothing
             }
-            "PageDown" => {
-                self.adjust_vertical(28, maybe_select(event));
+            Key::PageDown => {
+                self.adjust_vertical(28, maybe_select);
                 KeyReaction::Nothing
             }
-            "Tab" => KeyReaction::TriggerDefaultAction,
+            Key::Tab => KeyReaction::TriggerDefaultAction,
             _ => KeyReaction::Nothing,
         }
     }

servo/components/servo/Cargo.lock

@@ -1037,6 +1037,7 @@ dependencies = [
 name = "script_tests"
 version = "0.0.1"
 dependencies = [
+ "msg 0.0.1",
  "script 0.0.1",
 ]
 

servo/tests/unit/script/Cargo.toml

@@ -8,5 +8,8 @@ name = "script_tests"
 path = "lib.rs"
 doctest = false
 
+[dependencies.msg]
+path = "../../../components/msg"
+
 [dependencies.script]
 path = "../../../components/script"

servo/tests/unit/script/lib.rs

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 extern crate script;
+extern crate msg;
 
 #[cfg(all(test, target_pointer_width = "64"))] mod size_of;
 #[cfg(test)] mod textinput;

servo/tests/unit/script/textinput.rs

@@ -7,12 +7,20 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use msg::constellation_msg::{Key, KeyModifiers};
+
+#[cfg(target_os="macos")]
+use msg::constellation_msg::SUPER;
+#[cfg(not(target_os="macos"))]
+use msg::constellation_msg::CONTROL;
+
 use script::textinput::{TextInput, Selection, Lines, DeleteDir};
+use script::clipboard_provider::DummyClipboardContext;
 use std::borrow::ToOwned;
 
 #[test]
 fn test_textinput_delete_char() {
-    let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), None);
+    let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), DummyClipboardContext::new(""));
     textinput.adjust_horizontal(2, Selection::NotSelected);
     textinput.delete_char(DeleteDir::Backward);
     assert_eq!(textinput.get_content(), "acdefg");
@@ -27,7 +35,7 @@ fn test_textinput_delete_char() {
 
 #[test]
 fn test_textinput_insert_char() {
-    let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), None);
+    let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), DummyClipboardContext::new(""));
     textinput.adjust_horizontal(2, Selection::NotSelected);
     textinput.insert_char('a');
     assert_eq!(textinput.get_content(), "abacdefg");
@@ -39,7 +47,7 @@ fn test_textinput_insert_char() {
 
 #[test]
 fn test_textinput_get_sorted_selection() {
-    let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), None);
+    let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), DummyClipboardContext::new(""));
     textinput.adjust_horizontal(2, Selection::NotSelected);
     textinput.adjust_horizontal(2, Selection::Selected);
     let (begin, end) = textinput.get_sorted_selection();
@@ -56,7 +64,7 @@ fn test_textinput_get_sorted_selection() {
 
 #[test]
 fn test_textinput_replace_selection() {
-    let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), None);
+    let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), DummyClipboardContext::new(""));
     textinput.adjust_horizontal(2, Selection::NotSelected);
     textinput.adjust_horizontal(2, Selection::Selected);
 
@@ -66,7 +74,7 @@ fn test_textinput_replace_selection() {
 
 #[test]
 fn test_textinput_current_line_length() {
-    let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), None);
+    let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
     assert_eq!(textinput.current_line_length(), 3);
 
     textinput.adjust_vertical(1, Selection::NotSelected);
@@ -78,7 +86,7 @@ fn test_textinput_current_line_length() {
 
 #[test]
 fn test_textinput_adjust_vertical() {
-    let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), None);
+    let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
     textinput.adjust_horizontal(3, Selection::NotSelected);
     textinput.adjust_vertical(1, Selection::NotSelected);
     assert_eq!(textinput.edit_point.line, 1);
@@ -95,7 +103,7 @@ fn test_textinput_adjust_vertical() {
 
 #[test]
 fn test_textinput_adjust_horizontal() {
-    let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), None);
+    let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
     textinput.adjust_horizontal(4, Selection::NotSelected);
     assert_eq!(textinput.edit_point.line, 1);
     assert_eq!(textinput.edit_point.index, 0);
@@ -115,12 +123,12 @@ fn test_textinput_adjust_horizontal() {
 
 #[test]
 fn test_textinput_handle_return() {
-    let mut single_line_textinput = TextInput::new(Lines::Single, "abcdef".to_owned(), None);
+    let mut single_line_textinput = TextInput::new(Lines::Single, "abcdef".to_owned(), DummyClipboardContext::new(""));
     single_line_textinput.adjust_horizontal(3, Selection::NotSelected);
     single_line_textinput.handle_return();
     assert_eq!(single_line_textinput.get_content(), "abcdef");
 
-    let mut multi_line_textinput = TextInput::new(Lines::Multiple, "abcdef".to_owned(), None);
+    let mut multi_line_textinput = TextInput::new(Lines::Multiple, "abcdef".to_owned(), DummyClipboardContext::new(""));
     multi_line_textinput.adjust_horizontal(3, Selection::NotSelected);
     multi_line_textinput.handle_return();
     assert_eq!(multi_line_textinput.get_content(), "abc\ndef");
@@ -128,7 +136,7 @@ fn test_textinput_handle_return() {
 
 #[test]
 fn test_textinput_select_all() {
-    let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), None);
+    let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
     assert_eq!(textinput.edit_point.line, 0);
     assert_eq!(textinput.edit_point.index, 0);
 
@@ -139,16 +147,16 @@ fn test_textinput_select_all() {
 
 #[test]
 fn test_textinput_get_content() {
-    let single_line_textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), None);
+    let single_line_textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), DummyClipboardContext::new(""));
     assert_eq!(single_line_textinput.get_content(), "abcdefg");
 
-    let multi_line_textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), None);
+    let multi_line_textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
     assert_eq!(multi_line_textinput.get_content(), "abc\nde\nf");
 }
 
 #[test]
 fn test_textinput_set_content() {
-    let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), None);
+    let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
     assert_eq!(textinput.get_content(), "abc\nde\nf");
 
     textinput.set_content("abc\nf".to_owned());
@@ -165,3 +173,16 @@ fn test_textinput_set_content() {
     assert_eq!(textinput.edit_point.index, 2);
 }
 
+#[test]
+fn test_clipboard_paste() {
+    #[cfg(target_os="macos")]
+    const MODIFIERS: KeyModifiers = SUPER;
+    #[cfg(not(target_os="macos"))]
+    const MODIFIERS: KeyModifiers = CONTROL;
+
+    let mut textinput = TextInput::new(Lines::Single, "defg".to_owned(), DummyClipboardContext::new("abc"));
+    assert_eq!(textinput.get_content(), "defg");
+    assert_eq!(textinput.edit_point.index, 0);
+    textinput.handle_keydown_aux(Key::V, MODIFIERS);
+    assert_eq!(textinput.get_content(), "abcdefg");
+}