Auto merge of #7075 - Ms2ger:load_url, r=dzbarsky

Improve code around Window::load_url.



<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7075)
<!-- Reviewable:end -->

components/script/dom/document.rs

@@ -242,7 +242,6 @@ pub trait DocumentHelpers<'a> {
     fn disarm_reflow_timeout(self);
     fn unregister_named_element(self, to_unregister: &Element, id: Atom);
     fn register_named_element(self, element: &Element, id: Atom);
-    fn load_anchor_href(self, href: DOMString);
     fn find_fragment_node(self, fragid: DOMString) -> Option<Root<Element>>;
     fn hit_test(self, point: &Point2D<f32>) -> Option<UntrustedNodeAddress>;
     fn get_nodes_under_mouse(self, point: &Point2D<f32>) -> Vec<UntrustedNodeAddress>;
@@ -465,11 +464,6 @@ impl<'a> DocumentHelpers<'a> for &'a Document {
         }
     }
 
-    fn load_anchor_href(self, href: DOMString) {
-        let window = self.window.root();
-        window.r().load_url(href);
-    }
-
     /// Attempt to find a named element in this page's document.
     /// https://html.spec.whatwg.org/multipage/#the-indicated-part-of-the-document
     fn find_fragment_node(self, fragid: DOMString) -> Option<Root<Element>> {

components/script/dom/htmlanchorelement.rs

@@ -29,6 +29,8 @@ use std::default::Default;
 use string_cache::Atom;
 use util::str::DOMString;
 
+use url::UrlParser;
+
 #[dom_struct]
 pub struct HTMLAnchorElement {
     htmlelement: HTMLElement,
@@ -155,7 +157,13 @@ impl<'a> Activatable for &'a HTMLAnchorElement {
             value.push_str(&suffix);
         }
         debug!("clicked on link to {}", value);
-        doc.r().load_anchor_href(value);
+
+        let window = doc.window();
+        let base_url = window.get_url();
+        let url = UrlParser::new().base_url(&base_url).parse(&value);
+        // FIXME: handle URL parse errors more gracefully.
+        let url = url.unwrap();
+        window.load_url(url);
     }
 
     //TODO:https://html.spec.whatwg.org/multipage/#the-a-element

components/script/dom/location.rs

@@ -13,7 +13,7 @@ use dom::window::Window;
 use dom::window::WindowHelpers;
 
 use util::str::DOMString;
-use url::Url;
+use url::{Url, UrlParser};
 
 #[dom_struct]
 pub struct Location {
@@ -39,7 +39,13 @@ impl Location {
 impl<'a> LocationMethods for &'a Location {
     // https://html.spec.whatwg.org/multipage/#dom-location-assign
     fn Assign(self, url: DOMString) {
-        self.window.root().r().load_url(url);
+        let window = self.window.root();
+        // TODO: per spec, we should use the _API base URL_ specified by the
+        //       _entry settings object_.
+        let base_url = window.get_url();
+        if let Ok(url) = UrlParser::new().base_url(&base_url).parse(&url) {
+            window.load_url(url);
+        }
     }
 
     // https://url.spec.whatwg.org/#dom-urlutils-hash

components/script/dom/window.rs

@@ -62,7 +62,7 @@ use js::jsapi::{JS_GC, JS_GetRuntime, JSAutoCompartment, JSAutoRequest};
 use js::rust::Runtime;
 use js::rust::CompileOptionsWrapper;
 use selectors::parser::PseudoElement;
-use url::{Url, UrlParser};
+use url::Url;
 
 use libc;
 use rustc_serialize::base64::{FromBase64, ToBase64, STANDARD};
@@ -594,7 +594,7 @@ impl<'a> WindowMethods for &'a Window {
 pub trait WindowHelpers {
     fn clear_js_runtime(self);
     fn init_browsing_context(self, doc: &Document, frame_element: Option<&Element>);
-    fn load_url(self, href: DOMString);
+    fn load_url(self, url: Url);
     fn handle_fire_timer(self, timer_id: TimerId);
     fn force_reflow(self, goal: ReflowGoal, query_type: ReflowQueryType, reason: ReflowReason);
     fn reflow(self, goal: ReflowGoal, query_type: ReflowQueryType, reason: ReflowReason);
@@ -890,12 +890,7 @@ impl<'a> WindowHelpers for &'a Window {
     }
 
     /// Commence a new URL load which will either replace this window or scroll to a fragment.
-    fn load_url(self, href: DOMString) {
+    fn load_url(self, url: Url) {
-        let base_url = self.get_url();
-        debug!("current page url is {}", base_url);
-        let url = UrlParser::new().base_url(&base_url).parse(&href);
-        // FIXME: handle URL parse errors more gracefully.
-        let url = url.unwrap();
         match url.fragment {
             Some(fragment) => {
                 self.script_chan.send(ScriptMsg::TriggerFragment(self.id, fragment)).unwrap();

tests/wpt/web-platform-tests/html/browsers/history/the-location-interface/location_assign.html

@@ -15,6 +15,12 @@
       assert_equals((href + "#x"), location.href, "location href");
 
     }, "location assign");
+
+    test(function () {
+      var href = location.href;
+      location.assign("http://:");
+      assert_equals(location.href, href);
+    }, "URL that fails to parse");
     </script>
   </body>
 </html>