diff options
| -rw-r--r-- | components/layout/construct.rs | 32 | ||||
| -rw-r--r-- | components/layout/flow.rs | 85 | ||||
| -rw-r--r-- | components/layout/fragment.rs | 5 | ||||
| -rw-r--r-- | components/layout/inline.rs | 27 | ||||
| -rw-r--r-- | tests/ref/basic.list | 1 | ||||
| -rw-r--r-- | tests/ref/simple_inline_absolute_containing_block_a.html | 26 | ||||
| -rw-r--r-- | tests/ref/simple_inline_absolute_containing_block_ref.html | 4 |
7 files changed, 145 insertions, 35 deletions
diff --git a/components/layout/construct.rs b/components/layout/construct.rs index 797f7cf0703..d701edfc435 100644 --- a/components/layout/construct.rs +++ b/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: // // <div> // <span style="position: relative"> @@ -477,6 +479,7 @@ impl<'a> FlowConstructor<'a> { // </div> // // 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/components/layout/flow.rs b/components/layout/flow.rs index 6d319bdd991..f606a8355f9 100644 --- a/components/layout/flow.rs +++ b/components/layout/flow.rs @@ -502,6 +502,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: + /// + /// <div> + /// <span style="position: relative"> + /// <span style="position: absolute; ..."></span> + /// </span> + /// </div> + fn take_applicable_absolute_descendants(&mut self, + absolute_descendants: &mut AbsoluteDescendants); } #[derive(RustcEncodable, PartialEq, Debug)] @@ -723,6 +734,7 @@ impl AbsoluteDescendants { pub fn push(&mut self, given_descendant: FlowRef) { self.descendant_links.push(AbsoluteDescendantInfo { flow: given_descendant, + has_reached_containing_block: false, }); } @@ -741,29 +753,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: -/// -/// <div> -/// <span id=a style="position: absolute; ...">foo</span> -/// <span style="position: relative"> -/// <span id=b style="position: absolute; ...">bar</span> -/// </span> -/// </div> -/// -/// 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: + /// + /// <div> + /// <span id=a style="position: absolute; ...">foo</span> + /// <span style="position: relative"> + /// <span id=b style="position: absolute; ...">bar</span> + /// </span> + /// </div> + /// + /// 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> { @@ -1368,6 +1389,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: + /// + /// <div> + /// <span style="position: relative"> + /// <span style="position: absolute; ..."></span> + /// </span> + /// </div> + 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/components/layout/fragment.rs b/components/layout/fragment.rs index 749a3cf5660..c869f870cf1 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -2273,6 +2273,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/components/layout/inline.rs b/components/layout/inline.rs index 4202b6b9d16..8893a6efb76 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -1277,7 +1277,7 @@ impl InlineFlow { } fn containing_block_range_for_flow(&self, opaque_flow: OpaqueFlow) -> Range<FragmentIndex> { - 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)) + } + } } } @@ -1762,9 +1772,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 { @@ -1777,10 +1785,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 } diff --git a/tests/ref/basic.list b/tests/ref/basic.list index c2c3adf2963..b2d15cc936b 100644 --- a/tests/ref/basic.list +++ b/tests/ref/basic.list @@ -312,6 +312,7 @@ flaky_cpu == linebreak_simple_a.html linebreak_simple_b.html == rtl_table_a.html rtl_table_ref.html == servo_center_a.html servo_center_ref.html == setattribute_id_restyle_a.html setattribute_id_restyle_b.html +== simple_inline_absolute_containing_block_a.html simple_inline_absolute_containing_block_ref.html == stacking_context_overflow_a.html stacking_context_overflow_ref.html == stacking_context_overflow_relative_outline_a.html stacking_context_overflow_relative_outline_ref.html == style_is_in_doc.html style_is_in_doc_ref.html diff --git a/tests/ref/simple_inline_absolute_containing_block_a.html b/tests/ref/simple_inline_absolute_containing_block_a.html new file mode 100644 index 00000000000..3050a534268 --- /dev/null +++ b/tests/ref/simple_inline_absolute_containing_block_a.html @@ -0,0 +1,26 @@ +<!DOCTYPE html> +<style> +main { + position: relative; + padding: 0 16px; + display: inline; +} +section { + position: absolute; + top: 25%; + bottom: 25%; + left: 0; + right: 0; + background: red; +} +#coveritup { + position: relative; + background: white; + top: -20px; + display: inline; + color: white; +} +</style> +<div>There should be no red.<main><section></section></main></div> +<div>There should be no red.<div id=coveritup>XXXXX</div></div> + diff --git a/tests/ref/simple_inline_absolute_containing_block_ref.html b/tests/ref/simple_inline_absolute_containing_block_ref.html new file mode 100644 index 00000000000..413ac37355f --- /dev/null +++ b/tests/ref/simple_inline_absolute_containing_block_ref.html @@ -0,0 +1,4 @@ +<!DOCTYPE html> +<div>There should be no red.</div> +<div>There should be no red.</div> + |