From 40b5c4586ed7aa8327b2ae1f058bdaf6202bb90e Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 7 Aug 2015 15:19:16 +0200 Subject: [PATCH 1/3] Push the url parsing out of Window::load_url. This will allow the two callers to decide on the base url independently. --- components/script/dom/document.rs | 8 ++++++-- components/script/dom/location.rs | 11 +++++++++-- components/script/dom/window.rs | 11 +++-------- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 4f4b43437e7..42e3891a9fd 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -86,7 +86,7 @@ use html5ever::tree_builder::{QuirksMode, NoQuirks, LimitedQuirks, Quirks}; use ipc_channel::ipc; use layout_interface::{LayoutChan, Msg}; use string_cache::{Atom, QualName}; -use url::Url; +use url::{Url, UrlParser}; use js::jsapi::{JSContext, JSObject, JSRuntime}; use num::ToPrimitive; @@ -466,7 +466,11 @@ impl<'a> DocumentHelpers<'a> for &'a Document { fn load_anchor_href(self, href: DOMString) { let window = self.window.root(); - window.r().load_url(href); + let base_url = window.get_url(); + let url = UrlParser::new().base_url(&base_url).parse(&href); + // FIXME: handle URL parse errors more gracefully. + let url = url.unwrap(); + window.load_url(url); } /// Attempt to find a named element in this page's document. diff --git a/components/script/dom/location.rs b/components/script/dom/location.rs index 1d9f291c057..73f7d6f1fe1 100644 --- a/components/script/dom/location.rs +++ b/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,14 @@ 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(); + let url = UrlParser::new().base_url(&base_url).parse(&url); + // FIXME: handle URL parse errors more gracefully. + let url = url.unwrap(); + window.load_url(url); } // https://url.spec.whatwg.org/#dom-urlutils-hash diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 38100d87d16..7bfdf4e46d4 100644 --- a/components/script/dom/window.rs +++ b/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) { - 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(); + fn load_url(self, url: Url) { match url.fragment { Some(fragment) => { self.script_chan.send(ScriptMsg::TriggerFragment(self.id, fragment)).unwrap(); From da88e9ad9fbea334ac356e9d752cdb1c385a4354 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 7 Aug 2015 15:23:08 +0200 Subject: [PATCH 2/3] Inline Document::load_anchor_href into its only caller. --- components/script/dom/document.rs | 12 +----------- components/script/dom/htmlanchorelement.rs | 10 +++++++++- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 42e3891a9fd..009378b00f2 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -86,7 +86,7 @@ use html5ever::tree_builder::{QuirksMode, NoQuirks, LimitedQuirks, Quirks}; use ipc_channel::ipc; use layout_interface::{LayoutChan, Msg}; use string_cache::{Atom, QualName}; -use url::{Url, UrlParser}; +use url::Url; use js::jsapi::{JSContext, JSObject, JSRuntime}; use num::ToPrimitive; @@ -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>; fn hit_test(self, point: &Point2D) -> Option; fn get_nodes_under_mouse(self, point: &Point2D) -> Vec; @@ -464,15 +463,6 @@ impl<'a> DocumentHelpers<'a> for &'a Document { } } - fn load_anchor_href(self, href: DOMString) { - let window = self.window.root(); - let base_url = window.get_url(); - let url = UrlParser::new().base_url(&base_url).parse(&href); - // FIXME: handle URL parse errors more gracefully. - let url = url.unwrap(); - window.load_url(url); - } - /// 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> { diff --git a/components/script/dom/htmlanchorelement.rs b/components/script/dom/htmlanchorelement.rs index bace18798ed..2774b74a60a 100644 --- a/components/script/dom/htmlanchorelement.rs +++ b/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 From 7e179d924525096aa1f7ee0db1c52f8e85533107 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 7 Aug 2015 15:47:55 +0200 Subject: [PATCH 3/3] Handle url parse errors in Location#assign more gracefully. --- components/script/dom/location.rs | 7 +++---- .../history/the-location-interface/location_assign.html | 6 ++++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/components/script/dom/location.rs b/components/script/dom/location.rs index 73f7d6f1fe1..69cfb4b7d5b 100644 --- a/components/script/dom/location.rs +++ b/components/script/dom/location.rs @@ -43,10 +43,9 @@ impl<'a> LocationMethods for &'a Location { // TODO: per spec, we should use the _API base URL_ specified by the // _entry settings object_. let base_url = window.get_url(); - let url = UrlParser::new().base_url(&base_url).parse(&url); - // FIXME: handle URL parse errors more gracefully. - let url = url.unwrap(); - window.load_url(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 diff --git a/tests/wpt/web-platform-tests/html/browsers/history/the-location-interface/location_assign.html b/tests/wpt/web-platform-tests/html/browsers/history/the-location-interface/location_assign.html index 6f7da1a5332..a2d6e0fb820 100644 --- a/tests/wpt/web-platform-tests/html/browsers/history/the-location-interface/location_assign.html +++ b/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");