Fix percentage height calculation, absolute containing block height calculations.

It's not possible to correctly determine during the css cascade whether the container height
is explicitly specified. Additionally, the spec https://drafts.csswg.org/css2/visudet.html#the-height-property
says this should affect the *used* height, rather than the computed height.

This significantly improves the layout in #6643.

10 files changed, 151 insertions, 119 deletions

diff --git a/components/layout/block.rs b/components/layout/block.rs
index c7304865ecb..7bd383b30e8 100644
--- a/components/layout/block.rs
+++ b/components/layout/block.rs
@@ -699,7 +699,7 @@ impl BlockFlow {
     /// Right now, this only gets the containing block size for absolutely positioned elements.
     /// Note: We assume this is called in a top-down traversal, so it is ok to reference the CB.
     #[inline]
-    pub fn containing_block_size(&mut self, viewport_size: &Size2D<Au>, descendant: OpaqueFlow)
+    pub fn containing_block_size(&self, viewport_size: &Size2D<Au>, descendant: OpaqueFlow)
                                  -> LogicalSize<Au> {
         debug_assert!(self.base.flags.contains(IS_ABSOLUTELY_POSITIONED));
         if self.is_fixed() {
@@ -929,6 +929,7 @@ impl BlockFlow {
             let (collapsible_margins, delta) =
                 margin_collapse_info.finish_and_compute_collapsible_margins(
                 &self.fragment,
+                self.base.block_container_explicit_block_size,
                 can_collapse_block_end_margin_with_kids);
             self.base.collapsible_margins = collapsible_margins;
             translate_including_floats(&mut cur_b, delta, &mut floats);
@@ -1095,14 +1096,69 @@ impl BlockFlow {
                                                           self.base.position.size);
     }
 
+    pub fn explicit_block_containing_size(&self, layout_context: &LayoutContext) -> Option<Au> {
+        if self.is_root() || self.is_fixed() {
+            let screen_size = LogicalSize::from_physical(self.fragment.style.writing_mode,
+                                                         layout_context.shared.screen_size);
+            Some(screen_size.block)
+        } else if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) {
+            self.base.absolute_cb.explicit_block_containing_size(layout_context)
+        } else {
+            self.base.block_container_explicit_block_size
+        }
+    }
+
+    fn explicit_block_size(&self, containing_block_size: Option<Au>) -> Option<Au> {
+        let content_block_size = self.fragment.style().content_block_size();
+
+        match (content_block_size, containing_block_size) {
+            (LengthOrPercentageOrAuto::Length(length), _) => Some(length),
+            (LengthOrPercentageOrAuto::Percentage(percent), Some(container_size)) => {
+                Some(container_size.scale_by(percent))
+            }
+            (LengthOrPercentageOrAuto::Percentage(_), None) => {
+                None
+            }
+            (LengthOrPercentageOrAuto::Auto, None) => {
+                None
+            }
+            (LengthOrPercentageOrAuto::Auto, Some(container_size)) => {
+                let (block_start, block_end) = {
+                    let position = self.fragment.style().logical_position();
+                    (MaybeAuto::from_style(position.block_start, container_size),
+                     MaybeAuto::from_style(position.block_end, container_size))
+                };
+
+                match (block_start, block_end) {
+                    (MaybeAuto::Specified(block_start), MaybeAuto::Specified(block_end)) => {
+                        let available_block_size = container_size - self.fragment.border_padding.block_start_end();
+
+                        // Non-auto margin-block-start and margin-block-end values have already been
+                        // calculated during assign-inline-size.
+                        let margin = self.fragment.style().logical_margin();
+                        let margin_block_start = match margin.block_start {
+                            LengthOrPercentageOrAuto::Auto => MaybeAuto::Auto,
+                            _ => MaybeAuto::Specified(self.fragment.margin.block_start)
+                        };
+                        let margin_block_end = match margin.block_end {
+                            LengthOrPercentageOrAuto::Auto => MaybeAuto::Auto,
+                            _ => MaybeAuto::Specified(self.fragment.margin.block_end)
+                        };
+
+                        let margin_block_start = margin_block_start.specified_or_zero();
+                        let margin_block_end = margin_block_end.specified_or_zero();
+                        let sum = block_start + block_end + margin_block_start + margin_block_end;
+                        Some(available_block_size - sum)
+                    }
+
+                    (_, _) => {
+                        None
+                    }
+                }
+            }
+        }
+    }
 
-    /// Calculate and set the block-size, offsets, etc. for absolutely positioned flow.
-    ///
-    /// The layout for its in-flow children has been done during normal layout.
-    /// This is just the calculation of:
-    /// + block-size for the flow
-    /// + position in the block direction of the flow with respect to its Containing Block.
-    /// + block-size, vertical margins, and y-coordinate for the flow's box.
     fn calculate_absolute_block_size_and_margins(&mut self, layout_context: &LayoutContext) {
         let opaque_self = OpaqueFlow::from_flow(self);
         let containing_block_block_size =
@@ -1140,7 +1196,7 @@ impl BlockFlow {
                 // Calculate used value of block-size just like we do for inline replaced elements.
                 // TODO: Pass in the containing block block-size when Fragment's
                 // assign-block-size can handle it correctly.
-                self.fragment.assign_replaced_block_size_if_necessary(containing_block_block_size);
+                self.fragment.assign_replaced_block_size_if_necessary(Some(containing_block_block_size));
                 // TODO: Right now, this content block-size value includes the
                 // margin because of erroneous block-size calculation in fragment.
                 // Check this when that has been fixed.
@@ -1248,28 +1304,13 @@ impl BlockFlow {
             inline_size_of_preceding_right_floats = self.inline_size_of_preceding_right_floats;
         }
 
-        // Calculate non-auto block size to pass to children.
-        let content_block_size = self.fragment.style().content_block_size();
-
-        let parent_container_size = if self.is_root() {
-            let screen_size = LogicalSize::from_physical(self.fragment.style.writing_mode,
-                                                         layout_context.shared.screen_size);
-            Some(screen_size.block)
-        } else {
-            self.base.block_container_explicit_block_size
-        };
+        let opaque_self = OpaqueFlow::from_flow(self);
 
-        let explicit_content_size = match (content_block_size, parent_container_size) {
-            (LengthOrPercentageOrAuto::Percentage(percent), Some(container_size)) => {
-                Some(container_size.scale_by(percent))
-            }
-            (LengthOrPercentageOrAuto::Percentage(_), None) |
-            (LengthOrPercentageOrAuto::Auto, _) => None,
-            (LengthOrPercentageOrAuto::Length(length), _) => Some(length),
-        };
+        // Calculate non-auto block size to pass to children.
+        let parent_container_size = self.explicit_block_containing_size(layout_context);
+        let explicit_content_size = self.explicit_block_size(parent_container_size);
 
         // Calculate containing block inline size.
-        let opaque_self = OpaqueFlow::from_flow(self);
         let containing_block_size = if flags.contains(IS_ABSOLUTELY_POSITIONED) {
             self.containing_block_size(&layout_context.shared.screen_size, opaque_self).inline
         } else {
@@ -1286,7 +1327,9 @@ impl BlockFlow {
         while let Some((i, kid)) = iterator.next() {
             {
                 let kid_base = flow::mut_base(kid);
-                kid_base.block_container_explicit_block_size = explicit_content_size;
+                if !kid_base.flags.contains(IS_ABSOLUTELY_POSITIONED) {
+                    kid_base.block_container_explicit_block_size = explicit_content_size;
+                }
             }
 
             // Determine float impaction, and update the inline size speculations if necessary.
@@ -1675,7 +1718,7 @@ impl Flow for BlockFlow {
 
             // Assign block-size for fragment if it is an image fragment.
             let containing_block_block_size =
-                self.base.block_container_explicit_block_size.unwrap_or(Au(0));
+                self.base.block_container_explicit_block_size;
             self.fragment.assign_replaced_block_size_if_necessary(containing_block_block_size);
             if !self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) {
                 self.base.position.size.block = self.fragment.border_box.size.block;
diff --git a/components/layout/flow.rs b/components/layout/flow.rs
index 64ee4d2e0d2..d315b9962db 100644
--- a/components/layout/flow.rs
+++ b/components/layout/flow.rs
@@ -1389,7 +1389,7 @@ impl ContainingBlockLink {
     }
 
     #[inline]
-    pub fn generated_containing_block_size(&mut self, for_flow: OpaqueFlow) -> LogicalSize<Au> {
+    pub fn generated_containing_block_size(&self, for_flow: OpaqueFlow) -> LogicalSize<Au> {
         match self.link {
             None => {
                 panic!("Link to containing block not established; perhaps you forgot to call \
@@ -1398,6 +1398,24 @@ impl ContainingBlockLink {
             Some(ref link) => link.upgrade().unwrap().generated_containing_block_size(for_flow),
         }
     }
+
+    #[inline]
+    pub fn explicit_block_containing_size(&self, layout_context: &LayoutContext) -> Option<Au> {
+        match self.link {
+            None => {
+                panic!("Link to containing block not established; perhaps you forgot to call \
+                        `set_absolute_descendants`?")
+            }
+            Some(ref link) => {
+                let flow = link.upgrade().unwrap();
+                if flow.is_block_like() {
+                    flow.as_immutable_block().explicit_block_containing_size(layout_context)
+                } else {
+                    None
+                }
+            }
+        }
+    }
 }
 
 /// A wrapper for the pointer address of a flow. These pointer addresses may only be compared for
diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs
index 7285adca0e3..475535bbe97 100644
--- a/components/layout/fragment.rs
+++ b/components/layout/fragment.rs
@@ -216,10 +216,10 @@ impl fmt::Debug for SpecificFragmentInfo {
 fn clamp_size(size: Au,
               min_size: LengthOrPercentage,
               max_size: LengthOrPercentageOrNone,
-              container_inline_size: Au)
+              container_size: Au)
               -> Au {
-    let min_size = model::specified(min_size, container_inline_size);
-    let max_size = model::specified_or_none(max_size, container_inline_size);
+    let min_size = model::specified(min_size, container_size);
+    let max_size = model::specified_or_none(max_size, container_size);
 
     max(min_size, match max_size {
         None => size,
@@ -442,11 +442,15 @@ impl ReplacedImageFragmentInfo {
     // `style_length`: inline-size as given in the CSS
     pub fn style_length(style_length: LengthOrPercentageOrAuto,
                         dom_length: Option<Au>,
-                        container_inline_size: Au) -> MaybeAuto {
-        match (MaybeAuto::from_style(style_length,container_inline_size), dom_length) {
-            (MaybeAuto::Specified(length), _) => MaybeAuto::Specified(length),
-            (MaybeAuto::Auto, Some(length)) => MaybeAuto::Specified(length),
-            (MaybeAuto::Auto, None) => MaybeAuto::Auto,
+                        container_size: Option<Au>) -> MaybeAuto {
+        match (style_length, dom_length, container_size) {
+            (LengthOrPercentageOrAuto::Length(length), _, _) => MaybeAuto::Specified(length),
+            (LengthOrPercentageOrAuto::Percentage(pc), _, Some(container_size)) => {
+                MaybeAuto::Specified(container_size.scale_by(pc))
+            }
+            (LengthOrPercentageOrAuto::Percentage(_), _, None) => MaybeAuto::Auto,
+            (LengthOrPercentageOrAuto::Auto, Some(dom_length), _) => MaybeAuto::Specified(dom_length),
+            (LengthOrPercentageOrAuto::Auto, None, _) => MaybeAuto::Auto,
         }
     }
 
@@ -468,7 +472,7 @@ impl ReplacedImageFragmentInfo {
         let inline_size = ReplacedImageFragmentInfo::style_length(
             style_inline_size,
             self.dom_inline_size,
-            container_inline_size);
+            Some(container_inline_size));
 
         let inline_size = match inline_size {
             MaybeAuto::Auto => {
@@ -483,7 +487,7 @@ impl ReplacedImageFragmentInfo {
                     let specified_height = ReplacedImageFragmentInfo::style_length(
                         style_block_size,
                         self.dom_block_size,
-                        Au(0));
+                        None);
                     let specified_height = match specified_height {
                         MaybeAuto::Auto => intrinsic_height,
                         MaybeAuto::Specified(h) => h,
@@ -510,7 +514,7 @@ impl ReplacedImageFragmentInfo {
     pub fn calculate_replaced_block_size(&mut self,
                                          style: &ComputedValues,
                                          noncontent_block_size: Au,
-                                         containing_block_block_size: Au,
+                                         containing_block_block_size: Option<Au>,
                                          fragment_inline_size: Au,
                                          fragment_block_size: Au)
                                          -> Au {
@@ -574,12 +578,12 @@ impl IframeFragmentInfo {
         IframeFragmentInfo::calculate_replaced_size(style.content_inline_size(),
                                                     style.min_inline_size(),
                                                     style.max_inline_size(),
-                                                    containing_size,
+                                                    Some(containing_size),
                                                     Au::from_px(300))
     }
 
     #[inline]
-    pub fn calculate_replaced_block_size(&self, style: &ComputedValues, containing_size: Au)
+    pub fn calculate_replaced_block_size(&self, style: &ComputedValues, containing_size: Option<Au>)
                                          -> Au {
         // Calculate the replaced block size (or default) as per CSS 2.1 § 10.3.2
         IframeFragmentInfo::calculate_replaced_size(style.content_block_size(),
@@ -593,13 +597,16 @@ impl IframeFragmentInfo {
     fn calculate_replaced_size(content_size: LengthOrPercentageOrAuto,
                                style_min_size: LengthOrPercentage,
                                style_max_size: LengthOrPercentageOrNone,
-                               containing_size: Au,
+                               containing_size: Option<Au>,
                                default_size: Au) -> Au {
-        let computed_size = match MaybeAuto::from_style(content_size, containing_size) {
-            MaybeAuto::Specified(length) => length,
-            MaybeAuto::Auto => default_size,
+        let computed_size = match (content_size, containing_size) {
+            (LengthOrPercentageOrAuto::Length(length), _) => length,
+            (LengthOrPercentageOrAuto::Percentage(pc), Some(container_size)) => container_size.scale_by(pc),
+            (LengthOrPercentageOrAuto::Percentage(_), None) => default_size,
+            (LengthOrPercentageOrAuto::Auto, _) => default_size,
         };
 
+        let containing_size = containing_size.unwrap_or(Au(0));
         let size = clamp_size(computed_size,
                               style_min_size,
                               style_max_size,
@@ -1714,7 +1721,7 @@ impl Fragment {
     /// been assigned first.
     ///
     /// Ideally, this should follow CSS 2.1 § 10.6.2.
-    pub fn assign_replaced_block_size_if_necessary(&mut self, containing_block_block_size: Au) {
+    pub fn assign_replaced_block_size_if_necessary(&mut self, containing_block_block_size: Option<Au>) {
         match self.specific {
             SpecificFragmentInfo::Generic |
             SpecificFragmentInfo::GeneratedContent(_) |
diff --git a/components/layout/inline.rs b/components/layout/inline.rs
index abf23e488dd..0d8e1e19347 100644
--- a/components/layout/inline.rs
+++ b/components/layout/inline.rs
@@ -1346,7 +1346,7 @@ impl Flow for InlineFlow {
 
         // Assign the block-size and late-computed inline-sizes for the inline fragments.
         let containing_block_block_size =
-            self.base.block_container_explicit_block_size.unwrap_or(Au(0));
+            self.base.block_container_explicit_block_size;
         for fragment in self.fragments.fragments.iter_mut() {
             fragment.update_late_computed_replaced_inline_size_if_necessary();
             fragment.assign_replaced_block_size_if_necessary(
diff --git a/components/layout/list_item.rs b/components/layout/list_item.rs
index a1a108848ac..24ed6633d45 100644
--- a/components/layout/list_item.rs
+++ b/components/layout/list_item.rs
@@ -106,7 +106,7 @@ impl Flow for ListItemFlow {
 
         if let Some(ref mut marker) = self.marker {
             let containing_block_block_size =
-                self.block_flow.base.block_container_explicit_block_size.unwrap_or(Au(0));
+                self.block_flow.base.block_container_explicit_block_size;
             marker.assign_replaced_block_size_if_necessary(containing_block_block_size);
 
             let font_metrics =
diff --git a/components/layout/model.rs b/components/layout/model.rs
index 3c5cda25314..469d618cdfb 100644
--- a/components/layout/model.rs
+++ b/components/layout/model.rs
@@ -126,29 +126,34 @@ impl MarginCollapseInfo {
 
     pub fn finish_and_compute_collapsible_margins(mut self,
                                                   fragment: &Fragment,
+                                                  containing_block_size: Option<Au>,
                                                   can_collapse_block_end_margin_with_kids: bool)
                                                   -> (CollapsibleMargins, Au) {
         let state = match self.state {
             MarginCollapseState::AccumulatingCollapsibleTopMargin => {
-                match fragment.style().content_block_size() {
-                    LengthOrPercentageOrAuto::Auto | LengthOrPercentageOrAuto::Length(Au(0)) |
-                    LengthOrPercentageOrAuto::Percentage(0.) => {
-                        match fragment.style().min_block_size() {
-                            LengthOrPercentage::Length(Au(0)) | LengthOrPercentage::Percentage(0.) => {
-                                FinalMarginState::MarginsCollapseThrough
-                            },
-                            _ => {
-                                // If the fragment has non-zero min-block-size, margins may not
-                                // collapse through it.
-                                FinalMarginState::BottomMarginCollapses
-                            }
+                let may_collapse_through = match fragment.style().content_block_size() {
+                    LengthOrPercentageOrAuto::Auto => true,
+                    LengthOrPercentageOrAuto::Length(Au(0)) => true,
+                    LengthOrPercentageOrAuto::Percentage(0.) => true,
+                    LengthOrPercentageOrAuto::Percentage(_) if containing_block_size.is_none() => true,
+                    _ => false,
+                };
+
+                if may_collapse_through {
+                    match fragment.style().min_block_size() {
+                        LengthOrPercentage::Length(Au(0)) | LengthOrPercentage::Percentage(0.) => {
+                            FinalMarginState::MarginsCollapseThrough
+                        },
+                        _ => {
+                            // If the fragment has non-zero min-block-size, margins may not
+                            // collapse through it.
+                            FinalMarginState::BottomMarginCollapses
                         }
-                    },
-                    _ => {
-                        // If the fragment has an explicitly specified block-size, margins may not
-                        // collapse through it.
-                        FinalMarginState::BottomMarginCollapses
                     }
+                } else {
+                    // If the fragment has an explicitly specified block-size, margins may not
+                    // collapse through it.
+                    FinalMarginState::BottomMarginCollapses
                 }
             }
             MarginCollapseState::AccumulatingMarginIn => FinalMarginState::BottomMarginCollapses,
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index 5ead9ead378..046b9a57c5f 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -390,15 +390,15 @@ impl RawLayoutElementHelpers for Element {
         match height {
             LengthOrPercentageOrAuto::Auto => {}
             LengthOrPercentageOrAuto::Percentage(percentage) => {
-                let width_value = specified::LengthOrPercentageOrAuto::Percentage(percentage);
-                hints.push(from_declaration(PropertyDeclaration::Height(SpecifiedValue(
-                                height::SpecifiedValue(width_value)))));
+                let height_value = specified::LengthOrPercentageOrAuto::Percentage(percentage);
+                hints.push(from_declaration(
+                    PropertyDeclaration::Height(SpecifiedValue(height_value))));
             }
             LengthOrPercentageOrAuto::Length(length) => {
-                let width_value = specified::LengthOrPercentageOrAuto::Length(
+                let height_value = specified::LengthOrPercentageOrAuto::Length(
                     specified::Length::Absolute(length));
-                hints.push(from_declaration(PropertyDeclaration::Height(SpecifiedValue(
-                                height::SpecifiedValue(width_value)))));
+                hints.push(from_declaration(
+                    PropertyDeclaration::Height(SpecifiedValue(height_value))));
             }
         }
 
@@ -443,8 +443,7 @@ impl RawLayoutElementHelpers for Element {
             let value = specified::Length::FontRelative(specified::FontRelativeLength::Em(rows as CSSFloat));
             hints.push(from_declaration(
                 PropertyDeclaration::Height(SpecifiedValue(
-                    longhands::height::SpecifiedValue(
-                        specified::LengthOrPercentageOrAuto::Length(value))))));
+                        specified::LengthOrPercentageOrAuto::Length(value)))));
         }
 
 
diff --git a/components/style/properties.mako.rs b/components/style/properties.mako.rs
index d17241e009f..23618284757 100644
--- a/components/style/properties.mako.rs
+++ b/components/style/properties.mako.rs
@@ -534,45 +534,10 @@ pub mod longhands {
     ${predefined_type("width", "LengthOrPercentageOrAuto",
                       "computed::LengthOrPercentageOrAuto::Auto",
                       "parse_non_negative")}
-    <%self:longhand name="height">
-        use values::computed::Context;
-        use cssparser::ToCss;
-        use std::fmt;
 
-        impl ToCss for SpecifiedValue {
-            fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-                self.0.to_css(dest)
-            }
-        }
-
-        #[derive(Clone, PartialEq)]
-        pub struct SpecifiedValue(pub specified::LengthOrPercentageOrAuto);
-        pub mod computed_value {
-            pub use values::computed::LengthOrPercentageOrAuto as T;
-        }
-        #[inline]
-        pub fn get_initial_value() -> computed_value::T { computed::LengthOrPercentageOrAuto::Auto }
-        #[inline]
-        pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
-            specified::LengthOrPercentageOrAuto::parse_non_negative(input).map(SpecifiedValue)
-        }
-
-        impl ToComputedValue for SpecifiedValue {
-            type ComputedValue = computed_value::T;
-
-            #[inline]
-            fn to_computed_value(&self, context: &Context) -> computed_value::T {
-                match (self.0, context.inherited_height) {
-                    (specified::LengthOrPercentageOrAuto::Percentage(_),
-                     computed::LengthOrPercentageOrAuto::Auto)
-                    if !context.is_root_element && !context.positioned => {
-                        computed::LengthOrPercentageOrAuto::Auto
-                    },
-                    _ => self.0.to_computed_value(context)
-                }
-            }
-        }
-    </%self:longhand>
+    ${predefined_type("height", "LengthOrPercentageOrAuto",
+                      "computed::LengthOrPercentageOrAuto::Auto",
+                      "parse_non_negative")}
 
     ${predefined_type("min-width", "LengthOrPercentage",
                       "computed::LengthOrPercentage::Length(Au(0))",
@@ -6217,7 +6182,6 @@ pub fn cascade(viewport_size: Size2D<Au>,
             viewport_size: viewport_size,
             inherited_font_weight: inherited_font_style.font_weight,
             inherited_font_size: inherited_font_style.font_size,
-            inherited_height: inherited_style.get_box().height,
             inherited_text_decorations_in_effect:
                 inherited_style.get_inheritedtext()._servo_text_decorations_in_effect,
             // To be overridden by applicable declarations:
diff --git a/components/style/values.rs b/components/style/values.rs
index b03a239ae49..03167b3fd36 100644
--- a/components/style/values.rs
+++ b/components/style/values.rs
@@ -873,7 +873,6 @@ pub mod computed {
         pub inherited_font_size: longhands::font_size::computed_value::T,
         pub inherited_text_decorations_in_effect:
             longhands::_servo_text_decorations_in_effect::computed_value::T,
-        pub inherited_height: longhands::height::computed_value::T,
         pub color: longhands::color::computed_value::T,
         pub text_decoration: longhands::text_decoration::computed_value::T,
         pub font_size: longhands::font_size::computed_value::T,
diff --git a/tests/wpt/metadata-css/css21_dev/html4/block-in-inline-percents-001.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/block-in-inline-percents-001.htm.ini
deleted file mode 100644
index 144a6902bd7..00000000000
--- a/tests/wpt/metadata-css/css21_dev/html4/block-in-inline-percents-001.htm.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[block-in-inline-percents-001.htm]
-  type: reftest
-  expected: FAIL