auto merge of #3558 : Ms2ger/servo/fragment-node, r=jdm

components/script/dom/document.rs

@@ -171,6 +171,7 @@ pub trait DocumentHelpers<'a> {
     fn unregister_named_element(self, to_unregister: JSRef<Element>, id: Atom);
     fn register_named_element(self, element: JSRef<Element>, id: Atom);
     fn load_anchor_href(self, href: DOMString);
+    fn find_fragment_node(self, fragid: DOMString) -> Option<Temporary<Element>>;
 }
 
 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<Temporary<Element>> {
+        match self.GetElementById(fragid.clone()) {
+            Some(node) => Some(node),
+            None => {
+                let doc_node: JSRef<Node> = NodeCast::from_ref(self);
+                let mut anchors = doc_node.traverse_preorder()
+                                          .filter(|node| node.is_anchor_element());
+                anchors.find(|node| {
+                    let elem: JSRef<Element> = 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 {

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<Cell<Option<WindowSizeData>>>,
 
     /// Pending scroll to fragment event, if any
-    pub fragment_node: MutNullableJS<Element>,
+    pub fragment_name: RefCell<Option<String>>,
 
     /// Associated resource task for use by DOM objects like XMLHttpRequest
     pub resource_task: Untraceable<ResourceTask>,
@@ -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<Temporary<Element>> {
         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<Node> = NodeCast::from_ref(*document);
-                let mut anchors = doc_node.traverse_preorder()
-                                          .filter(|node| node.is_anchor_element());
-                anchors.find(|node| {
-                    let elem: JSRef<Element> = 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<f32>) -> Option<UntrustedNodeAddress> {

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<EventTarget> = 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 => {}
                     }