diff --git a/servo/components/layout/construct.rs b/servo/components/layout/construct.rs index 797f7cf07031..d701edfc435d 100644 --- a/servo/components/layout/construct.rs +++ b/servo/components/layout/construct.rs @@ -294,14 +294,16 @@ impl<'a> FlowConstructor<'a> { } #[inline] - fn set_flow_construction_result(&self, node: &ThreadSafeLayoutNode, result: ConstructionResult) { - match result { - ConstructionResult::None => { - let mut layout_data_ref = node.mutate_layout_data(); - let layout_data = layout_data_ref.as_mut().expect("no layout data"); - layout_data.remove_compositor_layers(self.layout_context.shared.constellation_chan.clone()); - } - _ => {} + fn set_flow_construction_result(&self, + node: &ThreadSafeLayoutNode, + result: ConstructionResult) { + if let ConstructionResult::None = result { + let mut layout_data_ref = node.mutate_layout_data(); + let layout_data = layout_data_ref.as_mut().expect("no layout data"); + layout_data.remove_compositor_layers(self.layout_context + .shared + .constellation_chan + .clone()); } node.set_flow_construction_result(result); @@ -467,8 +469,8 @@ impl<'a> FlowConstructor<'a> { // Set up absolute descendants as necessary. // - // TODO(pcwalton): The inline flow itself may need to become the containing block for - // absolute descendants in order to handle cases like: + // The inline flow itself may need to become the containing block for absolute descendants + // in order to handle cases like: // //
// @@ -477,6 +479,7 @@ impl<'a> FlowConstructor<'a> { //
// // See the comment above `flow::AbsoluteDescendantInfo` for more information. + inline_flow_ref.take_applicable_absolute_descendants(&mut fragments.absolute_descendants); absolute_descendants.push_descendants(fragments.absolute_descendants); { @@ -878,6 +881,15 @@ impl<'a> FlowConstructor<'a> { if opt_inline_block_splits.len() > 0 || !fragment_accumulator.fragments.is_empty() || abs_descendants.len() > 0 { fragment_accumulator.fragments.absolute_descendants.push_descendants(abs_descendants); + + // If the node is positioned, then it's the containing block for all absolutely- + // positioned descendants. + if node.style().get_box().position != position::T::static_ { + fragment_accumulator.fragments + .absolute_descendants + .mark_as_having_reached_containing_block(); + } + let construction_item = ConstructionItem::InlineFragments( InlineFragmentsConstructionResult { splits: opt_inline_block_splits, diff --git a/servo/components/layout/flow.rs b/servo/components/layout/flow.rs index 4e2da89f5133..beb3b00cfef4 100644 --- a/servo/components/layout/flow.rs +++ b/servo/components/layout/flow.rs @@ -510,6 +510,17 @@ pub trait MutableOwnedFlowUtils { /// /// Set this flow as the Containing Block for all the absolute descendants. fn set_absolute_descendants(&mut self, abs_descendants: AbsoluteDescendants); + + /// Sets the flow as the containing block for all absolute descendants that have been marked + /// as having reached their containing block. This is needed in order to handle cases like: + /// + ///
+ /// + /// + /// + ///
+ fn take_applicable_absolute_descendants(&mut self, + absolute_descendants: &mut AbsoluteDescendants); } #[derive(RustcEncodable, PartialEq, Debug)] @@ -731,6 +742,7 @@ impl AbsoluteDescendants { pub fn push(&mut self, given_descendant: FlowRef) { self.descendant_links.push(AbsoluteDescendantInfo { flow: given_descendant, + has_reached_containing_block: false, }); } @@ -749,29 +761,38 @@ impl AbsoluteDescendants { iter: self.descendant_links.iter_mut(), } } + + /// Mark these descendants as having reached their containing block. + pub fn mark_as_having_reached_containing_block(&mut self) { + for descendant_info in self.descendant_links.iter_mut() { + descendant_info.has_reached_containing_block = true + } + } } -/// TODO(pcwalton): This structure is going to need a flag to record whether the absolute -/// descendants have reached their containing block yet. The reason is so that we can handle cases -/// like the following: -/// -///
-/// foo -/// -/// bar -/// -///
-/// -/// When we go to create the `InlineFlow` for the outer `div`, our absolute descendants will -/// be `a` and `b`. At this point, we need a way to distinguish between the two, because the -/// containing block for `a` will be different from the containing block for `b`. Specifically, -/// the latter's containing block is the inline flow itself, while the former's containing -/// block is going to be some parent of the outer `div`. Hence we need this flag as a way to -/// distinguish the two; it will be false for `a` and true for `b`. +/// Information about each absolutely-positioned descendant of the given flow. #[derive(Clone)] pub struct AbsoluteDescendantInfo { /// The absolute descendant flow in question. flow: FlowRef, + + /// Whether the absolute descendant has reached its containing block. This exists so that we + /// can handle cases like the following: + /// + ///
+ /// foo + /// + /// bar + /// + ///
+ /// + /// When we go to create the `InlineFlow` for the outer `div`, our absolute descendants will + /// be `a` and `b`. At this point, we need a way to distinguish between the two, because the + /// containing block for `a` will be different from the containing block for `b`. Specifically, + /// the latter's containing block is the inline flow itself, while the former's containing + /// block is going to be some parent of the outer `div`. Hence we need this flag as a way to + /// distinguish the two; it will be false for `a` and true for `b`. + has_reached_containing_block: bool, } pub struct AbsoluteDescendantIter<'a> { @@ -1414,6 +1435,36 @@ impl MutableOwnedFlowUtils for FlowRef { let this = self.clone(); let base = mut_base(flow_ref::deref_mut(self)); base.abs_descendants = abs_descendants; + for descendant_link in base.abs_descendants.descendant_links.iter_mut() { + debug_assert!(!descendant_link.has_reached_containing_block); + let descendant_base = mut_base(flow_ref::deref_mut(&mut descendant_link.flow)); + descendant_base.absolute_cb.set(this.clone()); + } + } + + /// Sets the flow as the containing block for all absolute descendants that have been marked + /// as having reached their containing block. This is needed in order to handle cases like: + /// + ///
+ /// + /// + /// + ///
+ fn take_applicable_absolute_descendants(&mut self, + absolute_descendants: &mut AbsoluteDescendants) { + let mut applicable_absolute_descendants = AbsoluteDescendants::new(); + for absolute_descendant in absolute_descendants.descendant_links.iter() { + if absolute_descendant.has_reached_containing_block { + applicable_absolute_descendants.push(absolute_descendant.flow.clone()); + } + } + absolute_descendants.descendant_links.retain(|descendant| { + !descendant.has_reached_containing_block + }); + + let this = self.clone(); + let base = mut_base(flow_ref::deref_mut(self)); + base.abs_descendants = applicable_absolute_descendants; for descendant_link in base.abs_descendants.iter() { let descendant_base = mut_base(descendant_link); descendant_base.absolute_cb.set(this.clone()); diff --git a/servo/components/layout/fragment.rs b/servo/components/layout/fragment.rs index 3b9596d27b94..309522ab43b6 100644 --- a/servo/components/layout/fragment.rs +++ b/servo/components/layout/fragment.rs @@ -2272,6 +2272,11 @@ impl Fragment { } false } + + /// Returns true if this node is absolutely positioned. + pub fn is_absolutely_positioned(&self) -> bool { + self.style.get_box().position == position::T::absolute + } } impl fmt::Debug for Fragment { diff --git a/servo/components/layout/inline.rs b/servo/components/layout/inline.rs index 17d6af7ccbce..a47e600ac28d 100644 --- a/servo/components/layout/inline.rs +++ b/servo/components/layout/inline.rs @@ -1277,7 +1277,7 @@ impl InlineFlow { } fn containing_block_range_for_flow(&self, opaque_flow: OpaqueFlow) -> Range { - let index = FragmentIndex(self.fragments.fragments.iter().position(|fragment| { + match self.fragments.fragments.iter().position(|fragment| { match fragment.specific { SpecificFragmentInfo::InlineAbsolute(ref inline_absolute) => { OpaqueFlow::from_flow(&*inline_absolute.flow_ref) == opaque_flow @@ -1288,9 +1288,19 @@ impl InlineFlow { } _ => false, } - }).expect("containing_block_range_for_flow(): couldn't find inline absolute fragment!") - as isize); - self.containing_block_range_for_flow_surrounding_fragment_at_index(index) + }) { + Some(index) => { + let index = FragmentIndex(index as isize); + self.containing_block_range_for_flow_surrounding_fragment_at_index(index) + } + None => { + // FIXME(pcwalton): This is quite wrong. We should only return the range + // surrounding the inline fragments that constitute the containing block. But this + // suffices to get Google looking right. + Range::new(FragmentIndex(0), + FragmentIndex(self.fragments.fragments.len() as isize)) + } + } } } @@ -1766,9 +1776,7 @@ impl Flow for InlineFlow { } fn contains_positioned_fragments(&self) -> bool { - self.fragments.fragments.iter().any(|fragment| { - fragment.style.get_box().position != position::T::static_ - }) + self.fragments.fragments.iter().any(|fragment| fragment.is_positioned()) } fn contains_relatively_positioned_fragments(&self) -> bool { @@ -1781,10 +1789,13 @@ impl Flow for InlineFlow { let mut containing_block_size = LogicalSize::new(self.base.writing_mode, Au(0), Au(0)); for index in self.containing_block_range_for_flow(for_flow).each_index() { let fragment = &self.fragments.fragments[index.get() as usize]; + if fragment.is_absolutely_positioned() { + continue + } containing_block_size.inline = containing_block_size.inline + fragment.border_box.size.inline; containing_block_size.block = max(containing_block_size.block, - fragment.border_box.size.block) + fragment.border_box.size.block); } containing_block_size }