Auto merge of #10522 - KiChjang:input-cleanup, r=frewsxcv

Various cleanups in HTMLInputElement

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10522)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2016-04-12 07:56:38 +05:30
commit 934ae41fc9
4 changed files with 55 additions and 66 deletions

View file

@ -1050,7 +1050,7 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
return TextContent::Text(data); return TextContent::Text(data);
} }
if let Some(input) = this.downcast::<HTMLInputElement>() { if let Some(input) = this.downcast::<HTMLInputElement>() {
let data = unsafe { input.get_value_for_layout() }; let data = unsafe { input.value_for_layout() };
return TextContent::Text(data); return TextContent::Text(data);
} }
if let Some(area) = this.downcast::<HTMLTextAreaElement>() { if let Some(area) = this.downcast::<HTMLTextAreaElement>() {
@ -1076,7 +1076,7 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> {
} }
} }
if let Some(input) = this.downcast::<HTMLInputElement>() { if let Some(input) = this.downcast::<HTMLInputElement>() {
if let Some(selection) = unsafe { input.get_selection_for_layout() } { if let Some(selection) = unsafe { input.selection_for_layout() } {
return Some(Range::new(CharIndex(selection.begin()), return Some(Range::new(CharIndex(selection.begin()),
CharIndex(selection.length()))); CharIndex(selection.length())));
} }

View file

@ -377,7 +377,7 @@ impl LayoutElementHelpers for LayoutJS<Element> {
}, },
// Others // Others
_ => { _ => {
match this.get_size_for_layout() { match this.size_for_layout() {
0 => None, 0 => None,
s => Some(s as i32), s => Some(s as i32),
} }
@ -571,7 +571,7 @@ impl LayoutElementHelpers for LayoutJS<Element> {
// TODO option and menuitem can also have a checked state. // TODO option and menuitem can also have a checked state.
match self.downcast::<HTMLInputElement>() { match self.downcast::<HTMLInputElement>() {
Some(input) => unsafe { Some(input) => unsafe {
input.get_checked_state_for_layout() input.checked_state_for_layout()
}, },
None => false, None => false,
} }
@ -583,7 +583,7 @@ impl LayoutElementHelpers for LayoutJS<Element> {
// TODO progress elements can also be matched with :indeterminate // TODO progress elements can also be matched with :indeterminate
match self.downcast::<HTMLInputElement>() { match self.downcast::<HTMLInputElement>() {
Some(input) => unsafe { Some(input) => unsafe {
input.get_indeterminate_state_for_layout() input.indeterminate_state_for_layout()
}, },
None => false, None => false,
} }

View file

@ -414,7 +414,7 @@ impl HTMLFormElement {
HTMLElementTypeId::HTMLInputElement => { HTMLElementTypeId::HTMLInputElement => {
let input = child.downcast::<HTMLInputElement>().unwrap(); let input = child.downcast::<HTMLInputElement>().unwrap();
// Step 3.2-3.7 // Step 3.2-3.7
if let Some(datum) = input.get_form_datum(submitter) { if let Some(datum) = input.form_datum(submitter) {
data_set.push(datum); data_set.push(datum);
} }
} }

View file

@ -164,7 +164,7 @@ impl HTMLInputElement {
} }
// https://html.spec.whatwg.org/multipage/#input-type-attr-summary // https://html.spec.whatwg.org/multipage/#input-type-attr-summary
fn get_value_mode(&self) -> ValueMode { fn value_mode(&self) -> ValueMode {
match self.input_type.get() { match self.input_type.get() {
InputType::InputSubmit | InputType::InputSubmit |
InputType::InputReset | InputType::InputReset |
@ -208,15 +208,15 @@ impl HTMLInputElement {
pub trait LayoutHTMLInputElementHelpers { pub trait LayoutHTMLInputElementHelpers {
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn get_value_for_layout(self) -> String; unsafe fn value_for_layout(self) -> String;
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn get_size_for_layout(self) -> u32; unsafe fn size_for_layout(self) -> u32;
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn get_selection_for_layout(self) -> Option<Range<isize>>; unsafe fn selection_for_layout(self) -> Option<Range<isize>>;
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn get_checked_state_for_layout(self) -> bool; unsafe fn checked_state_for_layout(self) -> bool;
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn get_indeterminate_state_for_layout(self) -> bool; unsafe fn indeterminate_state_for_layout(self) -> bool;
} }
#[allow(unsafe_code)] #[allow(unsafe_code)]
@ -226,7 +226,7 @@ unsafe fn get_raw_textinput_value(input: LayoutJS<HTMLInputElement>) -> DOMStrin
impl LayoutHTMLInputElementHelpers for LayoutJS<HTMLInputElement> { impl LayoutHTMLInputElementHelpers for LayoutJS<HTMLInputElement> {
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn get_value_for_layout(self) -> String { unsafe fn value_for_layout(self) -> String {
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn get_raw_attr_value(input: LayoutJS<HTMLInputElement>, default: &str) -> String { unsafe fn get_raw_attr_value(input: LayoutJS<HTMLInputElement>, default: &str) -> String {
let elem = input.upcast::<Element>(); let elem = input.upcast::<Element>();
@ -245,7 +245,7 @@ impl LayoutHTMLInputElementHelpers for LayoutJS<HTMLInputElement> {
InputType::InputPassword => { InputType::InputPassword => {
let text = get_raw_textinput_value(self); let text = get_raw_textinput_value(self);
if !text.is_empty() { if !text.is_empty() {
// The implementation of get_selection_for_layout expects a 1:1 mapping of chars. // The implementation of selection_for_layout expects a 1:1 mapping of chars.
text.chars().map(|_| '●').collect() text.chars().map(|_| '●').collect()
} else { } else {
String::from((*self.unsafe_get()).placeholder.borrow_for_layout().clone()) String::from((*self.unsafe_get()).placeholder.borrow_for_layout().clone())
@ -254,7 +254,7 @@ impl LayoutHTMLInputElementHelpers for LayoutJS<HTMLInputElement> {
_ => { _ => {
let text = get_raw_textinput_value(self); let text = get_raw_textinput_value(self);
if !text.is_empty() { if !text.is_empty() {
// The implementation of get_selection_for_layout expects a 1:1 mapping of chars. // The implementation of selection_for_layout expects a 1:1 mapping of chars.
String::from(text) String::from(text)
} else { } else {
String::from((*self.unsafe_get()).placeholder.borrow_for_layout().clone()) String::from((*self.unsafe_get()).placeholder.borrow_for_layout().clone())
@ -265,19 +265,19 @@ impl LayoutHTMLInputElementHelpers for LayoutJS<HTMLInputElement> {
#[allow(unrooted_must_root)] #[allow(unrooted_must_root)]
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn get_size_for_layout(self) -> u32 { unsafe fn size_for_layout(self) -> u32 {
(*self.unsafe_get()).size.get() (*self.unsafe_get()).size.get()
} }
#[allow(unrooted_must_root)] #[allow(unrooted_must_root)]
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn get_selection_for_layout(self) -> Option<Range<isize>> { unsafe fn selection_for_layout(self) -> Option<Range<isize>> {
if !(*self.unsafe_get()).upcast::<Element>().focus_state() { if !(*self.unsafe_get()).upcast::<Element>().focus_state() {
return None; return None;
} }
// Use the raw textinput to get the index as long as we use a 1:1 char mapping // Use the raw textinput to get the index as long as we use a 1:1 char mapping
// in get_value_for_layout. // in value_for_layout.
let raw = match (*self.unsafe_get()).input_type.get() { let raw = match (*self.unsafe_get()).input_type.get() {
InputType::InputText | InputType::InputText |
InputType::InputPassword => get_raw_textinput_value(self), InputType::InputPassword => get_raw_textinput_value(self),
@ -293,13 +293,13 @@ impl LayoutHTMLInputElementHelpers for LayoutJS<HTMLInputElement> {
#[allow(unrooted_must_root)] #[allow(unrooted_must_root)]
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn get_checked_state_for_layout(self) -> bool { unsafe fn checked_state_for_layout(self) -> bool {
self.upcast::<Element>().get_state_for_layout().contains(IN_CHECKED_STATE) self.upcast::<Element>().get_state_for_layout().contains(IN_CHECKED_STATE)
} }
#[allow(unrooted_must_root)] #[allow(unrooted_must_root)]
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn get_indeterminate_state_for_layout(self) -> bool { unsafe fn indeterminate_state_for_layout(self) -> bool {
self.upcast::<Element>().get_state_for_layout().contains(IN_INDETERMINATE_STATE) self.upcast::<Element>().get_state_for_layout().contains(IN_INDETERMINATE_STATE)
} }
} }
@ -380,7 +380,7 @@ impl HTMLInputElementMethods for HTMLInputElement {
// https://html.spec.whatwg.org/multipage/#dom-input-value // https://html.spec.whatwg.org/multipage/#dom-input-value
fn Value(&self) -> DOMString { fn Value(&self) -> DOMString {
match self.get_value_mode() { match self.value_mode() {
ValueMode::Value => self.textinput.borrow().get_content(), ValueMode::Value => self.textinput.borrow().get_content(),
ValueMode::Default => { ValueMode::Default => {
self.upcast::<Element>() self.upcast::<Element>()
@ -403,7 +403,7 @@ impl HTMLInputElementMethods for HTMLInputElement {
// https://html.spec.whatwg.org/multipage/#dom-input-value // https://html.spec.whatwg.org/multipage/#dom-input-value
fn SetValue(&self, value: DOMString) -> ErrorResult { fn SetValue(&self, value: DOMString) -> ErrorResult {
match self.get_value_mode() { match self.value_mode() {
ValueMode::Value => { ValueMode::Value => {
self.textinput.borrow_mut().set_content(value); self.textinput.borrow_mut().set_content(value);
self.value_dirty.set(true); self.value_dirty.set(true);
@ -642,7 +642,7 @@ fn in_same_group(other: &HTMLInputElement, owner: Option<&HTMLFormElement>,
other.input_type.get() == InputType::InputRadio && other.input_type.get() == InputType::InputRadio &&
// TODO Both a and b are in the same home subtree. // TODO Both a and b are in the same home subtree.
other.form_owner().r() == owner && other.form_owner().r() == owner &&
match (other.get_radio_group_name(), group) { match (other.radio_group_name(), group) {
(Some(ref s1), Some(s2)) => compatibility_caseless_match_str(s1, s2) && s2 != &atom!(""), (Some(ref s1), Some(s2)) => compatibility_caseless_match_str(s1, s2) && s2 != &atom!(""),
_ => false _ => false
} }
@ -657,7 +657,7 @@ impl HTMLInputElement {
/// https://html.spec.whatwg.org/multipage/#constructing-the-form-data-set /// https://html.spec.whatwg.org/multipage/#constructing-the-form-data-set
/// Steps range from 3.1 to 3.7 which related to the HTMLInputElement /// Steps range from 3.1 to 3.7 which related to the HTMLInputElement
pub fn get_form_datum(&self, submitter: Option<FormSubmitter>) -> Option<FormDatum> { pub fn form_datum(&self, submitter: Option<FormSubmitter>) -> Option<FormDatum> {
// Step 3.2 // Step 3.2
let ty = self.type_(); let ty = self.type_();
// Step 3.4 // Step 3.4
@ -693,7 +693,7 @@ impl HTMLInputElement {
} }
// https://html.spec.whatwg.org/multipage/#radio-button-group // https://html.spec.whatwg.org/multipage/#radio-button-group
fn get_radio_group_name(&self) -> Option<Atom> { fn radio_group_name(&self) -> Option<Atom> {
//TODO: determine form owner //TODO: determine form owner
self.upcast::<Element>() self.upcast::<Element>()
.get_attribute(&ns!(), &atom!("name")) .get_attribute(&ns!(), &atom!("name"))
@ -709,19 +709,15 @@ impl HTMLInputElement {
if self.input_type.get() == InputType::InputRadio && checked { if self.input_type.get() == InputType::InputRadio && checked {
broadcast_radio_checked(self, broadcast_radio_checked(self,
self.get_radio_group_name().as_ref()); self.radio_group_name().as_ref());
} }
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage); self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
//TODO: dispatch change event //TODO: dispatch change event
} }
pub fn get_indeterminate_state(&self) -> bool {
self.Indeterminate()
}
// https://html.spec.whatwg.org/multipage/#concept-fe-mutable // https://html.spec.whatwg.org/multipage/#concept-fe-mutable
fn mutable(&self) -> bool { fn is_mutable(&self) -> bool {
// https://html.spec.whatwg.org/multipage/#the-input-element:concept-fe-mutable // https://html.spec.whatwg.org/multipage/#the-input-element:concept-fe-mutable
// https://html.spec.whatwg.org/multipage/#the-readonly-attribute:concept-fe-mutable // https://html.spec.whatwg.org/multipage/#the-readonly-attribute:concept-fe-mutable
!(self.upcast::<Element>().disabled_state() || self.ReadOnly()) !(self.upcast::<Element>().disabled_state() || self.ReadOnly())
@ -801,9 +797,9 @@ impl VirtualMethods for HTMLInputElement {
}; };
// https://html.spec.whatwg.org/multipage/#input-type-change // https://html.spec.whatwg.org/multipage/#input-type-change
let (old_value_mode, old_idl_value) = (self.get_value_mode(), self.Value()); let (old_value_mode, old_idl_value) = (self.value_mode(), self.Value());
self.input_type.set(new_type); self.input_type.set(new_type);
let new_value_mode = self.get_value_mode(); let new_value_mode = self.value_mode();
match (&old_value_mode, old_idl_value.is_empty(), new_value_mode) { match (&old_value_mode, old_idl_value.is_empty(), new_value_mode) {
@ -835,7 +831,7 @@ impl VirtualMethods for HTMLInputElement {
// Step 5 // Step 5
if new_type == InputType::InputRadio { if new_type == InputType::InputRadio {
self.radio_group_updated( self.radio_group_updated(
self.get_radio_group_name().as_ref()); self.radio_group_name().as_ref());
} }
// TODO: Step 6 - value sanitization // TODO: Step 6 - value sanitization
@ -844,7 +840,7 @@ impl VirtualMethods for HTMLInputElement {
if self.input_type.get() == InputType::InputRadio { if self.input_type.get() == InputType::InputRadio {
broadcast_radio_checked( broadcast_radio_checked(
self, self,
self.get_radio_group_name().as_ref()); self.radio_group_name().as_ref());
} }
self.input_type.set(InputType::InputText); self.input_type.set(InputType::InputText);
} }
@ -977,7 +973,7 @@ impl Activatable for HTMLInputElement {
// https://html.spec.whatwg.org/multipage/#checkbox-state-%28type=checkbox%29:activation-behaviour-2 // https://html.spec.whatwg.org/multipage/#checkbox-state-%28type=checkbox%29:activation-behaviour-2
// https://html.spec.whatwg.org/multipage/#radio-button-state-%28type=radio%29:activation-behaviour-2 // https://html.spec.whatwg.org/multipage/#radio-button-state-%28type=radio%29:activation-behaviour-2
InputType::InputSubmit | InputType::InputReset InputType::InputSubmit | InputType::InputReset
| InputType::InputCheckbox | InputType::InputRadio => self.mutable(), | InputType::InputCheckbox | InputType::InputRadio => self.is_mutable(),
_ => false _ => false
} }
} }
@ -988,7 +984,7 @@ impl Activatable for HTMLInputElement {
let mut cache = self.activation_state.borrow_mut(); let mut cache = self.activation_state.borrow_mut();
let ty = self.input_type.get(); let ty = self.input_type.get();
cache.old_type = ty; cache.old_type = ty;
cache.was_mutable = self.mutable(); cache.was_mutable = self.is_mutable();
if cache.was_mutable { if cache.was_mutable {
match ty { match ty {
// https://html.spec.whatwg.org/multipage/#submit-button-state-(type=submit):activation-behavior // https://html.spec.whatwg.org/multipage/#submit-button-state-(type=submit):activation-behavior
@ -1013,7 +1009,7 @@ impl Activatable for HTMLInputElement {
let owner = self.form_owner(); let owner = self.form_owner();
let doc = document_from_node(self); let doc = document_from_node(self);
let doc_node = doc.upcast::<Node>(); let doc_node = doc.upcast::<Node>();
let group = self.get_radio_group_name();; let group = self.radio_group_name();;
// Safe since we only manipulate the DOM tree after finding an element // Safe since we only manipulate the DOM tree after finding an element
let checked_member = doc_node.query_selector_iter(DOMString::from("input[type=radio]")) let checked_member = doc_node.query_selector_iter(DOMString::from("input[type=radio]"))
@ -1059,16 +1055,12 @@ impl Activatable for HTMLInputElement {
InputType::InputRadio => { InputType::InputRadio => {
// We want to restore state only if the element had been changed in the first place // We want to restore state only if the element had been changed in the first place
if cache.was_mutable { if cache.was_mutable {
let name = self.get_radio_group_name();
match cache.checked_radio.r() { match cache.checked_radio.r() {
Some(o) => { Some(o) => {
// Avoiding iterating through the whole tree here, instead // Avoiding iterating through the whole tree here, instead
// we can check if the conditions for radio group siblings apply // we can check if the conditions for radio group siblings apply
if name == o.get_radio_group_name() && // TODO should be compatibility caseless if in_same_group(&o, self.form_owner().r(), self.radio_group_name().as_ref()) {
self.form_owner() == o.form_owner() && o.SetChecked(true);
// TODO Both a and b are in the same home subtree
o.input_type.get() == InputType::InputRadio {
o.SetChecked(true);
} else { } else {
self.SetChecked(false); self.SetChecked(false);
} }
@ -1085,8 +1077,8 @@ impl Activatable for HTMLInputElement {
// https://html.spec.whatwg.org/multipage/#run-post-click-activation-steps // https://html.spec.whatwg.org/multipage/#run-post-click-activation-steps
fn activation_behavior(&self, _event: &Event, _target: &EventTarget) { fn activation_behavior(&self, _event: &Event, _target: &EventTarget) {
let ty = self.input_type.get(); let ty = self.input_type.get();
if self.activation_state.borrow().old_type != ty { if self.activation_state.borrow().old_type != ty || !self.is_mutable() {
// Type changed, abandon ship // Type changed or input is immutable, abandon ship
// https://www.w3.org/Bugs/Public/show_bug.cgi?id=27414 // https://www.w3.org/Bugs/Public/show_bug.cgi?id=27414
return; return;
} }
@ -1094,34 +1086,31 @@ impl Activatable for HTMLInputElement {
InputType::InputSubmit => { InputType::InputSubmit => {
// https://html.spec.whatwg.org/multipage/#submit-button-state-(type=submit):activation-behavior // https://html.spec.whatwg.org/multipage/#submit-button-state-(type=submit):activation-behavior
// FIXME (Manishearth): support document owners (needs ability to get parent browsing context) // FIXME (Manishearth): support document owners (needs ability to get parent browsing context)
if self.mutable() /* and document owner is fully active */ { // Check if document owner is fully active
self.form_owner().map(|o| { self.form_owner().map(|o| {
o.submit(SubmittedFrom::NotFromFormSubmitMethod, o.submit(SubmittedFrom::NotFromFormSubmitMethod,
FormSubmitter::InputElement(self.clone())) FormSubmitter::InputElement(self.clone()))
}); });
}
}, },
InputType::InputReset => { InputType::InputReset => {
// https://html.spec.whatwg.org/multipage/#reset-button-state-(type=reset):activation-behavior // https://html.spec.whatwg.org/multipage/#reset-button-state-(type=reset):activation-behavior
// FIXME (Manishearth): support document owners (needs ability to get parent browsing context) // FIXME (Manishearth): support document owners (needs ability to get parent browsing context)
if self.mutable() /* and document owner is fully active */ { // Check if document owner is fully active
self.form_owner().map(|o| { self.form_owner().map(|o| {
o.reset(ResetFrom::NotFromFormResetMethod) o.reset(ResetFrom::NotFromFormResetMethod)
}); });
}
}, },
InputType::InputCheckbox | InputType::InputRadio => { InputType::InputCheckbox | InputType::InputRadio => {
// https://html.spec.whatwg.org/multipage/#checkbox-state-(type=checkbox):activation-behavior // https://html.spec.whatwg.org/multipage/#checkbox-state-(type=checkbox):activation-behavior
// https://html.spec.whatwg.org/multipage/#radio-button-state-(type=radio):activation-behavior // https://html.spec.whatwg.org/multipage/#radio-button-state-(type=radio):activation-behavior
if self.mutable() { // Check if document owner is fully active
let target = self.upcast::<EventTarget>(); let target = self.upcast::<EventTarget>();
target.fire_event("input", target.fire_event("input",
EventBubbles::Bubbles, EventBubbles::Bubbles,
EventCancelable::NotCancelable); EventCancelable::NotCancelable);
target.fire_event("change", target.fire_event("change",
EventBubbles::Bubbles, EventBubbles::Bubbles,
EventCancelable::NotCancelable); EventCancelable::NotCancelable);
}
}, },
_ => () _ => ()
} }