aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorbors-servo <lbergstrom+bors@mozilla.com>2017-01-04 10:52:06 -0800
committerGitHub <noreply@github.com>2017-01-04 10:52:06 -0800
commit6f9ff7b8bf66cbeb7d539c6728db05f05aa8f85c (patch)
tree3b69d3f845bcdda83e27b7174370e8ff54b446d1
parent96fd0837d3de70b1f0d8f2bff0253b0220e7e5ce (diff)
parent5f0b3bd53c25c158117c91e9da36aaec0342244f (diff)
downloadservo-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.txt1
-rw-r--r--components/script/dom/document.rs58
-rw-r--r--components/script/dom/event.rs42
-rwxr-xr-xcomponents/script/dom/htmlinputelement.rs29
-rwxr-xr-xcomponents/script/dom/htmltextareaelement.rs25
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);
+ }
}
}
}