Auto merge of #15053 - emilio:image-width, r=jdm

Return the intrinsic image dimension when the image is not rendered

See individual commits for details.

r? @jdm

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15053)
<!-- Reviewable:end -->

components/layout/query.rs

@@ -54,7 +54,7 @@ pub struct LayoutThreadData {
     pub stylist: Arc<Stylist>,
 
     /// A queued response for the union of the content boxes of a node.
-    pub content_box_response: Rect<Au>,
+    pub content_box_response: Option<Rect<Au>>,
 
     /// A queued response for the content boxes of a node.
     pub content_boxes_response: Vec<Rect<Au>>,
@@ -384,15 +384,12 @@ impl FragmentBorderBoxIterator for MarginRetrievingFragmentBorderBoxIterator {
 }
 
 pub fn process_content_box_request<N: LayoutNode>(
-        requested_node: N, layout_root: &mut Flow) -> Rect<Au> {
+        requested_node: N, layout_root: &mut Flow) -> Option<Rect<Au>> {
     // 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 {
+    iterator.rect
-        Some(rect) => rect,
-        None       => Rect::zero()
-    }
 }
 
 pub fn process_content_boxes_request<N: LayoutNode>(requested_node: N, layout_root: &mut Flow)

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();

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::<Node>().bounding_content_box();
+            let rect = element.upcast::<Node>().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

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<DOMRect> {
         let win = window_from_node(self);
-        let rect = self.upcast::<Node>().bounding_content_box();
+        let rect = self.upcast::<Node>().bounding_content_box_or_zero();
         DOMRect::new(win.upcast(),
                      rect.origin.x.to_f64_px(),
                      rect.origin.y.to_f64_px(),

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::<Element>() {
             if target.is::<HTMLImageElement>() && element.has_attribute(&local_name!("ismap")) {
                 let target_node = element.upcast::<Node>();
-                let rect = window_from_node(target_node).content_box_query(
+                let rect = target_node.bounding_content_box_or_zero();
-                    target_node.to_trusted_node_address());
                 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())

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::<Node>();
-        let rect = node.bounding_content_box();
+        match node.bounding_content_box() {
-        rect.size.width.to_px() as u32
+            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::<Node>();
-        let rect = node.bounding_content_box();
+        match node.bounding_content_box() {
-        rect.size.height.to_px() as u32
+            Some(rect) => rect.size.height.to_px() as u32,
+            None => self.NaturalHeight(),
+        }
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-img-height

components/script/dom/node.rs

@@ -524,8 +524,15 @@ impl Node {
         TrustedNodeAddress(&*self as *const Node as *const libc::c_void)
     }
 
-    pub fn bounding_content_box(&self) -> Rect<Au> {
+    /// Returns the rendered bounding content box if the element is rendered,
-        window_from_node(self).content_box_query(self.to_trusted_node_address())
+    /// and none otherwise.
+    pub fn bounding_content_box(&self) -> Option<Rect<Au>> {
+        window_from_node(self)
+            .content_box_query(self.to_trusted_node_address())
+    }
+
+    pub fn bounding_content_box_or_zero(&self) -> Rect<Au> {
+        self.bounding_content_box().unwrap_or_else(Rect::zero)
     }
 
     pub fn content_boxes(&self) -> Vec<Rect<Au>> {

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::<Node>().bounding_content_box();
+                let content_size = e.upcast::<Node>().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),
@@ -1216,11 +1216,11 @@ impl Window {
         &*self.layout_rpc
     }
 
-    pub fn content_box_query(&self, content_box_request: TrustedNodeAddress) -> Rect<Au> {
+    pub fn content_box_query(&self, content_box_request: TrustedNodeAddress) -> Option<Rect<Au>> {
         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

components/script_layout_interface/rpc.rs

@@ -43,7 +43,7 @@ pub trait LayoutRPC {
     fn text_index(&self) -> TextIndexResponse;
 }
 
-pub struct ContentBoxResponse(pub Rect<Au>);
+pub struct ContentBoxResponse(pub Option<Rect<Au>>);
 
 pub struct ContentBoxesResponse(pub Vec<Rect<Au>>);
 

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"
+          }
         ]
       }
     },

tests/wpt/web-platform-tests/html/semantics/embedded-content/the-img-element/not-rendered-dimension-getter.html

@@ -0,0 +1,22 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Image intrinsic dimensions are returned if the image isn't rendered</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="help" href="https://html.spec.whatwg.org/multipage/#dom-img-width">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="container" style="display: none">
+</div>
+<script>
+async_test(function(t) {
+  var img = document.createElement('img');
+  img.onload = t.step_func_done(function() {
+    assert_equals(img.width, 389, "intrinsic width should've been returned")
+    assert_equals(img.height, 590, "intrinsic height should've been returned")
+    document.getElementById('container').appendChild(img);
+    assert_equals(img.width, 389, "intrinsic width should've been returned");
+    assert_equals(img.height, 590, "intrinsic height should've been returned");
+  });
+  img.src = "image-1.jpg";
+});
+</script>