From 2639e36c784179413f2324122ec1414949d94d4c Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 25 Feb 2014 19:04:47 +0100 Subject: [PATCH 1/4] Remove the image loading workaround from the parser. --- src/components/script/dom/htmlimageelement.rs | 2 +- src/components/script/html/hubbub_html_parser.rs | 13 ++----------- src/components/script/script_task.rs | 1 - 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/components/script/dom/htmlimageelement.rs b/src/components/script/dom/htmlimageelement.rs index d2304944eeb..a049d649763 100644 --- a/src/components/script/dom/htmlimageelement.rs +++ b/src/components/script/dom/htmlimageelement.rs @@ -66,7 +66,7 @@ impl HTMLImageElement { impl HTMLImageElement { /// Makes the local `image` member match the status of the `src` attribute and starts /// prefetching the image. This method must be called after `src` is changed. - pub fn update_image(&mut self, image_cache: ImageCacheTask, url: Option) { + fn update_image(&mut self, image_cache: ImageCacheTask, url: Option) { let elem = &mut self.htmlelement.element; let src_opt = elem.get_attribute(Null, "src").map(|x| x.get().Value()); match src_opt { diff --git a/src/components/script/html/hubbub_html_parser.rs b/src/components/script/html/hubbub_html_parser.rs index cf692d43dfd..26d1faacdbc 100644 --- a/src/components/script/html/hubbub_html_parser.rs +++ b/src/components/script/html/hubbub_html_parser.rs @@ -3,11 +3,11 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use dom::bindings::codegen::InheritTypes::{NodeBase, NodeCast, TextCast, ElementCast}; -use dom::bindings::codegen::InheritTypes::{HTMLIFrameElementCast, HTMLImageElementCast}; +use dom::bindings::codegen::InheritTypes::HTMLIFrameElementCast; use dom::bindings::js::JS; use dom::bindings::utils::Reflectable; use dom::document::Document; -use dom::element::{HTMLLinkElementTypeId, HTMLIframeElementTypeId, HTMLImageElementTypeId}; +use dom::element::{HTMLLinkElementTypeId, HTMLIframeElementTypeId}; use dom::htmlelement::HTMLElement; use dom::htmlheadingelement::{Heading1, Heading2, Heading3, Heading4, Heading5, Heading6}; use dom::htmliframeelement::IFrameSize; @@ -21,7 +21,6 @@ use extra::url::Url; use hubbub::hubbub; use js::jsapi::JSContext; use servo_msg::constellation_msg::SubpageId; -use servo_net::image_cache_task::ImageCacheTask; use servo_net::resource_task::{Load, Payload, Done, ResourceTask, load_whole_resource}; use servo_util::namespace::Null; use servo_util::str::DOMString; @@ -251,7 +250,6 @@ pub fn parse_html(cx: *JSContext, document: &mut JS, url: Url, resource_task: ResourceTask, - image_cache_task: ImageCacheTask, next_subpage_id: SubpageId) -> HtmlParserResult { debug!("Hubbub: parsing {:?}", url); @@ -382,13 +380,6 @@ pub fn parse_html(cx: *JSContext, sandboxed))); } } - - //FIXME: This should be taken care of by set_attr, but we don't have - // access to a window so HTMLImageElement::AfterSetAttr bails. - ElementNodeTypeId(HTMLImageElementTypeId) => { - let mut image_element: JS = HTMLImageElementCast::to(&element); - image_element.get_mut().update_image(image_cache_task.clone(), Some(url2.clone())); - } _ => {} } diff --git a/src/components/script/script_task.rs b/src/components/script/script_task.rs index a9a284ad21f..01539dcd128 100644 --- a/src/components/script/script_task.rs +++ b/src/components/script/script_task.rs @@ -722,7 +722,6 @@ impl ScriptTask { &mut document, url.clone(), self.resource_task.clone(), - self.image_cache_task.clone(), page.next_subpage_id.clone()); let HtmlParserResult { From e834e532c51a92d31f3e3feac6f85ce6d5b9a5df Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 25 Feb 2014 19:12:34 +0100 Subject: [PATCH 2/4] Fetch the image cache inside the update_image function. --- src/components/script/dom/htmlimageelement.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/script/dom/htmlimageelement.rs b/src/components/script/dom/htmlimageelement.rs index a049d649763..38996620b59 100644 --- a/src/components/script/dom/htmlimageelement.rs +++ b/src/components/script/dom/htmlimageelement.rs @@ -16,7 +16,6 @@ use extra::url::Url; use servo_util::geometry::to_px; use layout_interface::{ContentBoxQuery, ContentBoxResponse}; use servo_net::image_cache_task; -use servo_net::image_cache_task::ImageCacheTask; use servo_util::url::parse_url; use servo_util::namespace::Null; use servo_util::str::DOMString; @@ -66,8 +65,11 @@ impl HTMLImageElement { impl HTMLImageElement { /// Makes the local `image` member match the status of the `src` attribute and starts /// prefetching the image. This method must be called after `src` is changed. - fn update_image(&mut self, image_cache: ImageCacheTask, url: Option) { + fn update_image(&mut self, url: Option) { let elem = &mut self.htmlelement.element; + let document = elem.node.owner_doc(); + let window = document.get().window.get(); + let image_cache = &window.image_cache_task; let src_opt = elem.get_attribute(Null, "src").map(|x| x.get().Value()); match src_opt { None => {} @@ -90,7 +92,7 @@ impl HTMLImageElement { let document = self.htmlelement.element.node.owner_doc(); let window = document.get().window.get(); let url = window.page.url.as_ref().map(|&(ref url, _)| url.clone()); - self.update_image(window.image_cache_task.clone(), url); + self.update_image(url); } } @@ -99,9 +101,7 @@ impl HTMLImageElement { // This might not handle remove src attribute actually since // `self.update_image()` will see the missing src attribute and return early. if "src" == name { - let document = self.htmlelement.element.node.owner_doc(); - let window = document.get().window.get(); - self.update_image(window.image_cache_task.clone(), None); + self.update_image(None); } } From 9faf2c89e4483c29fa13c027ff12f793f188cf91 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 25 Feb 2014 19:17:58 +0100 Subject: [PATCH 3/4] Pass the attribute value to the update_image function. --- src/components/script/dom/htmlimageelement.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/components/script/dom/htmlimageelement.rs b/src/components/script/dom/htmlimageelement.rs index 38996620b59..61add6b1d7a 100644 --- a/src/components/script/dom/htmlimageelement.rs +++ b/src/components/script/dom/htmlimageelement.rs @@ -17,7 +17,6 @@ use servo_util::geometry::to_px; use layout_interface::{ContentBoxQuery, ContentBoxResponse}; use servo_net::image_cache_task; use servo_util::url::parse_url; -use servo_util::namespace::Null; use servo_util::str::DOMString; use extra::serialize::{Encoder, Encodable}; @@ -65,13 +64,12 @@ impl HTMLImageElement { impl HTMLImageElement { /// Makes the local `image` member match the status of the `src` attribute and starts /// prefetching the image. This method must be called after `src` is changed. - fn update_image(&mut self, url: Option) { + fn update_image(&mut self, value: Option, url: Option) { let elem = &mut self.htmlelement.element; let document = elem.node.owner_doc(); let window = document.get().window.get(); let image_cache = &window.image_cache_task; - let src_opt = elem.get_attribute(Null, "src").map(|x| x.get().Value()); - match src_opt { + match value { None => {} Some(src) => { let img_url = parse_url(src, url); @@ -87,12 +85,12 @@ impl HTMLImageElement { } } - pub fn AfterSetAttr(&mut self, name: DOMString, _value: DOMString) { + pub fn AfterSetAttr(&mut self, name: DOMString, value: DOMString) { if "src" == name { let document = self.htmlelement.element.node.owner_doc(); let window = document.get().window.get(); let url = window.page.url.as_ref().map(|&(ref url, _)| url.clone()); - self.update_image(url); + self.update_image(Some(value), url); } } @@ -101,7 +99,7 @@ impl HTMLImageElement { // This might not handle remove src attribute actually since // `self.update_image()` will see the missing src attribute and return early. if "src" == name { - self.update_image(None); + self.update_image(None, None); } } From 938f6baf9ede61570d94c648d54a847a40bf56f4 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Tue, 25 Feb 2014 19:33:49 +0100 Subject: [PATCH 4/4] Handle removing the src attribute from an img element (fixes #1469). --- src/components/script/dom/htmlimageelement.rs | 7 +++---- src/test/ref/basic.list | 1 + src/test/ref/img_dynamic_remove.html | 5 +++++ src/test/ref/img_dynamic_remove_ref.html | 2 ++ 4 files changed, 11 insertions(+), 4 deletions(-) create mode 100644 src/test/ref/img_dynamic_remove.html create mode 100644 src/test/ref/img_dynamic_remove_ref.html diff --git a/src/components/script/dom/htmlimageelement.rs b/src/components/script/dom/htmlimageelement.rs index 61add6b1d7a..4f18597449c 100644 --- a/src/components/script/dom/htmlimageelement.rs +++ b/src/components/script/dom/htmlimageelement.rs @@ -70,7 +70,9 @@ impl HTMLImageElement { let window = document.get().window.get(); let image_cache = &window.image_cache_task; match value { - None => {} + None => { + self.extra.image = None; + } Some(src) => { let img_url = parse_url(src, url); self.extra.image = Some(img_url.clone()); @@ -95,9 +97,6 @@ impl HTMLImageElement { } pub fn AfterRemoveAttr(&mut self, name: DOMString) { - // FIXME (#1469): - // This might not handle remove src attribute actually since - // `self.update_image()` will see the missing src attribute and return early. if "src" == name { self.update_image(None, None); } diff --git a/src/test/ref/basic.list b/src/test/ref/basic.list index e3e7997a5ae..661974e2996 100644 --- a/src/test/ref/basic.list +++ b/src/test/ref/basic.list @@ -23,6 +23,7 @@ == font_size_percentage.html font_size_em_ref.html == position_fixed_a.html position_fixed_b.html == img_size_a.html img_size_b.html +== img_dynamic_remove.html img_dynamic_remove_ref.html == upper_id_attr.html upper_id_attr_ref.html # inline_border_a.html inline_border_b.html == anon_block_inherit_a.html anon_block_inherit_b.html diff --git a/src/test/ref/img_dynamic_remove.html b/src/test/ref/img_dynamic_remove.html new file mode 100644 index 00000000000..9400764fcd4 --- /dev/null +++ b/src/test/ref/img_dynamic_remove.html @@ -0,0 +1,5 @@ + + + diff --git a/src/test/ref/img_dynamic_remove_ref.html b/src/test/ref/img_dynamic_remove_ref.html new file mode 100644 index 00000000000..6976ec6f2f4 --- /dev/null +++ b/src/test/ref/img_dynamic_remove_ref.html @@ -0,0 +1,2 @@ + +