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.
This commit is contained in:
Glenn Watson 2015-08-03 09:00:46 +10:00
parent ffe4bd25a4
commit b9fea3deb3
10 changed files with 151 additions and 119 deletions

View file

@ -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;
}
let opaque_self = OpaqueFlow::from_flow(self);
// 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 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),
};
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;

View file

@ -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

View file

@ -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(_) |

View file

@ -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(

View file

@ -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 =

View file

@ -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,

View file

@ -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)))));
}

View file

@ -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:

View file

@ -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,

View file

@ -1,3 +0,0 @@
[block-in-inline-percents-001.htm]
type: reftest
expected: FAIL