Minor keyboard updates

Support combined character input from winit.
Make use of utility methods from keyboard-types.
Remove prinatable attribute of KeyboardEvent.

ports/servo/Cargo.toml

@@ -40,7 +40,7 @@ crossbeam-channel = "0.2"
 euclid = "0.19"
 gleam = "0.6"
 glutin = "0.18"
-keyboard-types = {version = "0.4.0-serde", features = ["serde"]}
+keyboard-types = {version = "0.4.2-servo", features = ["serde"]}
 lazy_static = "1"
 libservo = {path = "../../components/servo"}
 log = "0.4"

ports/servo/glutin_app/keyutils.rs

@@ -2,8 +2,8 @@
  * 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 keyboard_types::{Code, Key, Modifiers, Location};
-use winit::VirtualKeyCode;
+use keyboard_types::{Code, Key, KeyboardEvent, KeyState, Modifiers, Location};
+use winit::{ElementState, KeyboardInput, ModifiersState, VirtualKeyCode};
 
 // Some shortcuts use Cmd on Mac and Control on other systems.
 #[cfg(target_os = "macos")]
@@ -17,7 +17,7 @@ pub const CMD_OR_ALT: Modifiers = Modifiers::META;
 #[cfg(not(target_os = "macos"))]
 pub const CMD_OR_ALT: Modifiers = Modifiers::ALT;
 
-pub fn get_servo_key_from_winit_key(key: Option<VirtualKeyCode>) -> Key {
+fn get_servo_key_from_winit_key(key: Option<VirtualKeyCode>) -> Key {
     use winit::VirtualKeyCode::*;
     // TODO: figure out how to map NavigateForward, NavigateBackward
     // TODO: map the remaining keys if possible
@@ -126,7 +126,7 @@ pub fn get_servo_key_from_winit_key(key: Option<VirtualKeyCode>) -> Key {
     }
 }
 
-pub fn get_servo_location_from_winit_key(key: Option<VirtualKeyCode>) -> Location {
+fn get_servo_location_from_winit_key(key: Option<VirtualKeyCode>) -> Location {
     use winit::VirtualKeyCode::*;
     // TODO: add more numpad keys
     let key = if let Some(key) = key {
@@ -145,7 +145,7 @@ pub fn get_servo_location_from_winit_key(key: Option<VirtualKeyCode>) -> Locatio
 }
 
 #[cfg(target_os = "linux")]
-pub fn get_servo_code_from_scancode(scancode: u32) -> Code {
+fn get_servo_code_from_scancode(scancode: u32) -> Code {
     // TODO: Map more codes
     use keyboard_types::Code::*;
     match scancode {
@@ -236,7 +236,32 @@ pub fn get_servo_code_from_scancode(scancode: u32) -> Code {
 }
 
 #[cfg(not(target_os = "linux"))]
-pub fn get_servo_code_from_scancode(_scancode: u32) -> Code {
+fn get_servo_code_from_scancode(_scancode: u32) -> Code {
     // TODO: Implement for Windows and Mac OS
     Code::Unidentified
 }
+
+fn get_modifiers(mods: ModifiersState) -> Modifiers {
+    let mut modifiers = Modifiers::empty();
+    modifiers.set(Modifiers::CONTROL, mods.ctrl);
+    modifiers.set(Modifiers::SHIFT, mods.shift);
+    modifiers.set(Modifiers::ALT, mods.alt);
+    modifiers.set(Modifiers::META, mods.logo);
+    modifiers
+}
+
+pub fn keyboard_event_from_winit(input: KeyboardInput) -> KeyboardEvent {
+    info!("winit keyboard input: {:?}", input);
+    KeyboardEvent {
+        state: match input.state {
+            ElementState::Pressed => KeyState::Down,
+            ElementState::Released => KeyState::Up,
+        },
+        key: get_servo_key_from_winit_key(input.virtual_keycode),
+        code: get_servo_code_from_scancode(input.scancode),
+        location: get_servo_location_from_winit_key(input.virtual_keycode),
+        modifiers: get_modifiers(input.modifiers),
+        repeat: false,
+        is_composing: false,
+    }
+}

ports/servo/glutin_app/window.rs

@@ -9,7 +9,7 @@ use euclid::{Length, TypedPoint2D, TypedVector2D, TypedScale, TypedSize2D};
 use gdi32;
 use gleam::gl;
 use glutin::{Api, ContextBuilder, GlContext, GlRequest, GlWindow};
-use keyboard_types::{Key, KeyboardEvent, Modifiers as KeyModifiers, KeyState};
+use keyboard_types::{Key, KeyboardEvent, KeyState};
 #[cfg(any(target_os = "linux", target_os = "macos"))]
 use osmesa_sys;
 use servo::compositing::windowing::{AnimationState, MouseWindowEvent, WindowEvent};
@@ -31,13 +31,13 @@ use std::rc::Rc;
 use std::sync::Arc;
 use std::thread;
 use std::time;
-use super::keyutils;
+use super::keyutils::keyboard_event_from_winit;
 #[cfg(target_os = "windows")]
 use user32;
 #[cfg(target_os = "windows")]
 use winapi;
 use winit;
-use winit::{ElementState, Event, ModifiersState, MouseButton, MouseScrollDelta, TouchPhase, VirtualKeyCode};
+use winit::{ElementState, Event, MouseButton, MouseScrollDelta, TouchPhase, KeyboardInput};
 use winit::dpi::{LogicalPosition, LogicalSize, PhysicalSize};
 #[cfg(target_os = "macos")]
 use winit::os::macos::{ActivationPolicy, WindowBuilderExt};
@@ -150,7 +150,6 @@ pub struct Window {
     mouse_down_point: Cell<TypedPoint2D<i32, DevicePixel>>,
     event_queue: RefCell<Vec<WindowEvent>>,
     mouse_pos: Cell<TypedPoint2D<i32, DevicePixel>>,
-    key_modifiers: Cell<KeyModifiers>,
     last_pressed: Cell<Option<KeyboardEvent>>,
     animation_state: Cell<AnimationState>,
     fullscreen: Cell<bool>,
@@ -276,10 +275,7 @@ impl Window {
             event_queue: RefCell::new(vec![]),
             mouse_down_button: Cell::new(None),
             mouse_down_point: Cell::new(TypedPoint2D::new(0, 0)),
-
             mouse_pos: Cell::new(TypedPoint2D::new(0, 0)),
-            key_modifiers: Cell::new(KeyModifiers::empty()),
-
             last_pressed: Cell::new(None),
             gl: gl.clone(),
             animation_state: Cell::new(AnimationState::Idle),
@@ -419,8 +415,15 @@ impl Window {
         }
         let mut event = if let Some(event) = self.last_pressed.replace(None) {
             event
+        } else if ch.is_ascii() {
+            // Some keys like Backspace emit a control character in winit
+            // but they are already dealt with in handle_keyboard_input
+            // so just ignore the character.
+            return
         } else {
-            return;
+            // For combined characters like the letter e with an acute accent
+            // no keyboard event is emitted. A dummy event is created in this case.
+            KeyboardEvent::default()
         };
         event.key = Key::Character(ch.to_string());
         self.event_queue
@@ -428,44 +431,15 @@ impl Window {
             .push(WindowEvent::Keyboard(event));
     }
 
-    fn toggle_keyboard_modifiers(&self, mods: ModifiersState) {
-        self.toggle_modifier(KeyModifiers::CONTROL, mods.ctrl);
-        self.toggle_modifier(KeyModifiers::SHIFT, mods.shift);
-        self.toggle_modifier(KeyModifiers::ALT, mods.alt);
-        self.toggle_modifier(KeyModifiers::META, mods.logo);
-    }
-
     fn handle_keyboard_input(
         &self,
-        scancode: u32,
-        element_state: ElementState,
-        code: Option<VirtualKeyCode>,
-        mods: ModifiersState,
+        input: KeyboardInput,
     ) {
-        info!(
-            "winit keyboard input: {}, {:?}, {:?}, {:?}",
-            scancode, element_state, code, mods
-        );
-        self.toggle_keyboard_modifiers(mods);
-        let key = keyutils::get_servo_key_from_winit_key(code);
-        let location = keyutils::get_servo_location_from_winit_key(code);
-        let state = match element_state {
-            ElementState::Pressed => KeyState::Down,
-            ElementState::Released => KeyState::Up,
-        };
-        let event = KeyboardEvent {
-            state,
-            key: key.clone(),
-            code: keyutils::get_servo_code_from_scancode(scancode),
-            location,
-            modifiers: self.key_modifiers.get(),
-            repeat: false,
-            is_composing: false,
-        };
-        if element_state == ElementState::Pressed && key == Key::Unidentified {
+        let event = keyboard_event_from_winit(input);
+        if event.state == KeyState::Down && event.key == Key::Unidentified {
             // If pressed and probably printable, we expect a ReceivedCharacter event.
             self.last_pressed.set(Some(event));
-        } else if key != Key::Unidentified {
+        } else if event.key != Key::Unidentified {
             self.last_pressed.set(None);
             self.event_queue
                 .borrow_mut()
@@ -482,17 +456,11 @@ impl Window {
             Event::WindowEvent {
                 event:
                     winit::WindowEvent::KeyboardInput {
-                        input:
-                            winit::KeyboardInput {
-                                scancode,
-                                state,
-                                virtual_keycode,
-                                modifiers,
-                            },
+                        input,
                         ..
                     },
                 ..
-            } => self.handle_keyboard_input(scancode, state, virtual_keycode, modifiers),
+            } => self.handle_keyboard_input(input),
             Event::WindowEvent {
                 event: winit::WindowEvent::MouseInput { state, button, .. },
                 ..
@@ -596,16 +564,6 @@ impl Window {
         }
     }
 
-    fn toggle_modifier(&self, modifier: KeyModifiers, pressed: bool) {
-        let mut modifiers = self.key_modifiers.get();
-        if pressed {
-            modifiers.insert(modifier);
-        } else {
-            modifiers.remove(modifier);
-        }
-        self.key_modifiers.set(modifiers);
-    }
-
     /// Helper function to handle a click
     fn handle_mouse(
         &self,