Auto merge of #25488 - pshaughn:clickactivate, r=jdm

Event dispatch rewritten to align to spec, activate on clicks better

I went over the changes to the event dispatch spec that had accumulated over the past few years, rewriting dispatch/invoke/inner-invoke almost completely and modifying other code where it was relevant. Most of the remaining obvious deviations from spec are things that will only come up when we start handling events in shadow DOM.

I am pushing now because I want to see CI test results, but please do not approve this PR just if automated test improvements look good. I may have broken some actual UI interactions in the course of fixing synthetic events, and some manual testing is needed, including checking that manual interactions with interactive content continue to fire the events they're supposed to.

---
<!-- 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
- [X] These changes fix #25384 and fix #22783 and fix #25199

<!-- Either: -->
- [ ] There are automated tests for the synthetic-click parts of these changes, BUT the effects on real UI events need some manual testing before merging

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
This commit is contained in:
bors-servo 2020-02-13 17:37:12 -05:00 committed by GitHub
commit e697e6cca7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
29 changed files with 466 additions and 556 deletions

View file

@ -2,7 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
use crate::dom::activation::{synthetic_click_activation, Activatable, ActivationSource};
use crate::dom::activation::Activatable;
use crate::dom::attr::Attr;
use crate::dom::bindings::cell::DomRefCell;
use crate::dom::bindings::codegen::Bindings::ElementBinding::ElementMethods;
@ -11,12 +11,11 @@ use crate::dom::bindings::codegen::Bindings::FileListBinding::FileListMethods;
use crate::dom::bindings::codegen::Bindings::HTMLFormElementBinding::SelectionMode;
use crate::dom::bindings::codegen::Bindings::HTMLInputElementBinding;
use crate::dom::bindings::codegen::Bindings::HTMLInputElementBinding::HTMLInputElementMethods;
use crate::dom::bindings::codegen::Bindings::KeyboardEventBinding::KeyboardEventMethods;
use crate::dom::bindings::codegen::Bindings::NodeBinding::{GetRootNodeOptions, NodeMethods};
use crate::dom::bindings::error::{Error, ErrorResult};
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::reflector::DomObject;
use crate::dom::bindings::root::{Dom, DomRoot, LayoutDom, MutNullableDom};
use crate::dom::bindings::root::{DomRoot, LayoutDom, MutNullableDom};
use crate::dom::bindings::str::{DOMString, USVString};
use crate::dom::compositionevent::CompositionEvent;
use crate::dom::document::Document;
@ -246,7 +245,6 @@ pub struct HTMLInputElement {
minlength: Cell<i32>,
#[ignore_malloc_size_of = "#7193"]
textinput: DomRefCell<TextInput<ScriptToConstellationChan>>,
activation_state: DomRefCell<InputActivationState>,
// https://html.spec.whatwg.org/multipage/#concept-input-value-dirty-flag
value_dirty: Cell<bool>,
// not specified explicitly, but implied by the fact that sanitization can't
@ -260,30 +258,13 @@ pub struct HTMLInputElement {
}
#[derive(JSTraceable)]
#[unrooted_must_root_lint::must_root]
#[derive(MallocSizeOf)]
struct InputActivationState {
pub struct InputActivationState {
indeterminate: bool,
checked: bool,
checked_changed: bool,
checked_radio: Option<Dom<HTMLInputElement>>,
// In case mutability changed
was_mutable: bool,
checked_radio: Option<DomRoot<HTMLInputElement>>,
// In case the type changed
old_type: InputType,
}
impl InputActivationState {
fn new() -> InputActivationState {
InputActivationState {
indeterminate: false,
checked: false,
checked_changed: false,
checked_radio: None,
was_mutable: false,
old_type: Default::default(),
}
}
// was_mutable is implied: pre-activation would return None if it wasn't
}
static DEFAULT_INPUT_SIZE: u32 = 20;
@ -323,7 +304,6 @@ impl HTMLInputElement {
None,
SelectionDirection::None,
)),
activation_state: DomRefCell::new(InputActivationState::new()),
value_dirty: Cell::new(false),
sanitization_flag: Cell::new(true),
filelist: MutNullableDom::new(None),
@ -1756,7 +1736,7 @@ impl HTMLInputElement {
// https://html.spec.whatwg.org/multipage/#implicit-submission
#[allow(unsafe_code)]
fn implicit_submission(&self, ctrl_key: bool, shift_key: bool, alt_key: bool, meta_key: bool) {
fn implicit_submission(&self) {
let doc = document_from_node(self);
let node = doc.upcast::<Node>();
let owner = self.form_owner();
@ -1777,14 +1757,11 @@ impl HTMLInputElement {
match submit_button {
Some(ref button) => {
if button.is_instance_activatable() {
synthetic_click_activation(
button.as_element(),
ctrl_key,
shift_key,
alt_key,
meta_key,
ActivationSource::NotFromClick,
)
// spec does not actually say to set the not trusted flag,
// but we can get here from synthetic keydown events
button
.upcast::<Node>()
.fire_synthetic_mouse_event_not_trusted(DOMString::from("click"));
}
},
None => {
@ -2199,14 +2176,19 @@ impl VirtualMethods for HTMLInputElement {
}
}
// This represents behavior for which the UIEvents spec and the
// DOM/HTML specs are out of sync.
// Compare:
// https://w3c.github.io/uievents/#default-action
// https://dom.spec.whatwg.org/#action-versus-occurance
fn handle_event(&self, event: &Event) {
if let Some(s) = self.super_type() {
s.handle_event(event);
}
if event.type_() == atom!("click") && !event.DefaultPrevented() {
// TODO: Dispatch events for non activatable inputs
// https://html.spec.whatwg.org/multipage/#common-input-element-events
// WHATWG-specified activation behaviors are handled elsewhere;
// this is for all the other things a UI click might do
//TODO: set the editing position for text inputs
@ -2242,12 +2224,7 @@ impl VirtualMethods for HTMLInputElement {
let action = self.textinput.borrow_mut().handle_keydown(keyevent);
match action {
TriggerDefaultAction => {
self.implicit_submission(
keyevent.CtrlKey(),
keyevent.ShiftKey(),
keyevent.AltKey(),
keyevent.MetaKey(),
);
self.implicit_submission();
},
DispatchInput => {
self.value_dirty.set(true);
@ -2365,91 +2342,90 @@ impl Activatable for HTMLInputElement {
}
}
// https://html.spec.whatwg.org/multipage/#run-pre-click-activation-steps
#[allow(unsafe_code)]
fn pre_click_activation(&self) {
let mut cache = self.activation_state.borrow_mut();
let ty = self.input_type();
cache.old_type = ty;
cache.was_mutable = self.is_mutable();
if cache.was_mutable {
match ty {
// https://html.spec.whatwg.org/multipage/#submit-button-state-(type=submit):activation-behavior
// InputType::Submit => (), // No behavior defined
// https://html.spec.whatwg.org/multipage/#reset-button-state-(type=reset):activation-behavior
// InputType::Submit => (), // No behavior defined
InputType::Checkbox => {
/*
https://html.spec.whatwg.org/multipage/#checkbox-state-(type=checkbox):pre-click-activation-steps
cache current values of `checked` and `indeterminate`
we may need to restore them later
*/
cache.indeterminate = self.Indeterminate();
cache.checked = self.Checked();
cache.checked_changed = self.checked_changed.get();
self.SetIndeterminate(false);
self.SetChecked(!cache.checked);
},
// https://html.spec.whatwg.org/multipage/#radio-button-state-(type=radio):pre-click-activation-steps
InputType::Radio => {
let checked_member = radio_group_iter(self, self.radio_group_name().as_ref())
.find(|r| r.Checked());
cache.checked_radio = checked_member.as_deref().map(Dom::from_ref);
cache.checked_changed = self.checked_changed.get();
self.SetChecked(true);
},
_ => (),
}
// https://dom.spec.whatwg.org/#eventtarget-legacy-pre-activation-behavior
fn legacy_pre_activation_behavior(&self) -> Option<InputActivationState> {
if !self.is_mutable() {
return None;
}
let ty = self.input_type();
match ty {
InputType::Checkbox => {
let was_checked = self.Checked();
let was_indeterminate = self.Indeterminate();
self.SetIndeterminate(false);
self.SetChecked(!was_checked);
return Some(InputActivationState {
checked: was_checked,
indeterminate: was_indeterminate,
checked_radio: None,
old_type: InputType::Checkbox,
});
},
InputType::Radio => {
let checked_member =
radio_group_iter(self, self.radio_group_name().as_ref()).find(|r| r.Checked());
let was_checked = self.Checked();
self.SetChecked(true);
return Some(InputActivationState {
checked: was_checked,
indeterminate: false,
checked_radio: checked_member.as_deref().map(DomRoot::from_ref),
old_type: InputType::Radio,
});
},
_ => (),
}
return None;
}
// https://html.spec.whatwg.org/multipage/#run-canceled-activation-steps
fn canceled_activation(&self) {
let cache = self.activation_state.borrow();
let ty = self.input_type();
if cache.old_type != ty {
// Type changed, abandon ship
// https://www.w3.org/Bugs/Public/show_bug.cgi?id=27414
// https://dom.spec.whatwg.org/#eventtarget-legacy-canceled-activation-behavior
fn legacy_canceled_activation_behavior(&self, cache: Option<InputActivationState>) {
// Step 1
if !self.is_mutable() {
return;
}
match ty {
// https://html.spec.whatwg.org/multipage/#submit-button-state-(type=submit):activation-behavior
// InputType::Submit => (), // No behavior defined
// https://html.spec.whatwg.org/multipage/#reset-button-state-(type=reset):activation-behavior
// InputType::Reset => (), // No behavior defined
// https://html.spec.whatwg.org/multipage/#checkbox-state-(type=checkbox):canceled-activation-steps
InputType::Checkbox => {
// We want to restore state only if the element had been changed in the first place
if cache.was_mutable {
self.SetIndeterminate(cache.indeterminate);
self.SetChecked(cache.checked);
self.checked_changed.set(cache.checked_changed);
let ty = self.input_type();
let cache = match cache {
Some(cache) => {
if cache.old_type != ty {
// Type changed, abandon ship
// https://www.w3.org/Bugs/Public/show_bug.cgi?id=27414
return;
}
cache
},
// https://html.spec.whatwg.org/multipage/#radio-button-state-(type=radio):canceled-activation-steps
None => {
return;
},
};
match ty {
// Step 2
InputType::Checkbox => {
self.SetIndeterminate(cache.indeterminate);
self.SetChecked(cache.checked);
},
// Step 3
InputType::Radio => {
// We want to restore state only if the element had been changed in the first place
if cache.was_mutable {
if let Some(ref o) = cache.checked_radio {
let tree_root = self
.upcast::<Node>()
.GetRootNode(&GetRootNodeOptions::empty());
// Avoiding iterating through the whole tree here, instead
// we can check if the conditions for radio group siblings apply
if in_same_group(
&o,
self.form_owner().as_deref(),
self.radio_group_name().as_ref(),
Some(&*tree_root),
) {
o.SetChecked(true);
} else {
self.SetChecked(false);
}
if let Some(ref o) = cache.checked_radio {
let tree_root = self
.upcast::<Node>()
.GetRootNode(&GetRootNodeOptions::empty());
// Avoiding iterating through the whole tree here, instead
// we can check if the conditions for radio group siblings apply
if in_same_group(
&o,
self.form_owner().as_deref(),
self.radio_group_name().as_ref(),
Some(&*tree_root),
) {
o.SetChecked(true);
} else {
self.SetChecked(false);
}
self.checked_changed.set(cache.checked_changed);
} else {
self.SetChecked(false);
}
},
_ => (),
@ -2459,11 +2435,6 @@ impl Activatable for HTMLInputElement {
// https://html.spec.whatwg.org/multipage/#run-post-click-activation-steps
fn activation_behavior(&self, _event: &Event, _target: &EventTarget) {
let ty = self.input_type();
if self.activation_state.borrow().old_type != ty || !self.is_mutable() {
// Type changed or input is immutable, abandon ship
// https://www.w3.org/Bugs/Public/show_bug.cgi?id=27414
return;
}
match ty {
InputType::Submit => {
// https://html.spec.whatwg.org/multipage/#submit-button-state-(type=submit):activation-behavior