diff --git a/components/script/dom/htmlbuttonelement.rs b/components/script/dom/htmlbuttonelement.rs index c2538fd9bd2..93f7f9b292a 100644 --- a/components/script/dom/htmlbuttonelement.rs +++ b/components/script/dom/htmlbuttonelement.rs @@ -232,13 +232,9 @@ impl<'a> Activatable for &'a HTMLButtonElement { if owner.is_none() || elem.click_in_progress() { return; } - // This is safe because we are stopping after finding the first element - // and only then performing actions which may modify the DOM tree - unsafe { - node.query_selector_iter("button[type=submit]".to_owned()).unwrap() - .filter_map(HTMLButtonElementCast::to_root) - .find(|r| r.r().form_owner() == owner) - .map(|s| s.r().synthetic_click_activation(ctrlKey, shiftKey, altKey, metaKey)); - } + node.query_selector_iter("button[type=submit]".to_owned()).unwrap() + .filter_map(HTMLButtonElementCast::to_root) + .find(|r| r.r().form_owner() == owner) + .map(|s| s.r().synthetic_click_activation(ctrlKey, shiftKey, altKey, metaKey)); } } diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 90cdb5bac6a..78ca2e3e726 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -359,12 +359,9 @@ fn broadcast_radio_checked(broadcaster: &HTMLInputElement, group: Option<&Atom>) // This function is a workaround for lifetime constraint difficulties. fn do_broadcast(doc_node: &Node, broadcaster: &HTMLInputElement, owner: Option<&HTMLFormElement>, group: Option<&Atom>) { - // There is no DOM tree manipulation here, so this is safe - let iter = unsafe { - doc_node.query_selector_iter("input[type=radio]".to_owned()).unwrap() - .filter_map(HTMLInputElementCast::to_root) - .filter(|r| in_same_group(r.r(), owner, group) && broadcaster != r.r()) - }; + let iter = doc_node.query_selector_iter("input[type=radio]".to_owned()) + .unwrap().filter_map(HTMLInputElementCast::to_root) + .filter(|r| in_same_group(r.r(), owner, group) && broadcaster != r.r()); for ref r in iter { if r.r().Checked() { r.r().SetChecked(false); @@ -714,15 +711,12 @@ impl Activatable for HTMLInputElement { let doc_node = NodeCast::from_ref(doc.r()); let group = self.get_radio_group_name();; - // Safe since we only manipulate the DOM tree after finding an element - let checked_member = unsafe { - doc_node.query_selector_iter("input[type=radio]".to_owned()).unwrap() - .filter_map(HTMLInputElementCast::to_root) - .find(|r| { - in_same_group(r.r(), owner.r(), group.as_ref()) && - r.r().Checked() - }) - }; + let checked_member = doc_node.query_selector_iter("input[type=radio]".to_owned()).unwrap() + .filter_map(HTMLInputElementCast::to_root) + .find(|r| { + in_same_group(r.r(), owner.r(), group.as_ref()) && + r.r().Checked() + }); cache.checked_radio = checked_member.r().map(JS::from_ref); cache.checked_changed = self.checked_changed.get(); self.SetChecked(true); @@ -849,14 +843,10 @@ impl Activatable for HTMLInputElement { if elem.click_in_progress() { return; } - // This is safe because we are stopping after finding the first element - // and only then performing actions which may modify the DOM tree let submit_button; - unsafe { - submit_button = node.query_selector_iter("input[type=submit]".to_owned()).unwrap() - .filter_map(HTMLInputElementCast::to_root) - .find(|r| r.r().form_owner() == owner); - } + submit_button = node.query_selector_iter("input[type=submit]".to_owned()).unwrap() + .filter_map(HTMLInputElementCast::to_root) + .find(|r| r.r().form_owner() == owner); match submit_button { Some(ref button) => { if button.r().is_instance_activatable() { @@ -864,28 +854,23 @@ impl Activatable for HTMLInputElement { } } None => { - unsafe { - // Safe because we don't perform any DOM modification - // until we're done with the iterator. - let inputs = node.query_selector_iter("input".to_owned()).unwrap() - .filter_map(HTMLInputElementCast::to_root) - .filter(|input| { - input.r().form_owner() == owner && match &*input.r().Type() { - "text" | "search" | "url" | "tel" | - "email" | "password" | "datetime" | - "date" | "month" | "week" | "time" | - "datetime-local" | "number" - => true, - _ => false - } - }); + let inputs = node.query_selector_iter("input".to_owned()).unwrap() + .filter_map(HTMLInputElementCast::to_root) + .filter(|input| { + input.r().form_owner() == owner && match &*input.r().Type() { + "text" | "search" | "url" | "tel" | + "email" | "password" | "datetime" | + "date" | "month" | "week" | "time" | + "datetime-local" | "number" + => true, + _ => false + } + }); - if inputs.skip(1).next().is_some() { - // lazily test for > 1 submission-blocking inputs - return; - } + if inputs.skip(1).next().is_some() { + // lazily test for > 1 submission-blocking inputs + return; } - form.r().submit(SubmittedFrom::NotFromFormSubmitMethod, FormSubmitter::FormElement(form.r())); } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 034b8dfc3db..b50d71ccd10 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -345,8 +345,7 @@ pub struct QuerySelectorIterator { } impl<'a> QuerySelectorIterator { - #[allow(unsafe_code)] - unsafe fn new(iter: TreeIterator, selectors: Vec) + fn new(iter: TreeIterator, selectors: Vec) -> QuerySelectorIterator { QuerySelectorIterator { selectors: selectors, @@ -737,8 +736,7 @@ impl Node { /// Get an iterator over all nodes which match a set of selectors /// Be careful not to do anything which may manipulate the DOM tree /// whilst iterating, otherwise the iterator may be invalidated. - #[allow(unsafe_code)] - pub unsafe fn query_selector_iter(&self, selectors: DOMString) + pub fn query_selector_iter(&self, selectors: DOMString) -> Fallible { // Step 1. match parse_author_origin_selector_list_from_str(&selectors) { @@ -755,7 +753,7 @@ impl Node { #[allow(unsafe_code)] pub fn query_selector_all(&self, selectors: DOMString) -> Fallible> { let window = window_from_node(self); - let iter = try!(unsafe { self.query_selector_iter(selectors) }); + let iter = try!(self.query_selector_iter(selectors)); Ok(NodeList::new_simple_list(window.r(), iter)) }