From fa3e9ab2444daab2c67b7e25fd6a5f88264b4ddb Mon Sep 17 00:00:00 2001 From: Daniel Johnson Date: Sun, 20 Aug 2017 14:50:02 -0700 Subject: [PATCH 1/8] "javascript:" urls: run in correct global Make some changes to javascript scheme url evaluation to bring it closer to https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigating-across-documents - Evaluate the js before the page load so that it happens in the correct `window` global. - If the result is not a string, the response should be a 204. This required saving some data in load_data, since it's not possible to modify the response at the point where we're evaluating the js. --- components/script/script_thread.rs | 124 +++++++++++++++++++---------- components/script_traits/lib.rs | 13 +++ 2 files changed, 93 insertions(+), 44 deletions(-) diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index d4911627a6f..e234805f0b6 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -87,8 +87,8 @@ use script_runtime::{CommonScriptMsg, ScriptChan, ScriptThreadEventCategory}; use script_runtime::{ScriptPort, StackRootTLS, get_reports, new_rt_and_cx}; use script_traits::{CompositorEvent, ConstellationControlMsg}; use script_traits::{DocumentActivity, DiscardBrowsingContext, EventResult}; -use script_traits::{InitialScriptState, LayoutMsg, LoadData, MouseButton, MouseEventType, MozBrowserEvent}; -use script_traits::{NewLayoutInfo, ScriptToConstellationChan, ScriptMsg, UpdatePipelineIdReason}; +use script_traits::{InitialScriptState, JsEvalResult, LayoutMsg, LoadData, MouseButton, MouseEventType}; +use script_traits::{MozBrowserEvent, NewLayoutInfo, ScriptToConstellationChan, ScriptMsg, UpdatePipelineIdReason}; use script_traits::{ScriptThreadFactory, TimerEvent, TimerSchedulerMsg, TimerSource}; use script_traits::{TouchEventType, TouchId, UntrustedNodeAddress, WindowSizeData, WindowSizeType}; use script_traits::CompositorEvent::{KeyEvent, MouseButtonEvent, MouseMoveEvent, ResizeEvent}; @@ -1506,7 +1506,7 @@ impl ScriptThread { load_data.url.clone(), origin); if load_data.url.as_str() == "about:blank" { - self.start_page_load_about_blank(new_load); + self.start_page_load_about_blank(new_load, load_data.js_eval_result); } else { self.pre_page_load(new_load, load_data); } @@ -1677,6 +1677,18 @@ impl ScriptThread { match idx { Some(idx) => { let load = self.incomplete_loads.borrow_mut().remove(idx); + + // https://html.spec.whatwg.org/multipage/#process-a-navigate-response + // 2. If response's status is 204 or 205, then abort these steps. + match metadata { + Some(Metadata { status: Some((204 ... 205, _)), .. }) => { + // TODO: This leaves the page in a broken state where you can't follow + // other links. Fix this. + return None; + }, + _ => () + }; + metadata.map(|meta| self.load(meta, load)) } None => { @@ -2116,39 +2128,7 @@ impl ScriptThread { // Notify devtools that a new script global exists. self.notify_devtools(document.Title(), final_url.clone(), (incomplete.pipeline_id, None)); - let is_javascript = incomplete.url.scheme() == "javascript"; - let parse_input = if is_javascript { - use url::percent_encoding::percent_decode; - - // Turn javascript: URL into JS code to eval, according to the steps in - // https://html.spec.whatwg.org/multipage/#javascript-protocol - - // This slice of the URL’s serialization is equivalent to (5.) to (7.): - // Start with the scheme data of the parsed URL; - // append question mark and query component, if any; - // append number sign and fragment component if any. - let encoded = &incomplete.url[Position::BeforePath..]; - - // Percent-decode (8.) and UTF-8 decode (9.) - let script_source = percent_decode(encoded.as_bytes()).decode_utf8_lossy(); - - // Script source is ready to be evaluated (11.) - unsafe { - let _ac = JSAutoCompartment::new(self.get_cx(), window.reflector().get_jsobject().get()); - rooted!(in(self.get_cx()) let mut jsval = UndefinedValue()); - window.upcast::().evaluate_js_on_global_with_result( - &script_source, jsval.handle_mut()); - let strval = DOMString::from_jsval(self.get_cx(), - jsval.handle(), - StringificationBehavior::Empty); - match strval { - Ok(ConversionResult::Success(s)) => s, - _ => DOMString::new(), - } - } - } else { - DOMString::new() - }; + let parse_input = DOMString::new(); document.set_https_state(metadata.https_state); @@ -2327,7 +2307,7 @@ impl ScriptThread { /// for the given pipeline (specifically the "navigate" algorithm). fn handle_navigate(&self, parent_pipeline_id: PipelineId, browsing_context_id: Option, - load_data: LoadData, + mut load_data: LoadData, replace: bool) { match browsing_context_id { Some(browsing_context_id) => { @@ -2335,8 +2315,56 @@ impl ScriptThread { if let Some(iframe) = iframe { iframe.navigate_or_reload_child_browsing_context(Some(load_data), NavigationType::Regular, replace); } + + // TODO: Test javascript: urls in iframes } None => { + let is_javascript = load_data.url.scheme() == "javascript"; + if is_javascript { + use url::percent_encoding::percent_decode; + + // Turn javascript: URL into JS code to eval, according to the steps in + // https://html.spec.whatwg.org/multipage/#javascript-protocol + + // This slice of the URL’s serialization is equivalent to (5.) to (7.): + // Start with the scheme data of the parsed URL; + // append question mark and query component, if any; + // append number sign and fragment component if any. + let encoded = &load_data.url[Position::BeforePath..]; + + // Percent-decode (8.) and UTF-8 decode (9.) + let script_source = percent_decode(encoded.as_bytes()).decode_utf8_lossy(); + + // Script source is ready to be evaluated (11.) + let window = self.documents.borrow().find_window(parent_pipeline_id); + if let Some(window) = window { + let _ac = JSAutoCompartment::new(self.get_cx(), window.reflector().get_jsobject().get()); + rooted!(in(self.get_cx()) let mut jsval = UndefinedValue()); + window.upcast::().evaluate_js_on_global_with_result( + &script_source, jsval.handle_mut()); + + load_data.js_eval_result = if jsval.get().is_string() { + unsafe { + let strval = DOMString::from_jsval(self.get_cx(), + jsval.handle(), + StringificationBehavior::Empty); + match strval { + Ok(ConversionResult::Success(s)) => { + Some(JsEvalResult::Ok(String::from(s).as_bytes().to_vec())) + }, + _ => None, + } + } + } else { + Some(JsEvalResult::NoContent) + }; + } + } + + if is_javascript { + load_data.url = ServoUrl::parse("about:blank").unwrap(); + } + self.script_sender .send((parent_pipeline_id, ScriptMsg::LoadUrl(load_data, replace))) .unwrap(); @@ -2375,7 +2403,7 @@ impl ScriptThread { /// argument until a notification is received that the fetch is complete. fn pre_page_load(&self, incomplete: InProgressLoad, load_data: LoadData) { let id = incomplete.pipeline_id.clone(); - let mut req_init = RequestInit { + let req_init = RequestInit { url: load_data.url.clone(), method: load_data.method, destination: Destination::Document, @@ -2391,10 +2419,6 @@ impl ScriptThread { .. RequestInit::default() }; - if req_init.url.scheme() == "javascript" { - req_init.url = ServoUrl::parse("about:blank").unwrap(); - } - let context = ParserContext::new(id, load_data.url); self.incomplete_parser_contexts.borrow_mut().push((id, context)); @@ -2434,7 +2458,7 @@ impl ScriptThread { /// Synchronously fetch `about:blank`. Stores the `InProgressLoad` /// argument until a notification is received that the fetch is complete. - fn start_page_load_about_blank(&self, incomplete: InProgressLoad) { + fn start_page_load_about_blank(&self, incomplete: InProgressLoad, js_eval_result: Option) { let id = incomplete.pipeline_id; self.incomplete_loads.borrow_mut().push(incomplete); @@ -2444,8 +2468,20 @@ impl ScriptThread { let mut meta = Metadata::default(url); meta.set_content_type(Some(&mime!(Text / Html))); + + // If this page load is the result of a javascript scheme url, map + // the evaluation result into a response. + let chunk = match js_eval_result { + Some(JsEvalResult::Ok(content)) => content, + Some(JsEvalResult::NoContent) => { + meta.status = Some((204, b"No Content".to_vec())); + vec![] + }, + None => vec![] + }; + context.process_response(Ok(FetchMetadata::Unfiltered(meta))); - context.process_response_chunk(vec![]); + context.process_response_chunk(chunk); context.process_response_eof(Ok(())); } diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs index 3b5a47a3ef7..9ad3acb9c21 100644 --- a/components/script_traits/lib.rs +++ b/components/script_traits/lib.rs @@ -146,12 +146,24 @@ pub struct LoadData { pub headers: Headers, /// The data. pub data: Option>, + /// The result of evaluating a javascript scheme url. + pub js_eval_result: Option, /// The referrer policy. pub referrer_policy: Option, /// The referrer URL. pub referrer_url: Option, } +/// The result of evaluating a javascript scheme url. +#[derive(Clone, Debug, Deserialize, Serialize)] +pub enum JsEvalResult { + /// The js evaluation had a non-string result, 204 status code. + /// https://html.spec.whatwg.org/multipage/#navigate 12.11 + NoContent, + /// The js evaluation had a string result. + Ok(Vec) +} + impl LoadData { /// Create a new `LoadData` object. pub fn new(url: ServoUrl, @@ -165,6 +177,7 @@ impl LoadData { method: Method::Get, headers: Headers::new(), data: None, + js_eval_result: None, referrer_policy: referrer_policy, referrer_url: referrer_url, } From af41769d70e134fa7013127604e8ecbf7ace24a5 Mon Sep 17 00:00:00 2001 From: Daniel Johnson Date: Mon, 29 May 2017 13:04:35 -0700 Subject: [PATCH 2/8] "javascript:" urls: add web-platform-test --- tests/wpt/metadata/MANIFEST.json | 10 +++++++ .../url/a-element-href-javascript.html | 28 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 tests/wpt/web-platform-tests/url/a-element-href-javascript.html diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json index 45a49991d1e..b89ab624698 100644 --- a/tests/wpt/metadata/MANIFEST.json +++ b/tests/wpt/metadata/MANIFEST.json @@ -359897,6 +359897,12 @@ {} ] ], + "url/a-element-href-javascript.html": [ + [ + "/url/a-element-href-javascript.html", + {} + ] + ], "url/a-element-origin-xhtml.xhtml": [ [ "/url/a-element-origin-xhtml.xhtml", @@ -600978,6 +600984,10 @@ "3dacc2783865ba292f20b72bc4c3942de521d9b0", "support" ], + "url/a-element-href-javascript.html": [ + "567d16dde294a2f3e75fdb8139d1af9a8c73e0fc", + "testharness" + ], "url/a-element-origin-xhtml.xhtml": [ "56019fd2d3870324ba412e3e0c602bad3b90ef49", "testharness" diff --git a/tests/wpt/web-platform-tests/url/a-element-href-javascript.html b/tests/wpt/web-platform-tests/url/a-element-href-javascript.html new file mode 100644 index 00000000000..628e411be1f --- /dev/null +++ b/tests/wpt/web-platform-tests/url/a-element-href-javascript.html @@ -0,0 +1,28 @@ + + + + + + + +link + + From ff786a050a9b8c8f526130e4bca9b2208f7998c4 Mon Sep 17 00:00:00 2001 From: Daniel Johnson Date: Sun, 27 Aug 2017 21:42:14 -0700 Subject: [PATCH 3/8] "javascript:" urls: clean up after aborting a page load Problem: After aborting on a 204 or 205 status code, you could no longer follow links on the page. Cause: `constellation.rs` ignores new LoadUrl requests since the aborted one is in its `pending_changes` list. Solution: Send a message to constellation that lets it clean up its `pending_changes` list. --- components/constellation/constellation.rs | 16 ++++++++++++++++ components/script/script_thread.rs | 8 ++++---- components/script_traits/script_msg.rs | 2 ++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index e35ad261864..fa58b1f7aed 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -1055,6 +1055,10 @@ impl Constellation debug!("constellation got URL load message from script"); self.handle_load_url_msg(source_top_ctx_id, source_pipeline_id, load_data, replace); } + FromScriptMsg::AbortLoadUrl => { + debug!("constellation got URL load message from script"); + self.handle_abort_load_url_msg(source_pipeline_id); + } // A page loaded has completed all parsing, script, and reflow messages have been sent. FromScriptMsg::LoadComplete => { debug!("constellation got load complete message"); @@ -1874,6 +1878,18 @@ impl Constellation } } + fn handle_abort_load_url_msg(&mut self, new_pipeline_id: PipelineId) { + let pending_index = self.pending_changes.iter().rposition(|change| { + change.new_pipeline_id == new_pipeline_id + }); + + // If it is found, remove it from the pending changes. + if let Some(pending_index) = pending_index { + self.pending_changes.remove(pending_index); + self.close_pipeline(new_pipeline_id, DiscardBrowsingContext::No, ExitPipelineMode::Normal); + } + } + fn handle_load_start_msg(&mut self, top_level_browsing_context_id: TopLevelBrowsingContextId, pipeline_id: PipelineId) { if self.pipelines.get(&pipeline_id).and_then(|p| p.parent_info).is_none() { diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index e234805f0b6..dc19687fb06 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -1676,19 +1676,19 @@ impl ScriptThread { // the pipeline exited before the page load completed. match idx { Some(idx) => { - let load = self.incomplete_loads.borrow_mut().remove(idx); - // https://html.spec.whatwg.org/multipage/#process-a-navigate-response // 2. If response's status is 204 or 205, then abort these steps. match metadata { Some(Metadata { status: Some((204 ... 205, _)), .. }) => { - // TODO: This leaves the page in a broken state where you can't follow - // other links. Fix this. + self.script_sender + .send((id.clone(), ScriptMsg::AbortLoadUrl)) + .unwrap(); return None; }, _ => () }; + let load = self.incomplete_loads.borrow_mut().remove(idx); metadata.map(|meta| self.load(meta, load)) } None => { diff --git a/components/script_traits/script_msg.rs b/components/script_traits/script_msg.rs index 2d449e986a4..b4293230d14 100644 --- a/components/script_traits/script_msg.rs +++ b/components/script_traits/script_msg.rs @@ -99,6 +99,8 @@ pub enum ScriptMsg { /// A new load has been requested, with an option to replace the current entry once loaded /// instead of adding a new entry. LoadUrl(LoadData, bool), + /// Abort loading after sending a LoadUrl message. + AbortLoadUrl, /// Post a message to the currently active window of a given browsing context. PostMessage(BrowsingContextId, Option, Vec), /// Dispatch a mozbrowser event to the parent of a mozbrowser iframe. From fc23cb1a6306e830e622b93e6eccc13cf66bd0a4 Mon Sep 17 00:00:00 2001 From: Daniel Johnson Date: Sun, 27 Aug 2017 23:17:03 -0700 Subject: [PATCH 4/8] "javascript:" urls: move test to correct location - move test to correct directory - update it to be more concise I also confirmed that this test passes in Firefox. --- tests/wpt/metadata/MANIFEST.json | 20 ++++++------- .../javascript-url-global-scope.html | 16 +++++++++++ .../url/a-element-href-javascript.html | 28 ------------------- 3 files changed, 26 insertions(+), 38 deletions(-) create mode 100644 tests/wpt/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html delete mode 100644 tests/wpt/web-platform-tests/url/a-element-href-javascript.html diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json index b89ab624698..bba8096707e 100644 --- a/tests/wpt/metadata/MANIFEST.json +++ b/tests/wpt/metadata/MANIFEST.json @@ -326433,6 +326433,12 @@ {} ] ], + "html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html": [ + [ + "/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html", + {} + ] + ], "html/browsers/browsing-the-web/navigating-across-documents/javascript-url-query-fragment-components.html": [ [ "/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-query-fragment-components.html", @@ -359897,12 +359903,6 @@ {} ] ], - "url/a-element-href-javascript.html": [ - [ - "/url/a-element-href-javascript.html", - {} - ] - ], "url/a-element-origin-xhtml.xhtml": [ [ "/url/a-element-origin-xhtml.xhtml", @@ -559904,6 +559904,10 @@ "3842ac825b9fb33d0d95ef99f77c8c7d02a88e9a", "support" ], + "html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html": [ + "d678c54e2c20d5f240fd68790ea4e03512db2c8a", + "testharness" + ], "html/browsers/browsing-the-web/navigating-across-documents/javascript-url-query-fragment-components.html": [ "1278f37be250f761f84bf352ebff8ed4c8a04e96", "testharness" @@ -600984,10 +600988,6 @@ "3dacc2783865ba292f20b72bc4c3942de521d9b0", "support" ], - "url/a-element-href-javascript.html": [ - "567d16dde294a2f3e75fdb8139d1af9a8c73e0fc", - "testharness" - ], "url/a-element-origin-xhtml.xhtml": [ "56019fd2d3870324ba412e3e0c602bad3b90ef49", "testharness" diff --git a/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html new file mode 100644 index 00000000000..d3dd38ebed0 --- /dev/null +++ b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html @@ -0,0 +1,16 @@ + + + + + +link + + diff --git a/tests/wpt/web-platform-tests/url/a-element-href-javascript.html b/tests/wpt/web-platform-tests/url/a-element-href-javascript.html deleted file mode 100644 index 628e411be1f..00000000000 --- a/tests/wpt/web-platform-tests/url/a-element-href-javascript.html +++ /dev/null @@ -1,28 +0,0 @@ - - - - - - - -link - - From 5d28dd64d929ce471a7889a4bdddf8d1acb61184 Mon Sep 17 00:00:00 2001 From: Daniel Johnson Date: Sun, 27 Aug 2017 23:18:19 -0700 Subject: [PATCH 5/8] "javascript:" urls: clean up js evaluation code - move it to its own function - move the `url = "about:blank" code into the same block - move the `use` statement to the top of the file --- components/script/script_thread.rs | 98 +++++++++++++++--------------- 1 file changed, 50 insertions(+), 48 deletions(-) diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index dc19687fb06..d3b5a81683b 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -119,6 +119,7 @@ use task_source::performance_timeline::{PerformanceTimelineTask, PerformanceTime use task_source::user_interaction::{UserInteractionTask, UserInteractionTaskSource}; use time::{get_time, precise_time_ns, Tm}; use url::Position; +use url::percent_encoding::percent_decode; use webdriver_handlers; use webvr_traits::{WebVREvent, WebVRMsg}; @@ -2309,62 +2310,19 @@ impl ScriptThread { browsing_context_id: Option, mut load_data: LoadData, replace: bool) { + let is_javascript = load_data.url.scheme() == "javascript"; + if is_javascript { + self.eval_js_url(parent_pipeline_id, &mut load_data); + } + match browsing_context_id { Some(browsing_context_id) => { let iframe = self.documents.borrow().find_iframe(parent_pipeline_id, browsing_context_id); if let Some(iframe) = iframe { iframe.navigate_or_reload_child_browsing_context(Some(load_data), NavigationType::Regular, replace); } - - // TODO: Test javascript: urls in iframes } None => { - let is_javascript = load_data.url.scheme() == "javascript"; - if is_javascript { - use url::percent_encoding::percent_decode; - - // Turn javascript: URL into JS code to eval, according to the steps in - // https://html.spec.whatwg.org/multipage/#javascript-protocol - - // This slice of the URL’s serialization is equivalent to (5.) to (7.): - // Start with the scheme data of the parsed URL; - // append question mark and query component, if any; - // append number sign and fragment component if any. - let encoded = &load_data.url[Position::BeforePath..]; - - // Percent-decode (8.) and UTF-8 decode (9.) - let script_source = percent_decode(encoded.as_bytes()).decode_utf8_lossy(); - - // Script source is ready to be evaluated (11.) - let window = self.documents.borrow().find_window(parent_pipeline_id); - if let Some(window) = window { - let _ac = JSAutoCompartment::new(self.get_cx(), window.reflector().get_jsobject().get()); - rooted!(in(self.get_cx()) let mut jsval = UndefinedValue()); - window.upcast::().evaluate_js_on_global_with_result( - &script_source, jsval.handle_mut()); - - load_data.js_eval_result = if jsval.get().is_string() { - unsafe { - let strval = DOMString::from_jsval(self.get_cx(), - jsval.handle(), - StringificationBehavior::Empty); - match strval { - Ok(ConversionResult::Success(s)) => { - Some(JsEvalResult::Ok(String::from(s).as_bytes().to_vec())) - }, - _ => None, - } - } - } else { - Some(JsEvalResult::NoContent) - }; - } - } - - if is_javascript { - load_data.url = ServoUrl::parse("about:blank").unwrap(); - } - self.script_sender .send((parent_pipeline_id, ScriptMsg::LoadUrl(load_data, replace))) .unwrap(); @@ -2372,6 +2330,50 @@ impl ScriptThread { } } + fn eval_js_url(&self, pipeline_id: PipelineId, load_data: &mut LoadData) { + { + // Turn javascript: URL into JS code to eval, according to the steps in + // https://html.spec.whatwg.org/multipage/#javascript-protocol + + // This slice of the URL’s serialization is equivalent to (5.) to (7.): + // Start with the scheme data of the parsed URL; + // append question mark and query component, if any; + // append number sign and fragment component if any. + let encoded = &load_data.url[Position::BeforePath..]; + + // Percent-decode (8.) and UTF-8 decode (9.) + let script_source = percent_decode(encoded.as_bytes()).decode_utf8_lossy(); + + // Script source is ready to be evaluated (11.) + let window = self.documents.borrow().find_window(pipeline_id); + + if let Some(window) = window { + let _ac = JSAutoCompartment::new(self.get_cx(), window.reflector().get_jsobject().get()); + rooted!(in(self.get_cx()) let mut jsval = UndefinedValue()); + window.upcast::().evaluate_js_on_global_with_result( + &script_source, jsval.handle_mut()); + + load_data.js_eval_result = if jsval.get().is_string() { + unsafe { + let strval = DOMString::from_jsval(self.get_cx(), + jsval.handle(), + StringificationBehavior::Empty); + match strval { + Ok(ConversionResult::Success(s)) => { + Some(JsEvalResult::Ok(String::from(s).as_bytes().to_vec())) + }, + _ => None, + } + } + } else { + Some(JsEvalResult::NoContent) + }; + } + }; + + load_data.url = ServoUrl::parse("about:blank").unwrap(); + } + fn handle_resize_event(&self, pipeline_id: PipelineId, new_size: WindowSizeData, size_type: WindowSizeType) { let document = match { self.documents.borrow().find_document(pipeline_id) } { Some(document) => document, From 6ae6031468662161280a4353da9c0c23368cd83d Mon Sep 17 00:00:00 2001 From: Daniel Johnson Date: Mon, 28 Aug 2017 22:22:39 -0700 Subject: [PATCH 6/8] "javascript:" urls: evaluate in iframe src attribute - generalize the eval_js_url function so it can be called from multiple places. - call it in htmliframeelement. - if the js eval results in a non-string result, then it won't navigate to a new page, so don't block on the new page loading. --- components/constellation/constellation.rs | 2 +- components/script/dom/htmliframeelement.rs | 21 ++++++++-- components/script/script_thread.rs | 46 +++++++++++----------- 3 files changed, 41 insertions(+), 28 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index fa58b1f7aed..6a6f4f5ce26 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -1056,7 +1056,7 @@ impl Constellation self.handle_load_url_msg(source_top_ctx_id, source_pipeline_id, load_data, replace); } FromScriptMsg::AbortLoadUrl => { - debug!("constellation got URL load message from script"); + debug!("constellation got abort URL load message from script"); self.handle_abort_load_url_msg(source_pipeline_id); } // A page loaded has completed all parsing, script, and reflow messages have been sent. diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs index 47a1d2adfcb..0b6102ab165 100644 --- a/components/script/dom/htmliframeelement.rs +++ b/components/script/dom/htmliframeelement.rs @@ -44,7 +44,7 @@ use msg::constellation_msg::{FrameType, BrowsingContextId, PipelineId, TopLevelB use net_traits::response::HttpsState; use script_layout_interface::message::ReflowQueryType; use script_thread::{ScriptThread, Runnable}; -use script_traits::{IFrameLoadInfo, IFrameLoadInfoWithData, LoadData, UpdatePipelineIdReason}; +use script_traits::{IFrameLoadInfo, IFrameLoadInfoWithData, JsEvalResult, LoadData, UpdatePipelineIdReason}; use script_traits::{MozBrowserEvent, NewLayoutInfo, ScriptMsg}; use script_traits::IFrameSandboxState::{IFrameSandboxed, IFrameUnsandboxed}; use servo_atoms::Atom; @@ -114,7 +114,7 @@ impl HTMLIFrameElement { } pub fn navigate_or_reload_child_browsing_context(&self, - load_data: Option, + mut load_data: Option, nav_type: NavigationType, replace: bool) { let sandboxed = if self.is_sandboxed() { @@ -140,11 +140,26 @@ impl HTMLIFrameElement { // document; the new navigation will continue blocking it. LoadBlocker::terminate(&mut load_blocker); + if let Some(ref mut load_data) = load_data { + let is_javascript = load_data.url.scheme() == "javascript"; + if is_javascript { + let window_proxy = self.GetContentWindow(); + if let Some(window_proxy) = window_proxy { + ScriptThread::eval_js_url(&window_proxy.global(), load_data); + } + } + } + //TODO(#9592): Deal with the case where an iframe is being reloaded so url is None. // The iframe should always have access to the nested context's active // document URL through the browsing context. if let Some(ref load_data) = load_data { - *load_blocker = Some(LoadBlocker::new(&*document, LoadType::Subframe(load_data.url.clone()))); + match load_data.js_eval_result { + Some(JsEvalResult::NoContent) => (), + _ => { + *load_blocker = Some(LoadBlocker::new(&*document, LoadType::Subframe(load_data.url.clone()))); + } + }; } let window = window_from_node(self); diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index d3b5a81683b..6ca5b05bc67 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -2312,7 +2312,10 @@ impl ScriptThread { replace: bool) { let is_javascript = load_data.url.scheme() == "javascript"; if is_javascript { - self.eval_js_url(parent_pipeline_id, &mut load_data); + let window = self.documents.borrow().find_window(parent_pipeline_id); + if let Some(window) = window { + ScriptThread::eval_js_url(window.upcast::(), &mut load_data); + } } match browsing_context_id { @@ -2330,7 +2333,7 @@ impl ScriptThread { } } - fn eval_js_url(&self, pipeline_id: PipelineId, load_data: &mut LoadData) { + pub fn eval_js_url(global_scope: &GlobalScope, load_data: &mut LoadData) { { // Turn javascript: URL into JS code to eval, according to the steps in // https://html.spec.whatwg.org/multipage/#javascript-protocol @@ -2345,30 +2348,25 @@ impl ScriptThread { let script_source = percent_decode(encoded.as_bytes()).decode_utf8_lossy(); // Script source is ready to be evaluated (11.) - let window = self.documents.borrow().find_window(pipeline_id); + let _ac = JSAutoCompartment::new(global_scope.get_cx(), global_scope.reflector().get_jsobject().get()); + rooted!(in(global_scope.get_cx()) let mut jsval = UndefinedValue()); + global_scope.evaluate_js_on_global_with_result(&script_source, jsval.handle_mut()); - if let Some(window) = window { - let _ac = JSAutoCompartment::new(self.get_cx(), window.reflector().get_jsobject().get()); - rooted!(in(self.get_cx()) let mut jsval = UndefinedValue()); - window.upcast::().evaluate_js_on_global_with_result( - &script_source, jsval.handle_mut()); - - load_data.js_eval_result = if jsval.get().is_string() { - unsafe { - let strval = DOMString::from_jsval(self.get_cx(), - jsval.handle(), - StringificationBehavior::Empty); - match strval { - Ok(ConversionResult::Success(s)) => { - Some(JsEvalResult::Ok(String::from(s).as_bytes().to_vec())) - }, - _ => None, - } + load_data.js_eval_result = if jsval.get().is_string() { + unsafe { + let strval = DOMString::from_jsval(global_scope.get_cx(), + jsval.handle(), + StringificationBehavior::Empty); + match strval { + Ok(ConversionResult::Success(s)) => { + Some(JsEvalResult::Ok(String::from(s).as_bytes().to_vec())) + }, + _ => None, } - } else { - Some(JsEvalResult::NoContent) - }; - } + } + } else { + Some(JsEvalResult::NoContent) + }; }; load_data.url = ServoUrl::parse("about:blank").unwrap(); From 709cd3a0b636f601c0a4ca606c241500eca13789 Mon Sep 17 00:00:00 2001 From: Daniel Johnson Date: Thu, 7 Sep 2017 17:03:41 -0700 Subject: [PATCH 7/8] "javascript:" urls: remove unnecessary block Remove this block and unindent the code one level. Doing this required cloning `load_data.url` so that we could later mutate `load_data.url` in the same block. --- components/script/script_thread.rs | 54 ++++++++++++++---------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 6ca5b05bc67..ccc3f70146a 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -2334,39 +2334,37 @@ impl ScriptThread { } pub fn eval_js_url(global_scope: &GlobalScope, load_data: &mut LoadData) { - { - // Turn javascript: URL into JS code to eval, according to the steps in - // https://html.spec.whatwg.org/multipage/#javascript-protocol + // Turn javascript: URL into JS code to eval, according to the steps in + // https://html.spec.whatwg.org/multipage/#javascript-protocol - // This slice of the URL’s serialization is equivalent to (5.) to (7.): - // Start with the scheme data of the parsed URL; - // append question mark and query component, if any; - // append number sign and fragment component if any. - let encoded = &load_data.url[Position::BeforePath..]; + // This slice of the URL’s serialization is equivalent to (5.) to (7.): + // Start with the scheme data of the parsed URL; + // append question mark and query component, if any; + // append number sign and fragment component if any. + let encoded = &load_data.url.clone()[Position::BeforePath..]; - // Percent-decode (8.) and UTF-8 decode (9.) - let script_source = percent_decode(encoded.as_bytes()).decode_utf8_lossy(); + // Percent-decode (8.) and UTF-8 decode (9.) + let script_source = percent_decode(encoded.as_bytes()).decode_utf8_lossy(); - // Script source is ready to be evaluated (11.) - let _ac = JSAutoCompartment::new(global_scope.get_cx(), global_scope.reflector().get_jsobject().get()); - rooted!(in(global_scope.get_cx()) let mut jsval = UndefinedValue()); - global_scope.evaluate_js_on_global_with_result(&script_source, jsval.handle_mut()); + // Script source is ready to be evaluated (11.) + let _ac = JSAutoCompartment::new(global_scope.get_cx(), global_scope.reflector().get_jsobject().get()); + rooted!(in(global_scope.get_cx()) let mut jsval = UndefinedValue()); + global_scope.evaluate_js_on_global_with_result(&script_source, jsval.handle_mut()); - load_data.js_eval_result = if jsval.get().is_string() { - unsafe { - let strval = DOMString::from_jsval(global_scope.get_cx(), - jsval.handle(), - StringificationBehavior::Empty); - match strval { - Ok(ConversionResult::Success(s)) => { - Some(JsEvalResult::Ok(String::from(s).as_bytes().to_vec())) - }, - _ => None, - } + load_data.js_eval_result = if jsval.get().is_string() { + unsafe { + let strval = DOMString::from_jsval(global_scope.get_cx(), + jsval.handle(), + StringificationBehavior::Empty); + match strval { + Ok(ConversionResult::Success(s)) => { + Some(JsEvalResult::Ok(String::from(s).as_bytes().to_vec())) + }, + _ => None, } - } else { - Some(JsEvalResult::NoContent) - }; + } + } else { + Some(JsEvalResult::NoContent) }; load_data.url = ServoUrl::parse("about:blank").unwrap(); From 452db052ff385e7abd112dec3b8147199b5d5b5f Mon Sep 17 00:00:00 2001 From: Daniel Johnson Date: Sat, 9 Sep 2017 00:38:01 -0700 Subject: [PATCH 8/8] "javascript:" urls: update test expectations --- .../XMLHttpRequest/open-url-javascript-window-2.htm.ini | 3 +-- .../XMLHttpRequest/open-url-javascript-window.htm.ini | 6 ------ .../contentType/contenttype_javascripturi.html.ini | 5 ----- .../navigating-across-documents/013.html.ini | 5 ----- .../navigating-across-documents/014.html.ini | 5 ----- .../navigating-across-documents/015.html.ini | 5 ----- .../javascript-url-query-fragment-components.html.ini | 5 ----- .../dom-tree-accessors/Document.currentScript.html.ini | 3 --- .../submission/Opera/script_scheduling/028.html.ini | 5 ----- .../submission/Opera/script_scheduling/029.html.ini | 5 ----- 10 files changed, 1 insertion(+), 46 deletions(-) delete mode 100644 tests/wpt/metadata/XMLHttpRequest/open-url-javascript-window.htm.ini delete mode 100644 tests/wpt/metadata/dom/nodes/Document-contentType/contentType/contenttype_javascripturi.html.ini delete mode 100644 tests/wpt/metadata/html/browsers/browsing-the-web/navigating-across-documents/013.html.ini delete mode 100644 tests/wpt/metadata/html/browsers/browsing-the-web/navigating-across-documents/014.html.ini delete mode 100644 tests/wpt/metadata/html/browsers/browsing-the-web/navigating-across-documents/015.html.ini delete mode 100644 tests/wpt/metadata/html/browsers/browsing-the-web/navigating-across-documents/javascript-url-query-fragment-components.html.ini delete mode 100644 tests/wpt/metadata/old-tests/submission/Opera/script_scheduling/028.html.ini delete mode 100644 tests/wpt/metadata/old-tests/submission/Opera/script_scheduling/029.html.ini diff --git a/tests/wpt/metadata/XMLHttpRequest/open-url-javascript-window-2.htm.ini b/tests/wpt/metadata/XMLHttpRequest/open-url-javascript-window-2.htm.ini index 8f9dc0ea819..ec4a12160d1 100644 --- a/tests/wpt/metadata/XMLHttpRequest/open-url-javascript-window-2.htm.ini +++ b/tests/wpt/metadata/XMLHttpRequest/open-url-javascript-window-2.htm.ini @@ -1,6 +1,5 @@ [open-url-javascript-window-2.htm] type: testharness - expected: TIMEOUT [XMLHttpRequest: open() - resolving URLs (javascript: