auto merge of #1520 : ksh8281/servo/img_s, r=pcwalton

it makes same result with firefix, chrome in http://www.w3.org/Style/CSS/Test/CSS1/current/sec5523.htm

src/components/main/layout/box_.rs

@@ -27,7 +27,7 @@ use std::cell::RefCell;
 use std::cmp::ApproxEq;
 use std::num::Zero;
 use style::{ComputedValues, TElement, TNode, cascade};
-use style::computed_values::{LengthOrPercentage, overflow};
+use style::computed_values::{LengthOrPercentage, LengthOrPercentageOrAuto, overflow, LPA_Auto};
 use style::computed_values::{border_style, clear, font_family, line_height};
 use style::computed_values::{text_align, text_decoration, vertical_align, visibility};
 
@@ -37,7 +37,7 @@ use layout::display_list_builder::{DisplayListBuilder, ExtraDisplayListData, ToG
 use layout::float_context::{ClearType, ClearLeft, ClearRight, ClearBoth};
 use layout::flow::Flow;
 use layout::flow;
-use layout::model::{MaybeAuto, Auto, specified};
+use layout::model::{MaybeAuto, specified, Auto, Specified};
 use layout::util::OpaqueNode;
 use layout::wrapper::LayoutNode;
 
@@ -108,9 +108,9 @@ pub enum SpecificBoxInfo {
 pub struct ImageBoxInfo {
     /// The image held within this box.
     image: RefCell<ImageHolder>,
-    /// The width attribute supplied by the DOM, if any.
+    computed_width: RefCell<Option<Au>>,
+    computed_height: RefCell<Option<Au>>,
     dom_width: Option<Au>,
-    /// The height attribute supplied by the DOM, if any.
     dom_height: Option<Au>,
 }
 
@@ -121,6 +121,7 @@ impl ImageBoxInfo {
     /// me.
     pub fn new(node: &LayoutNode, image_url: Url, local_image_cache: MutexArc<LocalImageCache>)
                -> ImageBoxInfo {
+
         fn convert_length(node: &LayoutNode, name: &str) -> Option<Au> {
             node.with_element(|element| {
                 element.get_attr(None, name).and_then(|string| {
@@ -132,27 +133,61 @@ impl ImageBoxInfo {
 
         ImageBoxInfo {
             image: RefCell::new(ImageHolder::new(image_url, local_image_cache)),
-            dom_width: convert_length(node, "width"),
-            dom_height: convert_length(node, "height"),
+            computed_width: RefCell::new(None),
+            computed_height: RefCell::new(None),
+            dom_width: convert_length(node,"width"),
+            dom_height: convert_length(node,"height"),
         }
     }
 
-    // Calculates the width of an image, accounting for the width attribute.
-    fn image_width(&self) -> Au {
-        // TODO(brson): Consult margins and borders?
-        self.dom_width.unwrap_or_else(|| {
+    /// Returns the calculated width of the image, accounting for the width attribute.
+    pub fn computed_width(&self) -> Au {
+        match self.computed_width.borrow().get() {
+            &Some(width) => {
+                width
+            },
+            &None => {
+                fail!("width is not computed yet!");
+            }
+        }
+    }
+    /// Returns width of image(just original width)
+    pub fn image_width(&self) -> Au {
         let mut image_ref = self.image.borrow_mut();
-            Au::from_px(image_ref.get().get_size().unwrap_or(Size2D(0, 0)).width)
-        })
+        Au::from_px(image_ref.get().get_size().unwrap_or(Size2D(0,0)).width)
     }
 
-    // Calculate the height of an image, accounting for the height attribute.
+    pub fn style_length(style_length: LengthOrPercentageOrAuto,
+                        dom_length: Option<Au>,
+                        container_width: Au) -> MaybeAuto {
+        match (MaybeAuto::from_style(style_length,container_width),dom_length) {
+            (Specified(length),_) => {
+                Specified(length)
+            },
+            (Auto,Some(length)) => {
+                Specified(length)
+            },
+            (Auto,None) => {
+                Auto
+            }
+        }
+    }
+    /// Returns the calculated height of the image, accounting for the height attribute.
+    pub fn computed_height(&self) -> Au {
+        match self.computed_height.borrow().get() {
+            &Some(height) => {
+                height
+            },
+            &None => {
+                fail!("image size is not computed yet!");
+            }
+        }
+    }
+
+    /// Returns height of image(just original height)
     pub fn image_height(&self) -> Au {
-        // TODO(brson): Consult margins and borders?
-        self.dom_height.unwrap_or_else(|| {
         let mut image_ref = self.image.borrow_mut();
-            Au::from_px(image_ref.get().get_size().unwrap_or(Size2D(0, 0)).height)
-        })
+        Au::from_px(image_ref.get().get_size().unwrap_or(Size2D(0,0)).height)
     }
 }
 
@@ -939,23 +974,28 @@ impl Box {
         (guessed_width + additional_minimum, guessed_width + additional_preferred)
     }
 
-    /// Returns, and computes, the height of this box.
-    ///
-    /// FIXME(pcwalton): Rename to just `height`?
-    /// FIXME(pcwalton): This function *mutates* the height? Gross! Refactor please.
-    pub fn box_height(&self) -> Au {
+
+    pub fn content_width(&self) -> Au {
         match self.specific {
             GenericBox | IframeBox(_) => Au(0),
             ImageBox(ref image_box_info) => {
-                let mut image_ref = image_box_info.image.borrow_mut();
-                let size = image_ref.get().get_size();
-                let height = Au::from_px(size.unwrap_or(Size2D(0, 0)).height);
-
-                // Eww. Refactor this.
-                self.position.borrow_mut().get().size.height = height;
-                debug!("box_height: found image height: {}", height);
-
-                height
+                image_box_info.computed_width()
+            }
+            ScannedTextBox(ref text_box_info) => {
+                let (range, run) = (&text_box_info.range, &text_box_info.run);
+                let text_bounds = run.get().metrics_for_range(range).bounding_box;
+                text_bounds.size.width
+            }
+            UnscannedTextBox(_) => fail!("Unscanned text boxes should have been scanned by now!"),
+        }
+    }
+    /// Returns, and computes, the height of this box.
+    ///
+    pub fn content_height(&self) -> Au {
+        match self.specific {
+            GenericBox | IframeBox(_) => Au(0),
+            ImageBox(ref image_box_info) => {
+                image_box_info.computed_height()
             }
             ScannedTextBox(ref text_box_info) => {
                 // Compute the height based on the line-height and font size.
@@ -1077,15 +1117,79 @@ impl Box {
     }
 
     /// Assigns the appropriate width to this box.
-    pub fn assign_width(&self) {
+    pub fn assign_width(&self,container_width: Au) {
         match self.specific {
             GenericBox | IframeBox(_) => {
-                // FIXME(pcwalton): This seems clownshoes; can we remove?
-                self.position.borrow_mut().get().size.width = Au::from_px(45)
             }
             ImageBox(ref image_box_info) => {
-                let image_width = image_box_info.image_width();
-                self.position.borrow_mut().get().size.width = image_width
+                // TODO(ksh8281): compute border,margin,padding
+                let width = ImageBoxInfo::style_length(self.style().Box.width,
+                                                       image_box_info.dom_width,
+                                                       container_width);
+
+                // FIXME(ksh8281): we shouldn't figure height this way
+                // now, we don't know about size of parent's height
+                let height = ImageBoxInfo::style_length(self.style().Box.height,
+                                                       image_box_info.dom_height,
+                                                       Au::new(0));
+
+                let width = match (width,height) {
+                    (Auto,Auto) => {
+                        image_box_info.image_width()
+                    },
+                    (Auto,Specified(h)) => {
+                        let scale = image_box_info.
+                            image_height().to_f32().unwrap() / h.to_f32().unwrap();
+                        Au::new((image_box_info.image_width().to_f32().unwrap() / scale) as i32)
+                    },
+                    (Specified(w),_) => {
+                        w
+                    }
+                };
+
+                let mut position = self.position.borrow_mut();
+                position.get().size.width = width;
+                image_box_info.computed_width.set(Some(width));
+            }
+            ScannedTextBox(_) => {
+                // Scanned text boxes will have already had their widths assigned by this point.
+            }
+            UnscannedTextBox(_) => fail!("Unscanned text boxes should have been scanned by now!"),
+        }
+    }
+
+    pub fn assign_height(&self) {
+        match self.specific {
+            GenericBox | IframeBox(_) => {
+            }
+            ImageBox(ref image_box_info) => {
+                // TODO(ksh8281): compute border,margin,padding
+                let width = image_box_info.computed_width();
+                // FIXME(ksh8281): we shouldn't assign height this way
+                // we don't know about size of parent's height
+                let height = ImageBoxInfo::style_length(self.style().Box.height,
+                                                        image_box_info.dom_height,
+                                                        Au::new(0));
+
+                let height = match (self.style().Box.width, 
+                                    image_box_info.dom_width,
+                                    height) {
+                    (LPA_Auto, None, Auto) => {
+                        image_box_info.image_height()
+                    },
+                    (_,_,Auto) => {
+                        let scale = image_box_info.image_width().to_f32().unwrap()
+                            / width.to_f32().unwrap();
+                        Au::new((image_box_info.image_height().to_f32().unwrap() / scale) as i32)
+                    },
+                    (_,_,Specified(h)) => {
+                        h
+                    }
+                };
+
+                let mut position = self.position.borrow_mut();
+                position.get().size.height = height;
+                image_box_info.computed_height.set(Some(height));
             }
             ScannedTextBox(_) => {
                 // Scanned text boxes will have already had their widths assigned by this point.

src/components/main/layout/inline.rs

@@ -160,7 +160,7 @@ impl LineboxScanner {
     // FIXME(eatkinson): this assumes that the tallest box in the line determines the line height
     // This might not be the case with some weird text fonts.
     fn new_height_for_line(&self, new_box: &Box) -> Au {
-        let box_height = new_box.box_height();
+        let box_height = new_box.content_height();
         if box_height > self.pending_line.bounds.size.height {
             box_height
         } else {
@@ -508,7 +508,7 @@ impl InlineFlow {
             vertical_align::middle => {
                 // TODO: x-height value should be used from font info.
                 let xheight = Au::new(0);
-                (-(xheight + cur_box.box_height()).scale_by(0.5), false)
+                (-(xheight + cur_box.content_height()).scale_by(0.5), false)
             },
             vertical_align::sub => {
                 // TODO: The proper position for subscripts should be used.
@@ -636,7 +636,7 @@ impl Flow for InlineFlow {
         {
             let this = &mut *self;
             for box_ in this.boxes.iter() {
-                box_.assign_width();
+                box_.assign_width(self.base.position.size.width);
             }
         }
 
@@ -676,6 +676,10 @@ impl Flow for InlineFlow {
         // TODO(pcwalton): Cache the linebox scanner?
         debug!("assign_height_inline: floats_in: {:?}", self.base.floats_in);
 
+        // assign height for inline boxes
+        for box_ in self.boxes.iter() {
+            box_.assign_height();
+        }
         let scanner_floats = self.base.floats_in.clone();
         let mut scanner = LineboxScanner::new(scanner_floats);
 
@@ -707,8 +711,8 @@ impl Flow for InlineFlow {
 
                 // FIXME(pcwalton): Move into `box.rs` like the rest of box-specific layout code?
                 let (top_from_base, bottom_from_base, ascent) = match cur_box.specific {
-                    ImageBox(ref image_box) => {
-                        let mut height = image_box.image_height();
+                    ImageBox(_) => {
+                        let mut height = cur_box.content_height();
 
                         // TODO: margin, border, padding's top and bottom should be calculated in
                         // advance, since baseline of image is bottom margin edge.

src/test/ref/basic.list

@@ -22,3 +22,4 @@
 == font_size_em.html font_size_em_ref.html
 == 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

src/test/ref/img_size_a.html

@@ -0,0 +1,18 @@
+<html>
+    <head>
+        <title>
+            The winter is too cold for me...
+        </title>
+    </head>
+    <body>
+        <div>
+            <img src="img_size.png" width="206" />
+        </div>
+        <div>
+            <img src="img_size.png" width="206" />
+        </div>
+        <div style="width:1000px">
+            <img src="img_size.png" style="width:100px" />
+        </div>
+    </body>
+</html>

src/test/ref/img_size_b.html

@@ -0,0 +1,18 @@
+<html>
+    <head>
+        <title>
+            The winter is too cold for me...
+        </title>
+    </head>
+    <body>
+        <div>
+            <img src="img_size.png" height="206" />
+        </div>
+        <div>
+            <img src="img_size.png" style="height:206px"/>
+        </div>
+        <div style="width:1000px">
+            <img src="img_size.png" style="width:10%" />
+        </div>
+    </body>
+</html>