diff options
author | bors-servo <lbergstrom+bors@mozilla.com> | 2017-01-04 10:52:06 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-01-04 10:52:06 -0800 |
commit | 6f9ff7b8bf66cbeb7d539c6728db05f05aa8f85c (patch) | |
tree | 3b69d3f845bcdda83e27b7174370e8ff54b446d1 | |
parent | 96fd0837d3de70b1f0d8f2bff0253b0220e7e5ce (diff) | |
parent | 5f0b3bd53c25c158117c91e9da36aaec0342244f (diff) | |
download | servo-6f9ff7b8bf66cbeb7d539c6728db05f05aa8f85c.tar.gz servo-6f9ff7b8bf66cbeb7d539c6728db05f05aa8f85c.zip |
Auto merge of #14738 - Wafflespeanut:keypress, r=jdm
Properly dispatch keypress event
<!-- Please describe your changes on the following line: -->
This was an attempt to fix #14659. It turned out that the problem wasn't what I thought it was. So, I didn't fix that. On the brighter side, this fixes two related issues.
- Previously, we were unable to launch `keypress` events from `input` and `textarea` elements, because [we'd been cancelling](https://github.com/servo/servo/blob/1327ebd52f53f5f6637a12fab6cf0cad0aa0be6f/components/script/dom/htmlinputelement.rs#L1120-L1124) the key events, so that they don't trigger window navigation - #8400). I've introduced an enum to represent an additional state to an event's cancellation.
- [According to the spec](https://w3c.github.io/uievents/#keypress-event-order), `keypress` (if available) should be dispatched immediately after `keydown`, and it should be followed by `input`. Canceling `keypress` should also cancel `input`. But, we'd been dispatching `input` before `keypress`. We now dispatch `input` once the `keypress` event is on the respective elements.
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
<!-- Either: -->
- [x] These changes do not require tests because it's a refactor?
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
r? @jdm or anyone interested
<!-- 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/14738)
<!-- Reviewable:end -->
-rw-r--r-- | components/atoms/static_atoms.txt | 1 | ||||
-rw-r--r-- | components/script/dom/document.rs | 58 | ||||
-rw-r--r-- | components/script/dom/event.rs | 42 | ||||
-rwxr-xr-x | components/script/dom/htmlinputelement.rs | 29 | ||||
-rwxr-xr-x | components/script/dom/htmltextareaelement.rs | 25 |
5 files changed, 93 insertions, 62 deletions
diff --git a/components/atoms/static_atoms.txt b/components/atoms/static_atoms.txt index e59c9cad809..c2a4ea84ec2 100644 --- a/components/atoms/static_atoms.txt +++ b/components/atoms/static_atoms.txt @@ -52,6 +52,7 @@ beforeunload message click keydown +keypress abort beforescriptexecute afterscriptexecute diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 79acb69096d..83dc980716c 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -13,7 +13,6 @@ use dom::bindings::codegen::Bindings::DOMRectBinding::DOMRectMethods; use dom::bindings::codegen::Bindings::DocumentBinding; use dom::bindings::codegen::Bindings::DocumentBinding::{DocumentMethods, DocumentReadyState}; use dom::bindings::codegen::Bindings::ElementBinding::ElementMethods; -use dom::bindings::codegen::Bindings::EventBinding::EventMethods; use dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull; use dom::bindings::codegen::Bindings::EventHandlerBinding::OnErrorEventHandlerNonNull; use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods; @@ -41,7 +40,7 @@ use dom::documenttype::DocumentType; use dom::domimplementation::DOMImplementation; use dom::element::{Element, ElementCreator, ElementPerformFullscreenEnter, ElementPerformFullscreenExit}; use dom::errorevent::ErrorEvent; -use dom::event::{Event, EventBubbles, EventCancelable}; +use dom::event::{Event, EventBubbles, EventCancelable, EventDefault}; use dom::eventdispatcher::EventStatus; use dom::eventtarget::EventTarget; use dom::focusevent::FocusEvent; @@ -1308,10 +1307,10 @@ impl Document { props.key_code); let event = keyevent.upcast::<Event>(); event.fire(target); - let mut prevented = event.DefaultPrevented(); + let mut cancel_state = event.get_cancel_state(); // https://w3c.github.io/uievents/#keys-cancelable-keys - if state != KeyState::Released && props.is_printable() && !prevented { + if state != KeyState::Released && props.is_printable() && cancel_state != EventDefault::Prevented { // https://w3c.github.io/uievents/#keypress-event-order let event = KeyboardEvent::new(&self.window, DOMString::from("keypress"), @@ -1334,40 +1333,39 @@ impl Document { 0); let ev = event.upcast::<Event>(); ev.fire(target); - prevented = ev.DefaultPrevented(); - // TODO: if keypress event is canceled, prevent firing input events + cancel_state = ev.get_cancel_state(); } - if !prevented { + if cancel_state == EventDefault::Allowed { constellation.send(ConstellationMsg::SendKeyEvent(ch, key, state, modifiers)).unwrap(); - } - // This behavior is unspecced - // We are supposed to dispatch synthetic click activation for Space and/or Return, - // however *when* we do it is up to us - // I'm dispatching it after the key event so the script has a chance to cancel it - // https://www.w3.org/Bugs/Public/show_bug.cgi?id=27337 - match key { - Key::Space if !prevented && state == KeyState::Released => { - let maybe_elem = target.downcast::<Element>(); - if let Some(el) = maybe_elem { - synthetic_click_activation(el, - false, - false, - false, - false, - ActivationSource::NotFromClick) + // This behavior is unspecced + // We are supposed to dispatch synthetic click activation for Space and/or Return, + // however *when* we do it is up to us. + // Here, we're dispatching it after the key event so the script has a chance to cancel it + // https://www.w3.org/Bugs/Public/show_bug.cgi?id=27337 + match key { + Key::Space if state == KeyState::Released => { + let maybe_elem = target.downcast::<Element>(); + if let Some(el) = maybe_elem { + synthetic_click_activation(el, + false, + false, + false, + false, + ActivationSource::NotFromClick) + } } - } - Key::Enter if !prevented && state == KeyState::Released => { - let maybe_elem = target.downcast::<Element>(); - if let Some(el) = maybe_elem { - if let Some(a) = el.as_maybe_activatable() { - a.implicit_submission(ctrl, alt, shift, meta); + Key::Enter if state == KeyState::Released => { + let maybe_elem = target.downcast::<Element>(); + if let Some(el) = maybe_elem { + if let Some(a) = el.as_maybe_activatable() { + a.implicit_submission(ctrl, alt, shift, meta); + } } } + _ => (), } - _ => (), } self.window.reflow(ReflowGoal::ForDisplay, diff --git a/components/script/dom/event.rs b/components/script/dom/event.rs index d471c25d873..fa3b9bfae6b 100644 --- a/components/script/dom/event.rs +++ b/components/script/dom/event.rs @@ -77,6 +77,28 @@ impl From<bool> for EventCancelable { } } +/// An enum to indicate whether the default action of an event is allowed. +/// +/// This should've been a bool. Instead, it's an enum, because, aside from the allowed/canceled +/// states, we also need something to stop the event from being handled again (without cancelling +/// the event entirely). For example, an Up/Down `KeyEvent` inside a `textarea` element will +/// trigger the cursor to go up/down if the text inside the element spans multiple lines. This enum +/// helps us to prevent such events from being [sent to the constellation][msg] where it will be +/// handled once again for page scrolling (which is definitely not what we'd want). +/// +/// [msg]: https://doc.servo.org/script_traits/enum.ConstellationMsg.html#variant.KeyEvent +/// +#[derive(JSTraceable, HeapSizeOf, Copy, Clone, PartialEq)] +pub enum EventDefault { + /// The default action of the event is allowed (constructor's default) + Allowed, + /// The default action has been prevented by calling `PreventDefault` + Prevented, + /// The event has been handled somewhere in the DOM, and it should be prevented from being + /// re-handled elsewhere. This doesn't affect the judgement of `DefaultPrevented` + Handled, +} + #[dom_struct] pub struct Event { reflector_: Reflector, @@ -84,7 +106,7 @@ pub struct Event { target: MutNullableJS<EventTarget>, type_: DOMRefCell<Atom>, phase: Cell<EventPhase>, - canceled: Cell<bool>, + canceled: Cell<EventDefault>, stop_propagation: Cell<bool>, stop_immediate: Cell<bool>, cancelable: Cell<bool>, @@ -103,7 +125,7 @@ impl Event { target: Default::default(), type_: DOMRefCell::new(atom!("")), phase: Cell::new(EventPhase::None), - canceled: Cell::new(false), + canceled: Cell::new(EventDefault::Allowed), stop_propagation: Cell::new(false), stop_immediate: Cell::new(false), cancelable: Cell::new(false), @@ -146,7 +168,7 @@ impl Event { self.initialized.set(true); self.stop_propagation.set(false); self.stop_immediate.set(false); - self.canceled.set(false); + self.canceled.set(EventDefault::Allowed); self.trusted.set(false); self.target.set(None); *self.type_.borrow_mut() = type_; @@ -229,6 +251,16 @@ impl Event { pub fn type_(&self) -> Atom { self.type_.borrow().clone() } + + #[inline] + pub fn mark_as_handled(&self) { + self.canceled.set(EventDefault::Handled); + } + + #[inline] + pub fn get_cancel_state(&self) -> EventDefault { + self.canceled.get() + } } impl EventMethods for Event { @@ -254,13 +286,13 @@ impl EventMethods for Event { // https://dom.spec.whatwg.org/#dom-event-defaultprevented fn DefaultPrevented(&self) -> bool { - self.canceled.get() + self.canceled.get() == EventDefault::Prevented } // https://dom.spec.whatwg.org/#dom-event-preventdefault fn PreventDefault(&self) { if self.cancelable.get() { - self.canceled.set(true) + self.canceled.set(EventDefault::Prevented) } } diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index eb344ef0a79..d133efd5e14 100755 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -1105,28 +1105,29 @@ impl VirtualMethods for HTMLInputElement { DispatchInput => { self.value_changed.set(true); self.update_placeholder_shown_state(); - - if event.IsTrusted() { - let window = window_from_node(self); - let _ = window.user_interaction_task_source().queue_event( - &self.upcast(), - atom!("input"), - EventBubbles::Bubbles, - EventCancelable::NotCancelable, - &window); - } - self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage); - event.PreventDefault(); + event.mark_as_handled(); } RedrawSelection => { self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage); - event.PreventDefault(); + event.mark_as_handled(); } Nothing => (), } } - } + } else if event.type_() == atom!("keypress") && !event.DefaultPrevented() && + (self.input_type.get() == InputType::InputText || + self.input_type.get() == InputType::InputPassword) { + if event.IsTrusted() { + let window = window_from_node(self); + let _ = window.user_interaction_task_source() + .queue_event(&self.upcast(), + atom!("input"), + EventBubbles::Bubbles, + EventCancelable::NotCancelable, + &window); + } + } } } diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index fd5b42fa25c..2582b13514a 100755 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -403,27 +403,26 @@ impl VirtualMethods for HTMLTextAreaElement { KeyReaction::DispatchInput => { self.value_changed.set(true); self.update_placeholder_shown_state(); - - if event.IsTrusted() { - let window = window_from_node(self); - let _ = window.user_interaction_task_source().queue_event( - &self.upcast(), - atom!("input"), - EventBubbles::Bubbles, - EventCancelable::NotCancelable, - &window); - } - self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage); - event.PreventDefault(); + event.mark_as_handled(); } KeyReaction::RedrawSelection => { self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage); - event.PreventDefault(); + event.mark_as_handled(); } KeyReaction::Nothing => (), } } + } else if event.type_() == atom!("keypress") && !event.DefaultPrevented() { + if event.IsTrusted() { + let window = window_from_node(self); + let _ = window.user_interaction_task_source() + .queue_event(&self.upcast(), + atom!("input"), + EventBubbles::Bubbles, + EventCancelable::NotCancelable, + &window); + } } } } |