From 311d2c65b676093057ca96f2b4037fbb0c3a3f4d Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 11 Apr 2016 08:07:32 -0400 Subject: [PATCH 1/8] Rename get_value_mode to value_mode --- components/script/dom/htmlinputelement.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 174a312448f..aefa40507f0 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -164,7 +164,7 @@ impl HTMLInputElement { } // 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() { InputType::InputSubmit | InputType::InputReset | @@ -361,7 +361,7 @@ impl HTMLInputElementMethods for HTMLInputElement { // https://html.spec.whatwg.org/multipage/#dom-input-value fn Value(&self) -> DOMString { - match self.get_value_mode() { + match self.value_mode() { ValueMode::Value => self.textinput.borrow().get_content(), ValueMode::Default => { self.upcast::() @@ -384,7 +384,7 @@ impl HTMLInputElementMethods for HTMLInputElement { // https://html.spec.whatwg.org/multipage/#dom-input-value fn SetValue(&self, value: DOMString) -> ErrorResult { - match self.get_value_mode() { + match self.value_mode() { ValueMode::Value => { self.textinput.borrow_mut().set_content(value); self.value_dirty.set(true); @@ -740,9 +740,9 @@ impl VirtualMethods for HTMLInputElement { }; // 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); - 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) { From e4cd8044a509ced782c1079aa79eceae197ae454 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 11 Apr 2016 08:18:53 -0400 Subject: [PATCH 2/8] Remove unused fn get_indeterminate_state --- components/script/dom/htmlinputelement.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index aefa40507f0..6dbacf34025 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -655,10 +655,6 @@ impl HTMLInputElement { //TODO: dispatch change event } - pub fn get_indeterminate_state(&self) -> bool { - self.Indeterminate() - } - // https://html.spec.whatwg.org/multipage/#concept-fe-mutable fn mutable(&self) -> bool { // https://html.spec.whatwg.org/multipage/#the-input-element:concept-fe-mutable From 0541a01924078a8c78067a6da3bcf0e7903ad059 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 11 Apr 2016 08:20:30 -0400 Subject: [PATCH 3/8] Rename get_radio_group_name to radio_group_name --- components/script/dom/htmlinputelement.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 6dbacf34025..e703e38f16b 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -581,7 +581,7 @@ fn in_same_group(other: &HTMLInputElement, owner: Option<&HTMLFormElement>, other.input_type.get() == InputType::InputRadio && // TODO Both a and b are in the same home subtree. 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!(""), _ => false } @@ -632,7 +632,7 @@ impl HTMLInputElement { } // https://html.spec.whatwg.org/multipage/#radio-button-group - fn get_radio_group_name(&self) -> Option { + fn radio_group_name(&self) -> Option { //TODO: determine form owner self.upcast::() .get_attribute(&ns!(), &atom!("name")) @@ -648,7 +648,7 @@ impl HTMLInputElement { if self.input_type.get() == InputType::InputRadio && checked { broadcast_radio_checked(self, - self.get_radio_group_name().as_ref()); + self.radio_group_name().as_ref()); } self.upcast::().dirty(NodeDamage::OtherNodeDamage); @@ -770,7 +770,7 @@ impl VirtualMethods for HTMLInputElement { // Step 5 if new_type == InputType::InputRadio { self.radio_group_updated( - self.get_radio_group_name().as_ref()); + self.radio_group_name().as_ref()); } // TODO: Step 6 - value sanitization @@ -779,7 +779,7 @@ impl VirtualMethods for HTMLInputElement { if self.input_type.get() == InputType::InputRadio { broadcast_radio_checked( self, - self.get_radio_group_name().as_ref()); + self.radio_group_name().as_ref()); } self.input_type.set(InputType::InputText); } @@ -948,7 +948,7 @@ impl Activatable for HTMLInputElement { let owner = self.form_owner(); let doc = document_from_node(self); let doc_node = doc.upcast::(); - 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 let checked_member = doc_node.query_selector_iter(DOMString::from("input[type=radio]")) @@ -994,12 +994,12 @@ impl Activatable for HTMLInputElement { InputType::InputRadio => { // We want to restore state only if the element had been changed in the first place if cache.was_mutable { - let name = self.get_radio_group_name(); + let name = self.radio_group_name(); match cache.checked_radio.r() { Some(o) => { // Avoiding iterating through the whole tree here, instead // we can check if the conditions for radio group siblings apply - if name == o.get_radio_group_name() && // TODO should be compatibility caseless + if name == o.radio_group_name() && // TODO should be compatibility caseless self.form_owner() == o.form_owner() && // TODO Both a and b are in the same home subtree o.input_type.get() == InputType::InputRadio { From f54a8fc8456afe8a8d8c2edc0890694506e175f3 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 11 Apr 2016 08:41:46 -0400 Subject: [PATCH 4/8] Use fn in_same_group where appropriate --- components/script/dom/htmlinputelement.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index e703e38f16b..845010ce627 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -994,16 +994,12 @@ impl Activatable for HTMLInputElement { InputType::InputRadio => { // We want to restore state only if the element had been changed in the first place if cache.was_mutable { - let name = self.radio_group_name(); match cache.checked_radio.r() { Some(o) => { // Avoiding iterating through the whole tree here, instead // we can check if the conditions for radio group siblings apply - if name == o.radio_group_name() && // TODO should be compatibility caseless - self.form_owner() == o.form_owner() && - // TODO Both a and b are in the same home subtree - o.input_type.get() == InputType::InputRadio { - o.SetChecked(true); + if in_same_group(&o, self.form_owner().r(), self.radio_group_name().as_ref()) { + o.SetChecked(true); } else { self.SetChecked(false); } From 916c035b57c7787e569186cd52406b73ea605ad6 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 11 Apr 2016 08:54:50 -0400 Subject: [PATCH 5/8] Rename fn mutable to is_mutable --- components/script/dom/htmlinputelement.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 845010ce627..77316f4b6f8 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -656,7 +656,7 @@ impl HTMLInputElement { } // 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-readonly-attribute:concept-fe-mutable !(self.upcast::().disabled_state() || self.ReadOnly()) @@ -912,7 +912,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/#radio-button-state-%28type=radio%29:activation-behaviour-2 InputType::InputSubmit | InputType::InputReset - | InputType::InputCheckbox | InputType::InputRadio => self.mutable(), + | InputType::InputCheckbox | InputType::InputRadio => self.is_mutable(), _ => false } } @@ -923,7 +923,7 @@ impl Activatable for HTMLInputElement { let mut cache = self.activation_state.borrow_mut(); let ty = self.input_type.get(); cache.old_type = ty; - cache.was_mutable = self.mutable(); + 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 @@ -1025,7 +1025,7 @@ impl Activatable for HTMLInputElement { InputType::InputSubmit => { // 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) - if self.mutable() /* and document owner is fully active */ { + if self.is_mutable() /* and document owner is fully active */ { self.form_owner().map(|o| { o.submit(SubmittedFrom::NotFromFormSubmitMethod, FormSubmitter::InputElement(self.clone())) @@ -1035,7 +1035,7 @@ impl Activatable for HTMLInputElement { InputType::InputReset => { // 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) - if self.mutable() /* and document owner is fully active */ { + if self.is_mutable() /* and document owner is fully active */ { self.form_owner().map(|o| { o.reset(ResetFrom::NotFromFormResetMethod) }); @@ -1044,7 +1044,7 @@ impl Activatable for HTMLInputElement { InputType::InputCheckbox | InputType::InputRadio => { // 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 - if self.mutable() { + if self.is_mutable() { let target = self.upcast::(); target.fire_event("input", EventBubbles::Bubbles, From 814afd6537b8d2fd9b86119748978f51660a54c2 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 11 Apr 2016 08:55:26 -0400 Subject: [PATCH 6/8] Move check for mutability up as a guard clause --- components/script/dom/htmlinputelement.rs | 41 +++++++++++------------ 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 77316f4b6f8..dc9884bca87 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -1016,8 +1016,8 @@ 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.get(); - if self.activation_state.borrow().old_type != ty { - // Type changed, abandon ship + 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; } @@ -1025,34 +1025,31 @@ impl Activatable for HTMLInputElement { InputType::InputSubmit => { // 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) - if self.is_mutable() /* and document owner is fully active */ { - self.form_owner().map(|o| { - o.submit(SubmittedFrom::NotFromFormSubmitMethod, - FormSubmitter::InputElement(self.clone())) - }); - } + // Check if document owner is fully active + self.form_owner().map(|o| { + o.submit(SubmittedFrom::NotFromFormSubmitMethod, + FormSubmitter::InputElement(self.clone())) + }); }, InputType::InputReset => { // 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) - if self.is_mutable() /* and document owner is fully active */ { - self.form_owner().map(|o| { - o.reset(ResetFrom::NotFromFormResetMethod) - }); - } + // Check if document owner is fully active + self.form_owner().map(|o| { + o.reset(ResetFrom::NotFromFormResetMethod) + }); }, InputType::InputCheckbox | InputType::InputRadio => { // 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 - if self.is_mutable() { - let target = self.upcast::(); - target.fire_event("input", - EventBubbles::Bubbles, - EventCancelable::NotCancelable); - target.fire_event("change", - EventBubbles::Bubbles, - EventCancelable::NotCancelable); - } + // Check if document owner is fully active + let target = self.upcast::(); + target.fire_event("input", + EventBubbles::Bubbles, + EventCancelable::NotCancelable); + target.fire_event("change", + EventBubbles::Bubbles, + EventCancelable::NotCancelable); }, _ => () } From 2d503c8281e0df7e4512b7df6c3032a5b6b5507f Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 11 Apr 2016 08:56:37 -0400 Subject: [PATCH 7/8] Rename fn get_form_datum to form_datum --- components/script/dom/htmlformelement.rs | 2 +- components/script/dom/htmlinputelement.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/script/dom/htmlformelement.rs b/components/script/dom/htmlformelement.rs index 70a2a7ad730..3e7421de443 100644 --- a/components/script/dom/htmlformelement.rs +++ b/components/script/dom/htmlformelement.rs @@ -414,7 +414,7 @@ impl HTMLFormElement { HTMLElementTypeId::HTMLInputElement => { let input = child.downcast::().unwrap(); // 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); } } diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index dc9884bca87..903c68ec9af 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -596,7 +596,7 @@ impl HTMLInputElement { /// https://html.spec.whatwg.org/multipage/#constructing-the-form-data-set /// Steps range from 3.1 to 3.7 which related to the HTMLInputElement - pub fn get_form_datum(&self, submitter: Option) -> Option { + pub fn form_datum(&self, submitter: Option) -> Option { // Step 3.2 let ty = self.type_(); // Step 3.4 From c9193eaeca0b16ef065136302a86a14b5461474c Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 11 Apr 2016 09:17:04 -0400 Subject: [PATCH 8/8] Remove get_ prefix for functions in LayoutHTMLInputElementHelpers --- components/layout/wrapper.rs | 4 ++-- components/script/dom/element.rs | 6 +++--- components/script/dom/htmlinputelement.rs | 26 +++++++++++------------ 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index ad8ce2f5141..e76af843ea0 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -1050,7 +1050,7 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> { return TextContent::Text(data); } if let Some(input) = this.downcast::() { - let data = unsafe { input.get_value_for_layout() }; + let data = unsafe { input.value_for_layout() }; return TextContent::Text(data); } if let Some(area) = this.downcast::() { @@ -1076,7 +1076,7 @@ impl<'ln> ThreadSafeLayoutNode for ServoThreadSafeLayoutNode<'ln> { } } if let Some(input) = this.downcast::() { - 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()), CharIndex(selection.length()))); } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 4e2af78f31a..f27bc3a35fb 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -377,7 +377,7 @@ impl LayoutElementHelpers for LayoutJS { }, // Others _ => { - match this.get_size_for_layout() { + match this.size_for_layout() { 0 => None, s => Some(s as i32), } @@ -571,7 +571,7 @@ impl LayoutElementHelpers for LayoutJS { // TODO option and menuitem can also have a checked state. match self.downcast::() { Some(input) => unsafe { - input.get_checked_state_for_layout() + input.checked_state_for_layout() }, None => false, } @@ -583,7 +583,7 @@ impl LayoutElementHelpers for LayoutJS { // TODO progress elements can also be matched with :indeterminate match self.downcast::() { Some(input) => unsafe { - input.get_indeterminate_state_for_layout() + input.indeterminate_state_for_layout() }, None => false, } diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 903c68ec9af..c8e053ece0f 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -208,15 +208,15 @@ impl HTMLInputElement { pub trait LayoutHTMLInputElementHelpers { #[allow(unsafe_code)] - unsafe fn get_value_for_layout(self) -> String; + unsafe fn value_for_layout(self) -> String; #[allow(unsafe_code)] - unsafe fn get_size_for_layout(self) -> u32; + unsafe fn size_for_layout(self) -> u32; #[allow(unsafe_code)] - unsafe fn get_selection_for_layout(self) -> Option>; + unsafe fn selection_for_layout(self) -> Option>; #[allow(unsafe_code)] - unsafe fn get_checked_state_for_layout(self) -> bool; + unsafe fn checked_state_for_layout(self) -> bool; #[allow(unsafe_code)] - unsafe fn get_indeterminate_state_for_layout(self) -> bool; + unsafe fn indeterminate_state_for_layout(self) -> bool; } #[allow(unsafe_code)] @@ -226,7 +226,7 @@ unsafe fn get_raw_textinput_value(input: LayoutJS) -> DOMStrin impl LayoutHTMLInputElementHelpers for LayoutJS { #[allow(unsafe_code)] - unsafe fn get_value_for_layout(self) -> String { + unsafe fn value_for_layout(self) -> String { #[allow(unsafe_code)] unsafe fn get_raw_attr_value(input: LayoutJS, default: &str) -> String { let elem = input.upcast::(); @@ -245,7 +245,7 @@ impl LayoutHTMLInputElementHelpers for LayoutJS { InputType::InputPassword => { let text = get_raw_textinput_value(self); 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() } else { String::from((*self.unsafe_get()).placeholder.borrow_for_layout().clone()) @@ -254,7 +254,7 @@ impl LayoutHTMLInputElementHelpers for LayoutJS { _ => { let text = get_raw_textinput_value(self); 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) } else { String::from((*self.unsafe_get()).placeholder.borrow_for_layout().clone()) @@ -265,19 +265,19 @@ impl LayoutHTMLInputElementHelpers for LayoutJS { #[allow(unrooted_must_root)] #[allow(unsafe_code)] - unsafe fn get_size_for_layout(self) -> u32 { + unsafe fn size_for_layout(self) -> u32 { (*self.unsafe_get()).size.get() } #[allow(unrooted_must_root)] #[allow(unsafe_code)] - unsafe fn get_selection_for_layout(self) -> Option> { + unsafe fn selection_for_layout(self) -> Option> { if !(*self.unsafe_get()).upcast::().focus_state() { return None; } // 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() { InputType::InputText | InputType::InputPassword => get_raw_textinput_value(self), @@ -293,13 +293,13 @@ impl LayoutHTMLInputElementHelpers for LayoutJS { #[allow(unrooted_must_root)] #[allow(unsafe_code)] - unsafe fn get_checked_state_for_layout(self) -> bool { + unsafe fn checked_state_for_layout(self) -> bool { self.upcast::().get_state_for_layout().contains(IN_CHECKED_STATE) } #[allow(unrooted_must_root)] #[allow(unsafe_code)] - unsafe fn get_indeterminate_state_for_layout(self) -> bool { + unsafe fn indeterminate_state_for_layout(self) -> bool { self.upcast::().get_state_for_layout().contains(IN_INDETERMINATE_STATE) } }