layout: Implement partial support for inline absolute containing blocks.

Improves the position of the down arrows on google.com SERPs.
This commit is contained in:
Patrick Walton 2015-08-31 18:56:52 -07:00
parent 9f85370885
commit 676f7014e8
7 changed files with 145 additions and 35 deletions

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -0,0 +1,4 @@
<!DOCTYPE html>
<div>There should be no red.</div>
<div>There should be no red.</div>