diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index cccb2658d40b..9a547c98efdc 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -171,6 +171,7 @@ pub trait DocumentHelpers<'a> { fn unregister_named_element(self, to_unregister: JSRef, id: Atom); fn register_named_element(self, element: JSRef, id: Atom); fn load_anchor_href(self, href: DOMString); + fn find_fragment_node(self, fragid: DOMString) -> Option>; } impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> { @@ -276,6 +277,24 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> { let window = self.window.root(); window.load_url(href); } + + /// Attempt to find a named element in this page's document. + fn find_fragment_node(self, fragid: DOMString) -> Option> { + match self.GetElementById(fragid.clone()) { + Some(node) => Some(node), + None => { + let doc_node: JSRef = NodeCast::from_ref(self); + let mut anchors = doc_node.traverse_preorder() + .filter(|node| node.is_anchor_element()); + anchors.find(|node| { + let elem: JSRef = ElementCast::to_ref(*node).unwrap(); + elem.get_attribute(ns!(""), "name").root().map_or(false, |attr| { + attr.deref().value().as_slice() == fragid.as_slice() + }) + }).map(|node| Temporary::from_rooted(ElementCast::to_ref(node).unwrap())) + } + } + } } impl Document { diff --git a/components/script/page.rs b/components/script/page.rs index b25be6122bbd..f2e46c165abe 100644 --- a/components/script/page.rs +++ b/components/script/page.rs @@ -2,15 +2,13 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use dom::attr::AttrHelpers; use dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods; -use dom::bindings::codegen::InheritTypes::{NodeCast, ElementCast}; -use dom::bindings::js::{MutNullableJS, JS, JSRef, Temporary}; -use dom::bindings::js::OptionalRootable; +use dom::bindings::codegen::InheritTypes::NodeCast; +use dom::bindings::js::{JS, JSRef, Temporary, OptionalRootable}; use dom::bindings::trace::{Traceable, Untraceable}; use dom::bindings::utils::GlobalStaticData; use dom::document::{Document, DocumentHelpers}; -use dom::element::{Element, AttributeHandlers}; +use dom::element::Element; use dom::node::{Node, NodeHelpers}; use dom::window::Window; use layout_interface::{DocumentDamage, ReflowForDisplay}; @@ -30,14 +28,12 @@ use servo_net::resource_task::ResourceTask; use servo_util::str::DOMString; use std::cell::{Cell, RefCell, Ref, RefMut}; use std::comm::{channel, Receiver, Empty, Disconnected}; -use std::default::Default; use std::mem::replace; use std::rc::Rc; use url::Url; /// Encapsulates a handle to a frame and its associated layout information. #[jstraceable] -#[allow(unrooted_must_root)] // FIXME(#3543) should be must_root. pub struct Page { /// Pipeline id associated with this page. pub id: PipelineId, @@ -80,7 +76,7 @@ pub struct Page { pub resize_event: Untraceable>>, /// Pending scroll to fragment event, if any - pub fragment_node: MutNullableJS, + pub fragment_name: RefCell>, /// Associated resource task for use by DOM objects like XMLHttpRequest pub resource_task: Untraceable, @@ -154,7 +150,7 @@ impl Page { url: Untraceable::new(RefCell::new(None)), next_subpage_id: Traceable::new(Cell::new(SubpageId(0))), resize_event: Untraceable::new(Cell::new(None)), - fragment_node: Default::default(), + fragment_name: RefCell::new(None), last_reflow_id: Traceable::new(Cell::new(0)), resource_task: Untraceable::new(resource_task), constellation_chan: Untraceable::new(constellation_chan), @@ -393,20 +389,7 @@ impl Page { /// Attempt to find a named element in this page's document. pub fn find_fragment_node(&self, fragid: DOMString) -> Option> { let document = self.frame().as_ref().unwrap().document.root(); - match document.deref().GetElementById(fragid.to_string()) { - Some(node) => Some(node), - None => { - let doc_node: JSRef = NodeCast::from_ref(*document); - let mut anchors = doc_node.traverse_preorder() - .filter(|node| node.is_anchor_element()); - anchors.find(|node| { - let elem: JSRef = ElementCast::to_ref(*node).unwrap(); - elem.get_attribute(ns!(""), "name").root().map_or(false, |attr| { - attr.deref().value().as_slice() == fragid.as_slice() - }) - }).map(|node| Temporary::from_rooted(ElementCast::to_ref(node).unwrap())) - } - } + document.find_fragment_node(fragid) } pub fn hit_test(&self, point: &Point2D) -> Option { diff --git a/components/script/script_task.rs b/components/script/script_task.rs index a8c7901bc778..f6a0fdb332bc 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -12,8 +12,7 @@ use dom::bindings::codegen::InheritTypes::{EventTargetCast, NodeCast, EventCast, use dom::bindings::conversions; use dom::bindings::conversions::{FromJSValConvertible, Empty}; use dom::bindings::global; -use dom::bindings::js::{JS, JSRef, RootCollection, Temporary, OptionalSettable}; -use dom::bindings::js::OptionalRootable; +use dom::bindings::js::{JS, JSRef, RootCollection, Temporary, OptionalRootable}; use dom::bindings::trace::JSTraceable; use dom::bindings::utils::Reflectable; use dom::bindings::utils::{wrap_for_same_compartment, pre_wrap}; @@ -800,8 +799,6 @@ impl ScriptTask { document.deref().content_changed(); window.flush_layout(ReflowForDisplay); - let fragment = url.fragment.as_ref().map(|ref fragment| fragment.to_string()); - { // No more reflow required let mut page_url = page.mut_url(); @@ -841,7 +838,7 @@ impl ScriptTask { let wintarget: JSRef = EventTargetCast::from_ref(*window); let _ = wintarget.dispatch_event_with_target(Some(doctarget), *event); - page.fragment_node.assign(fragment.map_or(None, |fragid| page.find_fragment_node(fragid))); + *page.fragment_name.borrow_mut() = url.fragment.clone(); let ConstellationChan(ref chan) = self.constellation_chan; chan.send(LoadCompleteMsg(page.id, url)); @@ -877,8 +874,13 @@ impl ScriptTask { page.reflow(ReflowForDisplay, self.control_chan.clone(), &*self.compositor) } - let mut fragment_node = page.fragment_node.get(); - match fragment_node.take().map(|node| node.root()) { + let fragment_node = + page.fragment_name + .borrow_mut() + .take() + .and_then(|name| page.find_fragment_node(name)) + .root(); + match fragment_node { Some(node) => self.scroll_fragment_point(pipeline_id, *node), None => {} }