Make textinput handle actual key values. Don't restrict character values to a single byte.

components/constellation/constellation.rs

@@ -803,7 +803,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                 self.compositor_proxy.send(ToCompositorMsg::ChangePageTitle(pipeline_id, title))
             }
 
-            FromScriptMsg::SendKeyEvent(key, key_state, key_modifiers, ch) => {
+            FromScriptMsg::SendKeyEvent(ch, key, key_state, key_modifiers) => {
                 self.compositor_proxy.send(ToCompositorMsg::KeyEvent(ch, key, key_state, key_modifiers))
             }
 
@@ -1407,7 +1407,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
         match pipeline_id {
             Some(pipeline_id) => {
-                let event = CompositorEvent::KeyEvent(key, state, mods, ch);
+                let event = CompositorEvent::KeyEvent(ch, key, state, mods);
                 let msg = ConstellationControlMsg::SendEvent(pipeline_id, event);
                 let result = match self.pipelines.get(&pipeline_id) {
                     Some(pipeline) => pipeline.script_chan.send(msg),
@@ -1629,7 +1629,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                     None => return warn!("Pipeline {:?} SendKeys after closure.", pipeline_id),
                 };
                 for (key, mods, state) in cmd {
-                    let event = CompositorEvent::KeyEvent(key, state, mods, None);
+                    let event = CompositorEvent::KeyEvent(None, key, state, mods);
                     let control_msg = ConstellationControlMsg::SendEvent(pipeline_id, event);
                     if let Err(e) = script_channel.send(control_msg) {
                         return self.handle_send_error(pipeline_id, e);

components/script/dom/bindings/trace.rs

@@ -276,7 +276,7 @@ impl<A: JSTraceable, B: JSTraceable, C: JSTraceable> JSTraceable for (A, B, C) {
     }
 }
 
-no_jsmanaged_fields!(bool, f32, f64, String, Url, AtomicBool, AtomicUsize, UrlOrigin, Uuid);
+no_jsmanaged_fields!(bool, f32, f64, String, Url, AtomicBool, AtomicUsize, UrlOrigin, Uuid, char);
 no_jsmanaged_fields!(usize, u8, u16, u32, u64);
 no_jsmanaged_fields!(isize, i8, i16, i32, i64);
 no_jsmanaged_fields!(Sender<T>);

components/script/dom/document.rs

@@ -1125,7 +1125,7 @@ impl Document {
         }
 
         if !prevented {
-            constellation.send(ConstellationMsg::SendKeyEvent(key, state, modifiers, ch)).unwrap();
+            constellation.send(ConstellationMsg::SendKeyEvent(ch, key, state, modifiers)).unwrap();
         }
 
         // This behavior is unspecced

components/script/dom/keyboardevent.rs

@@ -37,6 +37,7 @@ pub struct KeyboardEvent {
     is_composing: Cell<bool>,
     char_code: Cell<Option<u32>>,
     key_code: Cell<u32>,
+    printable: Cell<Option<char>>,
 }
 
 impl KeyboardEvent {
@@ -55,6 +56,7 @@ impl KeyboardEvent {
             is_composing: Cell::new(false),
             char_code: Cell::new(None),
             key_code: Cell::new(0),
+            printable: Cell::new(None),
         }
     }
 
@@ -86,13 +88,14 @@ impl KeyboardEvent {
         let ev = KeyboardEvent::new_uninitialized(window);
         ev.InitKeyboardEvent(type_, canBubble, cancelable, view, key_string, location,
                              DOMString::new(), repeat, DOMString::new());
-        ev.key.set(logical_key(ch, key, location));
+        ev.key.set(key);
         *ev.code.borrow_mut() = code;
         ev.ctrl.set(ctrlKey);
         ev.alt.set(altKey);
         ev.shift.set(shiftKey);
         ev.meta.set(metaKey);
         ev.char_code.set(char_code);
+        ev.printable.set(ch);
         ev.key_code.set(key_code);
         ev.is_composing.set(isComposing);
         ev
@@ -129,6 +132,10 @@ impl KeyboardEvent {
 
 
 impl KeyboardEvent {
+    pub fn printable(&self) -> Option<char> {
+        self.printable.get()
+    }
+
     pub fn get_key(&self) -> Option<Key> {
         self.key.get().clone()
     }
@@ -151,18 +158,6 @@ impl KeyboardEvent {
     }
 }
 
-fn logical_key(ch: Option<char>, key: Option<Key>, location: u32) -> Option<Key> {
-    if let Some(ch) = ch {
-        let key = key_from_string(&format!("{}", ch), location);
-        if key.is_some() {
-            return key;
-        }
-    }
-
-    key
-}
-
-
 // https://w3c.github.io/uievents-key/#key-value-tables
 pub fn key_value(ch: Option<char>, key: Key, mods: KeyModifiers) -> Cow<'static, str> {
     if let Some(ch) = ch {

components/script/script_thread.rs

@@ -1937,7 +1937,7 @@ impl ScriptThread {
                 document.r().handle_touchpad_pressure_event(self.js_runtime.rt(), point, pressure, phase);
             }
 
-            KeyEvent(key, state, modifiers, ch) => {
+            KeyEvent(ch, key, state, modifiers) => {
                 let document = match self.root_browsing_context().find(pipeline_id) {
                     Some(browsing_context) => browsing_context.active_document(),
                     None => return warn!("Message sent to closed pipeline {}.", pipeline_id),

components/script/textinput.rs

@@ -6,10 +6,10 @@
 
 use clipboard_provider::ClipboardProvider;
 use dom::bindings::str::DOMString;
-use dom::keyboardevent::{KeyboardEvent, key_value};
+use dom::keyboardevent::KeyboardEvent;
 use msg::constellation_msg::{ALT, CONTROL, SHIFT, SUPER};
 use msg::constellation_msg::{Key, KeyModifiers};
-use std::borrow::{ToOwned, Borrow};
+use std::borrow::ToOwned;
 use std::cmp::{max, min};
 use std::default::Default;
 use std::ops::Range;
@@ -120,24 +120,6 @@ fn is_control_key(mods: KeyModifiers) -> bool {
     mods.contains(CONTROL) && !mods.contains(SUPER | ALT)
 }
 
-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,
-    }
-}
-
 /// The length in bytes of the first n characters in a UTF-8 string.
 ///
 /// If the string has fewer than n characters, returns the length of the whole string.
@@ -486,80 +468,80 @@ impl<T: ClipboardProvider> TextInput<T> {
     /// Process a given `KeyboardEvent` and return an action for the caller to execute.
     pub fn handle_keydown(&mut self, event: &KeyboardEvent) -> KeyReaction {
         if let Some(key) = event.get_key() {
-            self.handle_keydown_aux(key, event.get_key_modifiers())
+            self.handle_keydown_aux(event.printable(), key, event.get_key_modifiers())
         } else {
             KeyReaction::Nothing
         }
     }
-    pub fn handle_keydown_aux(&mut self, key: Key, mods: KeyModifiers) -> KeyReaction {
+
+    pub fn handle_keydown_aux(&mut self,
+                              printable: Option<char>,
+                              key: Key,
+                              mods: KeyModifiers) -> KeyReaction {
         let maybe_select = if mods.contains(SHIFT) { Selection::Selected } else { Selection::NotSelected };
-        match key {
+        match (printable, key) {
-            Key::A if is_control_key(mods) => {
+            (Some('a'), _) if is_control_key(mods) => {
                 self.select_all();
                 KeyReaction::RedrawSelection
             },
-            Key::C if is_control_key(mods) => {
+            (Some('c'), _) if is_control_key(mods) => {
                 if let Some(text) = self.get_selection_text() {
                     self.clipboard_provider.set_clipboard_contents(text);
                 }
                 KeyReaction::DispatchInput
             },
-            Key::V if is_control_key(mods) => {
+            (Some('v'), _) if is_control_key(mods) => {
                 let contents = self.clipboard_provider.clipboard_contents();
                 self.insert_string(contents);
                 KeyReaction::DispatchInput
             },
-            _ if is_printable_key(key) => {
+            (Some(c), _) => {
-                self.insert_string::<&str>(key_value(None, key, mods).borrow());
+                self.insert_char(c);
                 KeyReaction::DispatchInput
             }
-            Key::Space => {
+            (None, Key::Delete) => {
-                self.insert_char(' ');
-                KeyReaction::DispatchInput
-            }
-            Key::Delete => {
                 self.delete_char(Direction::Forward);
                 KeyReaction::DispatchInput
             }
-            Key::Backspace => {
+            (None, Key::Backspace) => {
                 self.delete_char(Direction::Backward);
                 KeyReaction::DispatchInput
             }
-            Key::Left => {
+            (None, Key::Left) => {
                 self.adjust_horizontal_by_one(Direction::Backward, maybe_select);
                 KeyReaction::RedrawSelection
             }
-            Key::Right => {
+            (None, Key::Right) => {
                 self.adjust_horizontal_by_one(Direction::Forward, maybe_select);
                 KeyReaction::RedrawSelection
             }
-            Key::Up => {
+            (None, Key::Up) => {
                 self.adjust_vertical(-1, maybe_select);
                 KeyReaction::RedrawSelection
             }
-            Key::Down => {
+            (None, Key::Down) => {
                 self.adjust_vertical(1, maybe_select);
                 KeyReaction::RedrawSelection
             }
-            Key::Enter | Key::KpEnter => self.handle_return(),
+            (None, Key::Enter) | (None, Key::KpEnter) => self.handle_return(),
-            Key::Home => {
+            (None, Key::Home) => {
                 self.edit_point.index = 0;
                 KeyReaction::RedrawSelection
             }
-            Key::End => {
+            (None, Key::End) => {
                 self.edit_point.index = self.current_line_length();
                 self.assert_ok_selection();
                 KeyReaction::RedrawSelection
             }
-            Key::PageUp => {
+            (None, Key::PageUp) => {
                 self.adjust_vertical(-28, maybe_select);
                 KeyReaction::RedrawSelection
             }
-            Key::PageDown => {
+            (None, Key::PageDown) => {
                 self.adjust_vertical(28, maybe_select);
                 KeyReaction::RedrawSelection
             }
-            Key::Tab => KeyReaction::TriggerDefaultAction,
+            (None, Key::Tab) => KeyReaction::TriggerDefaultAction,
             _ => KeyReaction::Nothing,
         }
     }

components/script_traits/lib.rs

@@ -282,7 +282,7 @@ pub enum CompositorEvent {
     /// Touchpad pressure event
     TouchpadPressureEvent(Point2D<f32>, f32, TouchpadPressurePhase),
     /// A key was pressed.
-    KeyEvent(Key, KeyState, KeyModifiers, Option<char>),
+    KeyEvent(Option<char>, Key, KeyState, KeyModifiers),
 }
 
 /// Touchpad pressure phase for TouchpadPressureEvent.

components/script_traits/script_msg.rs

@@ -103,7 +103,7 @@ pub enum ScriptMsg {
     /// https://html.spec.whatwg.org/multipage/#document.title
     SetTitle(PipelineId, Option<String>),
     /// Send a key event
-    SendKeyEvent(Key, KeyState, KeyModifiers, Option<char>),
+    SendKeyEvent(Option<char>, Key, KeyState, KeyModifiers),
     /// Get Window Informations size and position
     GetClientWindow(IpcSender<(Size2D<u32>, Point2D<i32>)>),
     /// Move the window to a point

ports/cef/browser_host.rs

@@ -17,6 +17,7 @@ use libc::{c_double, c_int};
 use msg::constellation_msg::{self, KeyModifiers, KeyState};
 use script_traits::{MouseButton, TouchEventType};
 use std::cell::{Cell, RefCell};
+use std::char;
 
 pub struct ServoCefBrowserHost {
     /// A reference to the browser.
@@ -424,7 +425,8 @@ full_cef_class_impl! {
             if (*event).modifiers & EVENTFLAG_ALT_DOWN as u32 != 0 {
                key_modifiers = key_modifiers | constellation_msg::ALT;
             }
-            this.downcast().send_window_event(WindowEvent::KeyEvent(key, key_state, key_modifiers))
+            let ch = char::from_u32((*event).character as u32);
+            this.downcast().send_window_event(WindowEvent::KeyEvent(ch, key, key_state, key_modifiers))
         }}
 
         fn send_mouse_click_event(&this,

ports/cef/window.rs

@@ -479,7 +479,7 @@ impl WindowMethods for Window {
         }
     }
 
-    fn handle_key(&self, _: Key, _: KeyModifiers) {
+    fn handle_key(&self, _: Option<char>, _: Key, _: KeyModifiers) {
         // TODO(negge)
     }
 

ports/glutin/window.rs

@@ -242,9 +242,8 @@ impl Window {
     fn handle_window_event(&self, event: glutin::Event) -> bool {
         match event {
             Event::ReceivedCharacter(ch) => {
-                // Glutin ends up providing non-printable characters like escape and backspace,
+                assert!(self.pending_key_event_char.get().is_none());
-                // which is no good for trying to figure out the logical key that was pressed.
+                if !ch.is_control() {
-                if !ch.is_control() && ch.len_utf8() == 1 {
                     self.pending_key_event_char.set(Some(ch));
                 }
             }
@@ -266,7 +265,9 @@ impl Window {
                         // Retrieve any previosly stored ReceivedCharacter value.
                         // Store the association between the scan code and the actual
                         // character value, if there is one.
-                        let ch = self.pending_key_event_char.get();
+                        let ch = self.pending_key_event_char
+                                     .get()
+                                     .and_then(|ch| filter_nonprintable(ch, virtual_key_code));
                         self.pending_key_event_char.set(None);
                         if let Some(ch) = ch {
                             self.pressed_key_map.borrow_mut().push((scan_code, ch));
@@ -962,6 +963,77 @@ fn glutin_pressure_stage_to_touchpad_pressure_phase(stage: i64) -> TouchpadPress
     }
 }
 
+fn filter_nonprintable(ch: char, key_code: VirtualKeyCode) -> Option<char> {
+    use glutin::VirtualKeyCode::*;
+    match key_code {
+        Escape |
+        F1 |
+        F2 |
+        F3 |
+        F4 |
+        F5 |
+        F6 |
+        F7 |
+        F8 |
+        F9 |
+        F10 |
+        F11 |
+        F12 |
+        F13 |
+        F14 |
+        F15 |
+        Snapshot |
+        Scroll |
+        Pause |
+        Insert |
+        Home |
+        Delete |
+        End |
+        PageDown |
+        PageUp |
+        Left |
+        Up |
+        Right |
+        Down |
+        Back |
+        LAlt |
+        LControl |
+        LMenu |
+        LShift |
+        LWin |
+        Mail |
+        MediaSelect |
+        MediaStop |
+        Mute |
+        MyComputer |
+        NavigateForward |
+        NavigateBackward |
+        NextTrack |
+        NoConvert |
+        PlayPause |
+        Power |
+        PrevTrack |
+        RAlt |
+        RControl |
+        RMenu |
+        RShift |
+        RWin |
+        Sleep |
+        Stop |
+        VolumeDown |
+        VolumeUp |
+        Wake |
+        WebBack |
+        WebFavorites |
+        WebForward |
+        WebHome |
+        WebRefresh |
+        WebSearch |
+        WebStop => None,
+        _ => Some(ch),
+    }
+}
+
 // These functions aren't actually called. They are here as a link
 // hack because Skia references them.
 

tests/unit/script/textinput.rs

@@ -401,7 +401,7 @@ fn test_clipboard_paste() {
                                        SelectionDirection::None);
     assert_eq!(textinput.get_content(), "defg");
     assert_eq!(textinput.edit_point.index, 0);
-    textinput.handle_keydown_aux(Key::V, MODIFIERS);
+    textinput.handle_keydown_aux(Some('v'), Key::V, MODIFIERS);
     assert_eq!(textinput.get_content(), "abcdefg");
 }