layout: Adjust the position of block formatting contexts based on float

placement.

Improves Reddit considerably.

Closes #3456.

5 files changed, 58 insertions, 3 deletions

diff --git a/components/layout/block.rs b/components/layout/block.rs
index a75a2e48274..0d8d4b2902f 100644
--- a/components/layout/block.rs
+++ b/components/layout/block.rs
@@ -16,7 +16,7 @@
 
 use construct::FlowConstructor;
 use context::LayoutContext;
-use floats::{ClearBoth, ClearLeft, ClearRight, FloatKind, Floats, PlacementInfo};
+use floats::{ClearBoth, ClearLeft, ClearRight, FloatKind, FloatLeft, Floats, PlacementInfo};
 use flow::{BaseFlow, BlockFlowClass, FlowClass, Flow, ImmutableFlowUtils};
 use flow::{MutableFlowUtils, PreorderFlowTraversal, PostorderFlowTraversal, mut_base};
 use flow;
@@ -37,7 +37,7 @@ use gfx::display_list::{FloatStackingLevel, PositionedDescendantStackingLevel};
 use gfx::display_list::{RootOfStackingContextLevel};
 use gfx::render_task::RenderLayer;
 use servo_msg::compositor_msg::{FixedPosition, LayerId, Scrollable};
-use servo_util::geometry::Au;
+use servo_util::geometry::{Au, MAX_AU};
 use servo_util::logical_geometry::WritingMode;
 use servo_util::logical_geometry::{LogicalPoint, LogicalRect, LogicalSize};
 use std::cmp::{max, min};
@@ -1463,6 +1463,39 @@ impl BlockFlow {
             _ => NonformattingContext,
         }
     }
+
+    /// Per CSS 2.1 § 9.5, block formatting contexts' inline widths and positions are affected by
+    /// the presence of floats. This is the part of the assign-heights traversal that computes
+    /// the final inline position and width for such flows.
+    ///
+    /// Note that this is part of the assign-block-sizes traversal, not the assign-inline-sizes
+    /// traversal as one might expect. That is because, in general, float placement cannot occur
+    /// until heights are assigned. To work around this unfortunate circular dependency, by the
+    /// time we get here we have already estimated the width of the block formatting context based
+    /// on the floats we could see at the time of inline-size assignment. The job of this function,
+    /// therefore, is not only to assign the final size but also to perform the layout again for
+    /// this block formatting context if our speculation was wrong.
+    fn assign_inline_position_for_formatting_context(&mut self) {
+        debug_assert!(self.formatting_context_type() != NonformattingContext);
+
+        let info = PlacementInfo {
+            size: LogicalSize::new(
+                self.fragment.style.writing_mode,
+                self.base.position.size.inline + self.fragment.margin.inline_start_end() +
+                    self.fragment.border_padding.inline_start_end(),
+                self.fragment.border_box.size.block),
+            ceiling: self.base.position.start.b,
+            max_inline_size: MAX_AU,
+            kind: FloatLeft,
+        };
+
+        // Offset our position by whatever displacement is needed to not impact the floats.
+        let rect = self.base.floats.place_between_floats(&info);
+        self.base.position.start.i = self.base.position.start.i + rect.start.i;
+
+        // TODO(pcwalton): If the inline-size of this flow is different from the size we estimated
+        // earlier, lay it out again.
+    }
 }
 
 impl Flow for BlockFlow {
@@ -1626,7 +1659,7 @@ impl Flow for BlockFlow {
             }
         }
 
-        // Move in from the inline-start border edge
+        // Move in from the inline-start border edge.
         let inline_start_content_edge = self.fragment.border_box.start.i + self.fragment.border_padding.inline_start;
         let padding_and_borders = self.fragment.border_padding.inline_start_end();
         let content_inline_size = self.fragment.border_box.size.inline - padding_and_borders;
@@ -1652,6 +1685,10 @@ impl Flow for BlockFlow {
             return true
         }
 
+        if self.formatting_context_type() != NonformattingContext {
+            self.assign_inline_position_for_formatting_context();
+        }
+
         let impacted = self.base.flags.impacted_by_floats();
         if impacted {
             self.assign_block_size(layout_context);
diff --git a/components/util/geometry.rs b/components/util/geometry.rs
index bc18ce66b8b..0bf7cffe60c 100644
--- a/components/util/geometry.rs
+++ b/components/util/geometry.rs
@@ -9,6 +9,7 @@ use geom::size::Size2D;
 
 use serialize::{Encodable, Encoder};
 use std::default::Default;
+use std::i32;
 use std::num::{NumCast, Zero};
 use std::fmt;
 
@@ -72,6 +73,8 @@ impl Default for Au {
     }
 }
 
+pub static MAX_AU: Au = Au(i32::MAX);
+
 impl<E, S: Encoder<E>> Encodable<S, E> for Au {
     fn encode(&self, e: &mut S) -> Result<(), E> {
         e.emit_f64(to_frac_px(*self))
diff --git a/tests/ref/basic.list b/tests/ref/basic.list
index 691ef8b4d12..c67aed95d13 100644
--- a/tests/ref/basic.list
+++ b/tests/ref/basic.list
@@ -139,3 +139,4 @@ flaky_gpu,flaky_linux == acid2_noscroll.html acid2_ref_broken.html
 == img_block_maxwidth_a.html img_block_maxwidth_ref.html
 == img_block_maxwidth_b.html img_block_maxwidth_ref.html
 == float_clearance_a.html float_clearance_ref.html
+== block_formatting_context_a.html block_formatting_context_ref.html
diff --git a/tests/ref/block_formatting_context_a.html b/tests/ref/block_formatting_context_a.html
new file mode 100644
index 00000000000..241de2e22e4
--- /dev/null
+++ b/tests/ref/block_formatting_context_a.html
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<html>
+<body>
+    <div style="float: left;">4913</div>
+    <div style="overflow: hidden;">RIP Richard Kiel</div>
+</body>
+</html>
diff --git a/tests/ref/block_formatting_context_ref.html b/tests/ref/block_formatting_context_ref.html
new file mode 100644
index 00000000000..55b4e3a6fb2
--- /dev/null
+++ b/tests/ref/block_formatting_context_ref.html
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<html>
+<body>
+    <div style="float: left;">4913</div>
+    <div>RIP Richard Kiel</div>
+</body>
+</html>