Revert "Add initial support for the line-height property. Line height is only based on the tallest box in each line and does not factor in the vertical-alignment. Improves whitespace handling by passing the whitespace state between function invocations."

This reverts commit 7dcff2b5ff, reversing
changes made to 93eea6b2e8.

src/components/gfx/geometry.rs

@@ -112,8 +112,9 @@ pub impl Au {
         Rect(Point2D(z, z), Size2D(z, z))
     }
 
+    // assumes 72 points per inch, and 96 px per inch
     pub fn from_pt(f: float) -> Au {
-        from_px(pt_to_px(f) as int)
+        from_px((f / 72f * 96f) as int)
     }
 
     pub fn from_frac_px(f: float) -> Au {
@@ -124,16 +125,6 @@ pub impl Au {
     pub fn max(x: Au, y: Au) -> Au { if *x > *y { x } else { y } }
 }
 
-// assumes 72 points per inch, and 96 px per inch
-pub fn pt_to_px(f: float) -> float {
-    f / 72f * 96f
-}
-
-// assumes 72 points per inch, and 96 px per inch
-pub fn px_to_pt(f: float) -> float {
-    f / 96f * 72f
-}
-
 pub fn zero_rect() -> Rect<Au> {
     let z = Au(0);
     Rect(Point2D(z, z), Size2D(z, z))

src/components/gfx/platform/linux/font.rs

@@ -219,7 +219,7 @@ impl FontHandleMethods for FontHandle {
             x_height:         geometry::from_pt(0.0), //FIXME
             em_size:          em_size,
             ascent:           ascent,
-            descent:          -descent, // linux font's seem to use the opposite sign from mac
+            descent:          descent,
             max_advance:      max_advance
         }
     }

src/components/gfx/platform/macos/font.rs

@@ -12,7 +12,7 @@ use font::{CSSFontWeight, FontHandleMethods, FontMetrics, FontTableMethods};
 use font::{FontTableTag, FontWeight100, FontWeight200, FontWeight300, FontWeight400};
 use font::{FontWeight500, FontWeight600, FontWeight700, FontWeight800, FontWeight900};
 use font::{FractionalPixel, SpecifiedFontStyle};
-use geometry::{Au, px_to_pt};
+use geometry::Au;
 use platform::macos::font_context::FontContextHandle;
 use text::glyph::GlyphIndex;
 
@@ -157,9 +157,6 @@ impl FontHandleMethods for FontHandle {
         let bounding_rect: CGRect = self.ctfont.bounding_box();
         let ascent = Au::from_pt(self.ctfont.ascent() as float);
         let descent = Au::from_pt(self.ctfont.descent() as float);
-        let em_size = Au::from_frac_px(self.ctfont.pt_size() as float);
-
-        let scale = px_to_pt(self.ctfont.pt_size() as float) / (self.ctfont.ascent() as float + self.ctfont.descent() as float);
 
         let metrics =  FontMetrics {
             underline_size:   Au::from_pt(self.ctfont.underline_thickness() as float),
@@ -171,9 +168,9 @@ impl FontHandleMethods for FontHandle {
             underline_offset: Au::from_pt(self.ctfont.underline_position() as float),
             leading:          Au::from_pt(self.ctfont.leading() as float),
             x_height:         Au::from_pt(self.ctfont.x_height() as float),
-            em_size:          em_size,
+            em_size:          ascent + descent,
-            ascent:           ascent.scale_by(scale),
+            ascent:           ascent,
-            descent:          descent.scale_by(scale),
+            descent:          descent,
             max_advance:      Au::from_pt(bounding_rect.size.width as float)
         };
 

src/components/gfx/text/util.rs

@@ -28,15 +28,15 @@ impl Eq for CompressionMode {
 // 
 // High level TODOs:
 //
-// * Issue #113: consider incoming text state (arabic, etc)
+// * Issue #113: consider incoming text state (preceding spaces, arabic, etc)
 //               and propogate outgoing text state (dual of above) 
 //
 // * Issue #114: record skipped and kept chars for mapping original to new text
 //
 // * Untracked: various edge cases for bidi, CJK, etc.
-pub fn transform_text(text: &str, mode: CompressionMode, incoming_whitespace: bool) -> (~str, bool) {
+pub fn transform_text(text: &str, mode: CompressionMode) -> ~str {
     let mut out_str: ~str = ~"";
-    let out_whitespace = match mode {
+    match mode {
         CompressNone | DiscardNewline => {
             for str::each_char(text) |ch: char| {
                 if is_discardable_char(ch, mode) {
@@ -49,14 +49,18 @@ pub fn transform_text(text: &str, mode: CompressionMode, incoming_whitespace: bo
                     str::push_char(&mut out_str, ch);
                 }
             }
-            text.len() > 0 && is_in_whitespace(text.char_at_reverse(0), mode)
         },
 
         CompressWhitespace | CompressWhitespaceNewline => {
-            let mut in_whitespace: bool = incoming_whitespace;
+            let mut in_whitespace: bool = false;
             for str::each_char(text) |ch: char| {
                 // TODO: discard newlines between CJK chars
-                let mut next_in_whitespace: bool = is_in_whitespace(ch, mode);
+                let mut next_in_whitespace: bool = match (ch, mode) {
+                    (' ', _)  => true,
+                    ('\t', _) => true,
+                    ('\n', CompressWhitespaceNewline) => true,
+                    (_, _)    => false
+                };
                 
                 if !next_in_whitespace {
                     if is_always_discardable_char(ch) {
@@ -78,21 +82,11 @@ pub fn transform_text(text: &str, mode: CompressionMode, incoming_whitespace: bo
                 // save whitespace context for next char
                 in_whitespace = next_in_whitespace;
             } /* /for str::each_char */
-            in_whitespace
         } 
-    };
-
-    return (out_str, out_whitespace);
-
-    fn is_in_whitespace(ch: char, mode: CompressionMode) -> bool {
-        match (ch, mode) {
-            (' ', _)  => true,
-            ('\t', _) => true,
-            ('\n', CompressWhitespaceNewline) => true,
-            (_, _)    => false
-        }
     }
 
+    return out_str;
+
     fn is_discardable_char(ch: char, mode: CompressionMode) -> bool {
         if is_always_discardable_char(ch) {
             return true;
@@ -149,8 +143,7 @@ fn test_transform_compress_none() {
     let mode = CompressNone;
 
     for uint::range(0, test_strs.len()) |i| {
-        (trimmed_str, _out) = transform_text(test_strs[i], mode, true);
+        assert!(transform_text(test_strs[i], mode) == test_strs[i]);
-        assert!(trimmed_str == test_strs[i])
     }
 }
 
@@ -177,8 +170,7 @@ fn test_transform_discard_newline() {
     let mode = DiscardNewline;
 
     for uint::range(0, test_strs.len()) |i| {
-        (trimmed_str, _out) = transform_text(test_strs[i], mode, true);
+        assert!(transform_text(test_strs[i], mode) == oracle_strs[i]);
-        assert!(trimmed_str == oracle_strs[i])
     }
 }
 
@@ -204,8 +196,7 @@ fn test_transform_compress_whitespace() {
     let mode = CompressWhitespace;
 
     for uint::range(0, test_strs.len()) |i| {
-        (trimmed_str, _out) = transform_text(test_strs[i], mode, true);
+        assert!(transform_text(test_strs[i], mode) == oracle_strs[i]);
-        assert!(trimmed_str == oracle_strs[i])
     }
 }
 
@@ -219,36 +210,7 @@ fn test_transform_compress_whitespace_newline() {
                                  ~"foo bar baz",
                                  ~"foobarbaz\n\n"];
 
-    let oracle_strs : ~[~str] = ~[~"foo bar",
-                                 ~"foo bar ",
-                                 ~"foo bar",
-                                 ~"foo bar",
-                                 ~" foo bar baz",
-                                 ~"foo bar baz",
-                                 ~"foobarbaz "];
-
-    assert!(test_strs.len() == oracle_strs.len());
-    let mode = CompressWhitespaceNewline;
-
-    for uint::range(0, test_strs.len()) |i| {
-        (trimmed_str, _out) = transform_text(test_strs[i], mode, true);
-        assert!(trimmed_str == oracle_strs[i])
-    }
-}
-
-#[test]
-fn test_transform_compress_whitespace_newline() {
-    let  test_strs : ~[~str] = ~[~"  foo bar",
-                                 ~"\nfoo bar",
-                                 ~"foo bar  ",
-                                 ~"foo\n bar",
-                                 ~"foo \nbar",
-                                 ~"  foo  bar  \nbaz",
-                                 ~"foo bar baz",
-                                 ~"foobarbaz\n\n"];
-
     let oracle_strs : ~[~str] = ~[~" foo bar",
-                                 ~" foo bar",
                                  ~"foo bar ",
                                  ~"foo bar",
                                  ~"foo bar",
@@ -260,7 +222,6 @@ fn test_transform_compress_whitespace_newline() {
     let mode = CompressWhitespaceNewline;
 
     for uint::range(0, test_strs.len()) |i| {
-        (trimmed_str, _out) = transform_text(test_strs[i], mode, false);
+        assert!(transform_text(test_strs[i], mode) == oracle_strs[i]);
-        assert!(trimmed_str == oracle_strs[i])
     }
 }

src/components/main/layout/box.rs

@@ -21,14 +21,14 @@ use gfx::display_list::{DisplayList, ImageDisplayItem, ImageDisplayItemClass};
 use gfx::display_list::{SolidColorDisplayItem, SolidColorDisplayItemClass, TextDisplayItem};
 use gfx::display_list::{TextDisplayItemClass};
 use gfx::font::{FontStyle, FontWeight300};
-use gfx::geometry::{Au, pt_to_px};
+use gfx::geometry::Au;
 use gfx::text::text_run::TextRun;
 use newcss::color::rgb;
 use newcss::complete::CompleteStyle;
 use newcss::units::{Cursive, Em, Fantasy, Monospace, Pt, Px, SansSerif, Serif};
 use newcss::values::{CSSFontFamilyFamilyName, CSSFontFamilyGenericFamily};
 use newcss::values::{CSSFontSizeLength, CSSFontStyleItalic, CSSFontStyleNormal};
-use newcss::values::{CSSFontStyleOblique, CSSTextAlign, CSSTextDecoration, CSSLineHeight};
+use newcss::values::{CSSFontStyleOblique, CSSTextAlign, CSSTextDecoration};
 use newcss::values::{CSSTextDecorationNone, CSSFloatNone, CSSPositionStatic};
 use newcss::values::{CSSDisplayInlineBlock, CSSDisplayInlineTable};
 use script::dom::node::{AbstractNode, LayoutView};
@@ -559,6 +559,9 @@ pub impl RenderBox {
             return;
         }
 
+        // Add the background to the list, if applicable.
+        self.paint_background_if_applicable(list, &absolute_box_bounds);
+
         match *self {
             UnscannedTextRenderBoxClass(*) => fail!(~"Shouldn't see unscanned boxes here."),
             TextRenderBoxClass(text_box) => {
@@ -622,35 +625,26 @@ pub impl RenderBox {
                     ()
                 });
             },
+
             GenericRenderBoxClass(_) => {
-
+                // FIXME(pcwalton): This is somewhat of an abuse of the logging system.
-                // Add the background to the list, if applicable.
+                debug!("%?", { 
-                self.paint_background_if_applicable(list, &absolute_box_bounds);
+                    // Compute the text box bounds and draw a border surrounding them.
-
-                // FIXME(pcwalton): This is a bit of an abuse of the logging infrastructure. We
-                // should have a real `SERVO_DEBUG` system.
-                debug!("%?", {
                     do list.with_mut_ref |list| {
                         let border_display_item = ~BorderDisplayItem {
                             base: BaseDisplayItem {
-                                bounds: absolute_box_bounds,
+                                    bounds: absolute_box_bounds,
-                                extra: ExtraDisplayListData::new(*self),
+                                    extra: ExtraDisplayListData::new(*self),
                             },
                             width: Au::from_px(1),
-                            color: rgb(0, 0, 200).to_gfx_color(),
+                            color: rgb(0, 0, 0).to_gfx_color(),
                         };
                         list.append_item(BorderDisplayItemClass(border_display_item))
                     }
-                    
-                    ()
                 });
+            }
 
-            },
             ImageRenderBoxClass(image_box) => {
-
-                // Add the background to the list, if applicable.
-                self.paint_background_if_applicable(list, &absolute_box_bounds);
-
                 match image_box.image.get_image() {
                     Some(image) => {
                         debug!("(building display list) building image box");
@@ -714,9 +708,6 @@ pub impl RenderBox {
     fn font_style(&self) -> FontStyle {
         let my_style = self.nearest_ancestor_element().style();
 
-        debug!("(font style) start: %?", self.nearest_ancestor_element().type_id());
-        self.dump();
-
         // FIXME: Too much allocation here.
         let font_families = do my_style.font_family().map |family| {
             match *family {
@@ -732,13 +723,12 @@ pub impl RenderBox {
         debug!("(font style) font families: `%s`", font_families);
 
         let font_size = match my_style.font_size() {
-            CSSFontSizeLength(Px(length)) => length,
+            CSSFontSizeLength(Px(length)) |
-            CSSFontSizeLength(Pt(length)) => pt_to_px(length),
+            CSSFontSizeLength(Pt(length)) |
-            // todo: this is based on a hard coded font size, should be the parent element's font size
+            CSSFontSizeLength(Em(length)) => length,
-            CSSFontSizeLength(Em(length)) => length * 16f, 
+            _ => 16.0
-            _ => 16f // px units
         };
-        debug!("(font style) font size: `%fpx`", font_size);
+        debug!("(font style) font size: `%f`", font_size);
 
         let (italic, oblique) = match my_style.font_style() {
             CSSFontStyleNormal => (false, false),
@@ -761,10 +751,6 @@ pub impl RenderBox {
         self.nearest_ancestor_element().style().text_align()
     }
 
-    fn line_height(&self) -> CSSLineHeight {
-        self.nearest_ancestor_element().style().line_height()
-    }
-
     /// Returns the text decoration of the computed style of the nearest `Element` node
     fn text_decoration(&self) -> CSSTextDecoration {
         /// Computes the propagated value of text-decoration, as specified in CSS 2.1 § 16.3.1

src/components/main/layout/inline.rs

@@ -20,9 +20,6 @@ use gfx::text::util::*;
 use newcss::values::{CSSTextAlignCenter, CSSTextAlignJustify, CSSTextAlignLeft};
 use newcss::values::{CSSTextAlignRight, CSSTextDecoration, CSSTextDecorationUnderline};
 use script::dom::node::{AbstractNode, LayoutView};
-use newcss::units::{Em, Px, Pt};
-use newcss::values::{CSSLineHeightNormal, CSSLineHeightNumber, CSSLineHeightLength, CSSLineHeightPercentage};
-
 use servo_util::range::Range;
 use std::deque::Deque;
 
@@ -194,18 +191,17 @@ impl TextRunScanner {
         assert!(inline.boxes.len() > 0);
         debug!("TextRunScanner: scanning %u boxes for text runs...", inline.boxes.len());
 
-        let mut last_whitespace = true;
         let mut out_boxes = ~[];
         for uint::range(0, flow.inline().boxes.len()) |box_i| {
             debug!("TextRunScanner: considering box: %?", flow.inline().boxes[box_i].debug_str());
             if box_i > 0 && !can_coalesce_text_nodes(flow.inline().boxes, box_i-1, box_i) {
-                last_whitespace = self.flush_clump_to_list(ctx, flow, last_whitespace, &mut out_boxes);
+                self.flush_clump_to_list(ctx, flow, &mut out_boxes);
             }
             self.clump.extend_by(1);
         }
         // handle remaining clumps
         if self.clump.length() > 0 {
-            self.flush_clump_to_list(ctx, flow, last_whitespace, &mut out_boxes);
+            self.flush_clump_to_list(ctx, flow, &mut out_boxes);
         }
 
         debug!("TextRunScanner: swapping out boxes.");
@@ -242,8 +238,7 @@ impl TextRunScanner {
     fn flush_clump_to_list(&mut self,
                            ctx: &mut LayoutContext,
                            flow: FlowContext,
-                           last_whitespace: bool,
+                           out_boxes: &mut ~[RenderBox]) {
-                           out_boxes: &mut ~[RenderBox]) -> bool {
         let inline = &mut *flow.inline();
         let in_boxes = &inline.boxes;
 
@@ -263,8 +258,6 @@ impl TextRunScanner {
             _ => false
         };
 
-        let mut new_whitespace = last_whitespace;
-
         match (is_singleton, is_text_clump) {
             (false, false) => {
                 fail!(~"WAT: can't coalesce non-text nodes in flush_clump_to_list()!")
@@ -282,15 +275,13 @@ impl TextRunScanner {
                 // TODO(#115): Use the actual CSS `white-space` property of the relevant style.
                 let compression = CompressWhitespaceNewline;
 
-                let (transformed_text, whitespace) = transform_text(text, compression, last_whitespace);
+                let transformed_text = transform_text(text, compression);
-                new_whitespace = whitespace;
 
-                if transformed_text.len() > 0 {
+                // TODO(#177): Text run creation must account for the renderability of text by
-                    // TODO(#177): Text run creation must account for the renderability of text by
+                // font group fonts. This is probably achieved by creating the font group above
-                    // font group fonts. This is probably achieved by creating the font group above
+                // and then letting `FontGroup` decide which `Font` to stick into the text run.
-                    // and then letting `FontGroup` decide which `Font` to stick into the text run.
+                let fontgroup = ctx.font_ctx.get_resolved_font_for_style(&font_style);
-                    let fontgroup = ctx.font_ctx.get_resolved_font_for_style(&font_style);
+                let run = @fontgroup.create_textrun(transformed_text, underline);
-                    let run = @fontgroup.create_textrun(transformed_text, underline);
 
                 debug!("TextRunScanner: pushing single text box in range: %?", self.clump);
                 let new_box = do old_box.with_base |old_box_base| {
@@ -298,26 +289,21 @@ impl TextRunScanner {
                     @mut adapt_textbox_with_range(*old_box_base, run, range)
                 };
 
-                    out_boxes.push(TextRenderBoxClass(new_box));
+                out_boxes.push(TextRenderBoxClass(new_box));
-                }
             },
             (false, true) => {
                 // TODO(#115): Use the actual CSS `white-space` property of the relevant style.
                 let compression = CompressWhitespaceNewline;
 
                 // First, transform/compress text of all the nodes.
-                let mut last_whitespace = true;
                 let transformed_strs: ~[~str] = do vec::from_fn(self.clump.length()) |i| {
                     // TODO(#113): We should be passing the compression context between calls to
                     // `transform_text`, so that boxes starting and/or ending with whitespace can
                     // be compressed correctly with respect to the text run.
                     let idx = i + self.clump.begin();
-                    let (new_str, new_whitespace) = transform_text(in_boxes[idx].raw_text(), compression, last_whitespace);
+                    transform_text(in_boxes[idx].raw_text(), compression)
-                    last_whitespace = new_whitespace;
-                    new_str
                 };
-                new_whitespace = last_whitespace;
+
-                
                 // Next, concatenate all of the transformed strings together, saving the new
                 // character indices.
                 let mut run_str: ~str = ~"";
@@ -342,7 +328,7 @@ impl TextRunScanner {
                 // TextRuns contain a cycle which is usually resolved by the teardown
                 // sequence. If no clump takes ownership, however, it will leak.
                 let clump = self.clump;
-                let run = if clump.length() != 0 && run_str.len() > 0 {
+                let run = if clump.length() != 0 {
                     Some(@TextRun::new(fontgroup.fonts[0], run_str, underline))
                 } else {
                     None
@@ -387,8 +373,6 @@ impl TextRunScanner {
 
         let end = self.clump.end(); // FIXME: borrow checker workaround
         self.clump.reset(end, 0);
-
-        new_whitespace
     } // End of `flush_clump_to_list`.
 }
 
@@ -764,6 +748,7 @@ impl InlineFlowData {
         // TODO(#226): Get the CSS `line-height` property from each non-replaced inline element to
         // determine its height for computing linebox height.
 
+        let line_height = Au::from_px(20);
         let mut cur_y = Au(0);
 
         for self.lines.eachi |i, line_span| {
@@ -771,65 +756,64 @@ impl InlineFlowData {
 
             // These coordinates are relative to the left baseline.
             let mut linebox_bounding_box = Au::zero_rect();
-            let mut linebox_height = Au(0);
-            let mut baseline_offset = Au(0);
-
             let boxes = &mut self.boxes;
-
             for line_span.eachi |box_i| {
                 let cur_box = boxes[box_i]; // FIXME: borrow checker workaround
 
-                // Compute the height and bounding box of each box.
+                // Compute the height of each box.
-                let bounding_box = match cur_box {
+                match cur_box {
                     ImageRenderBoxClass(image_box) => {
                         let size = image_box.image.get_size();
                         let height = Au::from_px(size.get_or_default(Size2D(0, 0)).height);
                         image_box.base.position.size.height = height;
-
-                        image_box.base.position.translate(&Point2D(Au(0), -height))
                     }
-                    TextRenderBoxClass(text_box) => {
+                    TextRenderBoxClass(*) => {
-
+                        // Text boxes are preinitialized.
-                        let range = &text_box.range;
-                        let run = &text_box.run;
-                        
-                        // Compute the height based on the line-height and font size
-                        let text_bounds = run.metrics_for_range(range).bounding_box;
-                        let em_size = text_bounds.size.height;
-                        let line_height = match cur_box.line_height() {
-                            // use the font's leading for normal
-                            CSSLineHeightNormal => text_box.run.font.metrics.leading,
-                            CSSLineHeightNumber(l) => em_size.scale_by(l),
-                            CSSLineHeightLength(Em(l)) => em_size.scale_by(l),
-                            CSSLineHeightLength(Px(l)) => Au::from_frac_px(l),
-                            CSSLineHeightLength(Pt(l)) => Au::from_pt(l),
-                            CSSLineHeightPercentage(p) => em_size.scale_by(p / 100.0f)
-                        };
-
-                        // If this is the current tallest box then use it for baseline
-                        // calculations.
-                        // TODO: this will need to take into account type of line-height
-                        // and the vertical-align value.
-                        if line_height > linebox_height {
-                            linebox_height = line_height;
-                            // Offset from the top of the linebox is 1/2 of the leading + ascent
-                            baseline_offset = text_box.run.font.metrics.ascent +
-                                    (linebox_height - em_size).scale_by(0.5f);
-                        }
-                        text_bounds.translate(&Point2D(text_box.base.position.origin.x, Au(0)))
                     }
                     GenericRenderBoxClass(generic_box) => {
                         // TODO(Issue #225): There will be different cases here for `inline-block`
                         // and other replaced content.
                         // FIXME(pcwalton): This seems clownshoes; can we remove?
                         generic_box.position.size.height = Au::from_px(30);
-                        generic_box.position
                     }
                     // FIXME(pcwalton): This isn't very type safe!
                     _ => {
                         fail!(fmt!("Tried to assign height to unknown Box variant: %s",
                                    cur_box.debug_str()))
                     }
+                }
+
+                // Compute the bounding rect with the left baseline as origin. Determining line box
+                // height is a matter of lining up ideal baselines and then taking the union of all
+                // these rects.
+                let bounding_box = match cur_box {
+                    // Adjust to baseline coordinates.
+                    //
+                    // TODO(#227): Use left/right margins, border, padding for nonreplaced content,
+                    // and also use top/bottom margins, border, padding for replaced or
+                    // inline-block content.
+                    //
+                    // TODO(#225): Use height, width for `inline-block` and other replaced content.
+                    ImageRenderBoxClass(*) | GenericRenderBoxClass(*) => {
+                        let height = cur_box.position().size.height;
+                        cur_box.position().translate(&Point2D(Au(0), -height))
+                    },
+
+                    // Adjust the bounding box metric to the box's horizontal offset.
+                    //
+                    // TODO: We can use font metrics directly instead of re-measuring for the
+                    // bounding box.
+                    TextRenderBoxClass(text_box) => {
+                        let range = &text_box.range;
+                        let run = &text_box.run;
+                        let text_bounds = run.metrics_for_range(range).bounding_box;
+                        text_bounds.translate(&Point2D(text_box.base.position.origin.x, Au(0)))
+                    },
+
+                    _ => {
+                        fail!(fmt!("Tried to compute bounding box of unknown Box variant: %s",
+                                   cur_box.debug_str()))
+                    }
                 };
 
                 debug!("assign_height_inline: bounding box for box b%d = %?",
@@ -841,6 +825,9 @@ impl InlineFlowData {
                 debug!("assign_height_inline: linebox bounding box = %?", linebox_bounding_box);
             }
 
+            let linebox_height = linebox_bounding_box.size.height;
+            let baseline_offset = -linebox_bounding_box.origin.y;
+
             // Now go back and adjust the Y coordinates to match the baseline we determined.
             for line_span.eachi |box_i| {
                 let cur_box = boxes[box_i];
@@ -848,19 +835,30 @@ impl InlineFlowData {
                 // TODO(#226): This is completely wrong. We need to use the element's `line-height`
                 // when calculating line box height. Then we should go back over and set Y offsets
                 // according to the `vertical-align` property of the containing block.
-                let offset = match cur_box {
+                let halfleading = match cur_box {
                     TextRenderBoxClass(text_box) => {
-                        baseline_offset - text_box.run.font.metrics.ascent
+                        //ad is the AD height as defined by CSS 2.1 § 10.8.1
+                        let ad = text_box.run.font.metrics.ascent + text_box.run.font.metrics.descent;
+                        (line_height - ad).scale_by(0.5)
                     },
                     _ => Au(0),
                 };
 
+                //FIXME: when line-height is set on an inline element, the half leading
+                //distance can be negative.
+                let halfleading = Au::max(halfleading, Au(0));
+                
+                let height = match cur_box {
+                    TextRenderBoxClass(text_box) => text_box.run.font.metrics.ascent,
+                    _ => cur_box.position().size.height
+                };
+
                 do cur_box.with_mut_base |base| {
-                    base.position.origin.y = offset + cur_y;
+                    base.position.origin.y = cur_y + halfleading + baseline_offset - height;
                 }
             }
 
-            cur_y += linebox_height;
+            cur_y += Au::max(line_height, linebox_height);
         } // End of `lines.each` loop.
 
         self.common.position.size.height = cur_y;

src/test/html/lineheight-simple.css

@@ -1,10 +0,0 @@
-#larger1 {
-	font-size: 20px;
-	line-height: 2;
-}
-
-#larger2 {
-	font-size: 30px;
-	line-height: 1;
-}
-

src/test/html/lineheight-simple.html

@@ -1,10 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<link rel="stylesheet" href="lineheight-simple.css"/>
-</head>
-<body>
-<div>Regular font <span id="larger1">Even larger with line-height 2</span></div>
-<div id="larger2">Large line 2!</div>
-</body>
-</html>