From 8159dac506f7fc24675270079d3286e86731bb5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 17 Jan 2017 00:21:37 +0100 Subject: [PATCH 1/5] dom: Refactor htmlanchorelement.rs layout query for an existing helper. --- components/script/dom/htmlanchorelement.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/components/script/dom/htmlanchorelement.rs b/components/script/dom/htmlanchorelement.rs index 3731b9d46de..ab237f702dd 100644 --- a/components/script/dom/htmlanchorelement.rs +++ b/components/script/dom/htmlanchorelement.rs @@ -21,7 +21,7 @@ use dom::eventtarget::EventTarget; use dom::htmlelement::HTMLElement; use dom::htmlimageelement::HTMLImageElement; use dom::mouseevent::MouseEvent; -use dom::node::{Node, document_from_node, window_from_node}; +use dom::node::{Node, document_from_node}; use dom::urlhelper::UrlHelper; use dom::virtualmethods::VirtualMethods; use html5ever_atoms::LocalName; @@ -544,8 +544,7 @@ impl Activatable for HTMLAnchorElement { if let Some(element) = target.downcast::() { if target.is::() && element.has_attribute(&local_name!("ismap")) { let target_node = element.upcast::(); - let rect = window_from_node(target_node).content_box_query( - target_node.to_trusted_node_address()); + let rect = target_node.bounding_content_box(); ismap_suffix = Some( format!("?{},{}", mouse_event.ClientX().to_f32().unwrap() - rect.origin.x.to_f32_px(), mouse_event.ClientY().to_f32().unwrap() - rect.origin.y.to_f32_px()) From 485fe874e8674e9a54b69e3f78f1bb9f43fdbbc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 17 Jan 2017 00:22:42 +0100 Subject: [PATCH 2/5] layout: Expose whether the element was rendered in content_box_query to script. But don't change the API yet. --- components/layout/query.rs | 9 +++------ components/layout_thread/lib.rs | 4 ++-- components/script/dom/node.rs | 6 +++++- components/script/dom/window.rs | 4 ++-- components/script_layout_interface/rpc.rs | 2 +- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/components/layout/query.rs b/components/layout/query.rs index 4fe98c05969..bd6088a1e05 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -54,7 +54,7 @@ pub struct LayoutThreadData { pub stylist: Arc, /// A queued response for the union of the content boxes of a node. - pub content_box_response: Rect, + pub content_box_response: Option>, /// A queued response for the content boxes of a node. pub content_boxes_response: Vec>, @@ -384,15 +384,12 @@ impl FragmentBorderBoxIterator for MarginRetrievingFragmentBorderBoxIterator { } pub fn process_content_box_request( - requested_node: N, layout_root: &mut Flow) -> Rect { + requested_node: N, layout_root: &mut Flow) -> Option> { // FIXME(pcwalton): This has not been updated to handle the stacking context relative // stuff. So the position is wrong in most cases. let mut iterator = UnioningFragmentBorderBoxIterator::new(requested_node.opaque()); sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); - match iterator.rect { - Some(rect) => rect, - None => Rect::zero() - } + iterator.rect } pub fn process_content_boxes_request(requested_node: N, layout_root: &mut Flow) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 6dc72bc86c3..c641a23dcd0 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -464,7 +464,7 @@ impl LayoutThread { constellation_chan: constellation_chan, display_list: None, stylist: stylist, - content_box_response: Rect::zero(), + content_box_response: None, content_boxes_response: Vec::new(), client_rect_response: Rect::zero(), hit_test_response: (None, false), @@ -1012,7 +1012,7 @@ impl LayoutThread { debug!("layout: No root node: bailing"); match data.query_type { ReflowQueryType::ContentBoxQuery(_) => { - rw_data.content_box_response = Rect::zero(); + rw_data.content_box_response = None; }, ReflowQueryType::ContentBoxesQuery(_) => { rw_data.content_boxes_response = Vec::new(); diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 8902a0545ab..940f98fc8fd 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -524,8 +524,12 @@ impl Node { TrustedNodeAddress(&*self as *const Node as *const libc::c_void) } + /// Returns the rendered bounding content box if the element is rendered, + /// and none otherwise. pub fn bounding_content_box(&self) -> Rect { - window_from_node(self).content_box_query(self.to_trusted_node_address()) + window_from_node(self) + .content_box_query(self.to_trusted_node_address()) + .unwrap_or_else(Rect::zero) } pub fn content_boxes(&self) -> Vec> { diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index d0a87168ab7..9bc4586dc38 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1216,11 +1216,11 @@ impl Window { &*self.layout_rpc } - pub fn content_box_query(&self, content_box_request: TrustedNodeAddress) -> Rect { + pub fn content_box_query(&self, content_box_request: TrustedNodeAddress) -> Option> { if !self.reflow(ReflowGoal::ForScriptQuery, ReflowQueryType::ContentBoxQuery(content_box_request), ReflowReason::Query) { - return Rect::zero(); + return None; } let ContentBoxResponse(rect) = self.layout_rpc.content_box(); rect diff --git a/components/script_layout_interface/rpc.rs b/components/script_layout_interface/rpc.rs index 7b559078291..2fb75f6b959 100644 --- a/components/script_layout_interface/rpc.rs +++ b/components/script_layout_interface/rpc.rs @@ -43,7 +43,7 @@ pub trait LayoutRPC { fn text_index(&self) -> TextIndexResponse; } -pub struct ContentBoxResponse(pub Rect); +pub struct ContentBoxResponse(pub Option>); pub struct ContentBoxesResponse(pub Vec>); From bdd7cb9753981ad09e8258335546641b1aea3c45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 17 Jan 2017 00:27:49 +0100 Subject: [PATCH 3/5] script: Rename bounding_content_box to bounding_content_box_or_zero. And make bounding_content_box preserve whether the element is rendered. --- components/script/dom/document.rs | 2 +- components/script/dom/element.rs | 2 +- components/script/dom/htmlanchorelement.rs | 2 +- components/script/dom/htmlimageelement.rs | 4 ++-- components/script/dom/node.rs | 7 +++++-- components/script/dom/window.rs | 2 +- 6 files changed, 11 insertions(+), 8 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index fc3025615a7..8dd5adf7e3b 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -625,7 +625,7 @@ impl Document { // Really what needs to happen is that this needs to go through layout to ask which // layer the element belongs to, and have it send the scroll message to the // compositor. - let rect = element.upcast::().bounding_content_box(); + let rect = element.upcast::().bounding_content_box_or_zero(); // In order to align with element edges, we snap to unscaled pixel boundaries, since // the paint thread currently does the same for drawing elements. This is important diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index bd3925c04f9..e51b8908189 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -1604,7 +1604,7 @@ impl ElementMethods for Element { // https://drafts.csswg.org/cssom-view/#dom-element-getboundingclientrect fn GetBoundingClientRect(&self) -> Root { let win = window_from_node(self); - let rect = self.upcast::().bounding_content_box(); + let rect = self.upcast::().bounding_content_box_or_zero(); DOMRect::new(win.upcast(), rect.origin.x.to_f64_px(), rect.origin.y.to_f64_px(), diff --git a/components/script/dom/htmlanchorelement.rs b/components/script/dom/htmlanchorelement.rs index ab237f702dd..91ebadbd0f2 100644 --- a/components/script/dom/htmlanchorelement.rs +++ b/components/script/dom/htmlanchorelement.rs @@ -544,7 +544,7 @@ impl Activatable for HTMLAnchorElement { if let Some(element) = target.downcast::() { if target.is::() && element.has_attribute(&local_name!("ismap")) { let target_node = element.upcast::(); - let rect = target_node.bounding_content_box(); + let rect = target_node.bounding_content_box_or_zero(); ismap_suffix = Some( format!("?{},{}", mouse_event.ClientX().to_f32().unwrap() - rect.origin.x.to_f32_px(), mouse_event.ClientY().to_f32().unwrap() - rect.origin.y.to_f32_px()) diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index 629d5d50fb6..27fe846ec11 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/components/script/dom/htmlimageelement.rs @@ -358,7 +358,7 @@ impl HTMLImageElementMethods for HTMLImageElement { // https://html.spec.whatwg.org/multipage/#dom-img-width fn Width(&self) -> u32 { let node = self.upcast::(); - let rect = node.bounding_content_box(); + let rect = node.bounding_content_box_or_zero(); rect.size.width.to_px() as u32 } @@ -370,7 +370,7 @@ impl HTMLImageElementMethods for HTMLImageElement { // https://html.spec.whatwg.org/multipage/#dom-img-height fn Height(&self) -> u32 { let node = self.upcast::(); - let rect = node.bounding_content_box(); + let rect = node.bounding_content_box_or_zero(); rect.size.height.to_px() as u32 } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 940f98fc8fd..71547c860bc 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -526,10 +526,13 @@ impl Node { /// Returns the rendered bounding content box if the element is rendered, /// and none otherwise. - pub fn bounding_content_box(&self) -> Rect { + pub fn bounding_content_box(&self) -> Option> { window_from_node(self) .content_box_query(self.to_trusted_node_address()) - .unwrap_or_else(Rect::zero) + } + + pub fn bounding_content_box_or_zero(&self) -> Rect { + self.bounding_content_box().unwrap_or_else(Rect::zero) } pub fn content_boxes(&self) -> Vec> { diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 9bc4586dc38..c8e4ccc832d 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -970,7 +970,7 @@ impl Window { let body = self.Document().GetBody(); let (x, y) = match body { Some(e) => { - let content_size = e.upcast::().bounding_content_box(); + let content_size = e.upcast::().bounding_content_box_or_zero(); let content_height = content_size.size.height.to_f64_px(); let content_width = content_size.size.width.to_f64_px(); (xfinite.max(0.0f64).min(content_width - width), From 728ce16b9d64849e8dc6f1a6ba430b1684639800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 17 Jan 2017 00:29:57 +0100 Subject: [PATCH 4/5] script: Properly implement the image width and height getter. Per https://html.spec.whatwg.org/multipage/embedded-content.html#dom-img-width: The IDL attributes width and height must return the rendered width and height of the image, in CSS pixels, if the image is being rendered, and is being rendered to a visual medium; or else the density-corrected intrinsic width and height of the image, in CSS pixels, if the image has intrinsic dimensions and is available but not being rendered to a visual medium; or else 0, if the image is not available or does not have intrinsic dimensions. --- components/script/dom/htmlimageelement.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index 27fe846ec11..ec9f2e1ec80 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/components/script/dom/htmlimageelement.rs @@ -358,8 +358,10 @@ impl HTMLImageElementMethods for HTMLImageElement { // https://html.spec.whatwg.org/multipage/#dom-img-width fn Width(&self) -> u32 { let node = self.upcast::(); - let rect = node.bounding_content_box_or_zero(); - rect.size.width.to_px() as u32 + match node.bounding_content_box() { + Some(rect) => rect.size.width.to_px() as u32, + None => self.NaturalWidth(), + } } // https://html.spec.whatwg.org/multipage/#dom-img-width @@ -370,8 +372,10 @@ impl HTMLImageElementMethods for HTMLImageElement { // https://html.spec.whatwg.org/multipage/#dom-img-height fn Height(&self) -> u32 { let node = self.upcast::(); - let rect = node.bounding_content_box_or_zero(); - rect.size.height.to_px() as u32 + match node.bounding_content_box() { + Some(rect) => rect.size.height.to_px() as u32, + None => self.NaturalHeight(), + } } // https://html.spec.whatwg.org/multipage/#dom-img-height From 127da41a2a2eb393b8497e178a6c28bc6ee3129e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 17 Jan 2017 00:32:45 +0100 Subject: [PATCH 5/5] script: Add tests for image intrinsic width when the element is not rendered. --- tests/wpt/metadata/MANIFEST.json | 6 +++++ .../not-rendered-dimension-getter.html | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 tests/wpt/web-platform-tests/html/semantics/embedded-content/the-img-element/not-rendered-dimension-getter.html diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json index 3035761923a..01441eb3d85 100644 --- a/tests/wpt/metadata/MANIFEST.json +++ b/tests/wpt/metadata/MANIFEST.json @@ -45872,6 +45872,12 @@ "path": "html/semantics/embedded-content/the-iframe-element/iframe-synchronously-discard.html", "url": "/html/semantics/embedded-content/the-iframe-element/iframe-synchronously-discard.html" } + ], + "html/semantics/embedded-content/the-img-element/not-rendered-dimension-getter.html": [ + { + "path": "html/semantics/embedded-content/the-img-element/not-rendered-dimension-getter.html", + "url": "/html/semantics/embedded-content/the-img-element/not-rendered-dimension-getter.html" + } ] } }, diff --git a/tests/wpt/web-platform-tests/html/semantics/embedded-content/the-img-element/not-rendered-dimension-getter.html b/tests/wpt/web-platform-tests/html/semantics/embedded-content/the-img-element/not-rendered-dimension-getter.html new file mode 100644 index 00000000000..4d929fd8b16 --- /dev/null +++ b/tests/wpt/web-platform-tests/html/semantics/embedded-content/the-img-element/not-rendered-dimension-getter.html @@ -0,0 +1,22 @@ + + +Image intrinsic dimensions are returned if the image isn't rendered + + + + + +