Merge pull request #3492 from pcwalton/clear-generated-content-table

layout: Support any `display` property in generated content, and allow

Reviewed-by: glennw

components/layout/construct.rs

@@ -45,7 +45,7 @@ use table_cell::TableCellFlow;
 use text::TextRunScanner;
 use util::{LayoutDataAccess, OpaqueNodeMethods};
 use wrapper::{PostorderNodeMutTraversal, TLayoutNode, ThreadSafeLayoutNode};
-use wrapper::{Before, BeforeBlock, After, AfterBlock, Normal};
+use wrapper::{Before, After, Normal};
 
 use gfx::display_list::OpaqueNode;
 use script::dom::element::{HTMLIFrameElementTypeId, HTMLImageElementTypeId};
@@ -887,8 +887,9 @@ impl<'a> PostorderNodeMutTraversal for FlowConstructor<'a> {
                 // Pseudo-element.
                 let style = node.style();
                 let display = match node.get_pseudo_element_type() {
-                    Normal | Before | After => display::inline,
+                    Normal => display::inline,
-                    BeforeBlock | AfterBlock => display::block,
+                    Before(display) => display,
+                    After(display) => display,
                 };
                 (display, style.get_box().float, style.get_box().position)
             }
@@ -1041,12 +1042,8 @@ impl<'ln> NodeUtils for ThreadSafeLayoutNode<'ln> {
         match &mut *layout_data_ref {
             &Some(ref mut layout_data) =>{
                 match self.get_pseudo_element_type() {
-                    Before | BeforeBlock => {
+                    Before(_) => layout_data.data.before_flow_construction_result = result,
-                        layout_data.data.before_flow_construction_result = result
+                    After(_) => layout_data.data.after_flow_construction_result = result,
-                    },
-                    After | AfterBlock => {
-                        layout_data.data.after_flow_construction_result = result
-                    },
                     Normal => layout_data.data.flow_construction_result = result,
                 }
             },
@@ -1060,11 +1057,11 @@ impl<'ln> NodeUtils for ThreadSafeLayoutNode<'ln> {
         match &mut *layout_data_ref {
             &Some(ref mut layout_data) => {
                 match self.get_pseudo_element_type() {
-                    Before | BeforeBlock => {
+                    Before(_) => {
                         mem::replace(&mut layout_data.data.before_flow_construction_result,
                                      NoConstructionResult)
                     }
-                    After | AfterBlock => {
+                    After(_) => {
                         mem::replace(&mut layout_data.data.after_flow_construction_result,
                                      NoConstructionResult)
                     }

components/layout/css/node_util.rs

@@ -5,7 +5,7 @@
 use incremental::RestyleDamage;
 use util::LayoutDataAccess;
 use wrapper::{TLayoutNode, ThreadSafeLayoutNode};
-use wrapper::{After, AfterBlock, Before, BeforeBlock, Normal};
+use wrapper::{After, Before, Normal};
 use std::mem;
 use style::ComputedValues;
 use sync::Arc;
@@ -26,7 +26,7 @@ impl<'ln> NodeUtil for ThreadSafeLayoutNode<'ln> {
         unsafe {
             let layout_data_ref = self.borrow_layout_data();
             match self.get_pseudo_element_type() {
-                Before | BeforeBlock => {
+                Before(_) => {
                      mem::transmute(layout_data_ref.as_ref()
                                                    .unwrap()
                                                    .data
@@ -34,7 +34,7 @@ impl<'ln> NodeUtil for ThreadSafeLayoutNode<'ln> {
                                                    .as_ref()
                                                    .unwrap())
                 }
-                After | AfterBlock => {
+                After(_) => {
                     mem::transmute(layout_data_ref.as_ref()
                                                   .unwrap()
                                                   .data

components/layout/table_wrapper.rs

@@ -19,7 +19,7 @@ use wrapper::ThreadSafeLayoutNode;
 use servo_util::geometry::Au;
 use std::cmp::max;
 use std::fmt;
-use style::computed_values::{float, table_layout};
+use style::computed_values::{clear, float, table_layout};
 
 #[deriving(Encodable)]
 pub enum TableLayout {
@@ -252,6 +252,10 @@ impl Flow for TableWrapperFlow {
         &mut self.block_flow
     }
 
+    fn float_clearance(&self) -> clear::T {
+        self.block_flow.float_clearance()
+    }
+
     fn float_kind(&self) -> float::T {
         self.block_flow.float_kind()
     }

components/layout/wrapper.rs

@@ -465,10 +465,24 @@ fn get_content(content_list: &content::T) -> String {
 #[deriving(PartialEq, Clone)]
 pub enum PseudoElementType {
     Normal,
-    Before,
+    Before(display::T),
-    After,
+    After(display::T),
-    BeforeBlock,
+}
-    AfterBlock,
+
+impl PseudoElementType {
+    pub fn is_before(&self) -> bool {
+        match *self {
+            Before(_) => true,
+            _ => false,
+        }
+    }
+
+    pub fn is_after(&self) -> bool {
+        match *self {
+            After(_) => true,
+            _ => false,
+        }
+    }
 }
 
 /// A thread-safe version of `LayoutNode`, used during flow construction. This type of layout
@@ -516,12 +530,18 @@ impl<'ln> TLayoutNode for ThreadSafeLayoutNode<'ln> {
         }
 
         if self.has_before_pseudo() {
-            if self.is_block(Before) && self.pseudo == Normal {
+            // FIXME(pcwalton): This logic looks weird. Is it right?
-                let pseudo_before_node = self.with_pseudo(BeforeBlock);
+            match self.pseudo {
-                return Some(pseudo_before_node)
+                Normal => {
-            } else if self.pseudo == Normal || self.pseudo == BeforeBlock {
+                    let pseudo_before_node = self.with_pseudo(Before(self.get_before_display()));
-                let pseudo_before_node = self.with_pseudo(Before);
+                    return Some(pseudo_before_node)
-                return Some(pseudo_before_node)
+                }
+                Before(display::inline) => {}
+                Before(_) => {
+                    let pseudo_before_node = self.with_pseudo(Before(display::inline));
+                    return Some(pseudo_before_node)
+                }
+                _ => {}
             }
         }
 
@@ -535,7 +555,7 @@ impl<'ln> TLayoutNode for ThreadSafeLayoutNode<'ln> {
             let layout_data_ref = self.borrow_layout_data();
             let node_layout_data_wrapper = layout_data_ref.as_ref().unwrap();
 
-            if self.pseudo == Before || self.pseudo == BeforeBlock {
+            if self.pseudo.is_before() {
                 let before_style = node_layout_data_wrapper.data.before_style.as_ref().unwrap();
                 return get_content(&before_style.get_box().content)
             } else {
@@ -574,7 +594,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
 
     /// Returns the next sibling of this node. Unsafe and private because this can lead to races.
     unsafe fn next_sibling(&self) -> Option<ThreadSafeLayoutNode<'ln>> {
-        if self.pseudo == Before || self.pseudo == BeforeBlock {
+        if self.pseudo.is_before() {
             return self.get_jsmanaged().first_child_ref().map(|node| self.new_with_this_lifetime(&node))
         }
 
@@ -608,26 +628,25 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
         self.pseudo
     }
 
-    pub fn is_block(&self, kind: PseudoElementType) -> bool {
+    pub fn get_normal_display(&self) -> display::T {
         let mut layout_data_ref = self.mutate_layout_data();
         let node_layout_data_wrapper = layout_data_ref.as_mut().unwrap();
+        let style = node_layout_data_wrapper.shared_data.style.as_ref().unwrap();
+        style.get_box().display
+    }
 
-        let display = match kind {
+    pub fn get_before_display(&self) -> display::T {
-            Before | BeforeBlock => {
+        let mut layout_data_ref = self.mutate_layout_data();
-                let before_style = node_layout_data_wrapper.data.before_style.as_ref().unwrap();
+        let node_layout_data_wrapper = layout_data_ref.as_mut().unwrap();
-                before_style.get_box().display
+        let style = node_layout_data_wrapper.data.before_style.as_ref().unwrap();
-            }
+        style.get_box().display
-            After | AfterBlock => {
+    }
-                let after_style = node_layout_data_wrapper.data.after_style.as_ref().unwrap();
-                after_style.get_box().display
-            }
-            Normal => {
-                let after_style = node_layout_data_wrapper.shared_data.style.as_ref().unwrap();
-                after_style.get_box().display
-            }
-        };
 
-        display == display::block
+    pub fn get_after_display(&self) -> display::T {
+        let mut layout_data_ref = self.mutate_layout_data();
+        let node_layout_data_wrapper = layout_data_ref.as_mut().unwrap();
+        let style = node_layout_data_wrapper.data.after_style.as_ref().unwrap();
+        style.get_box().display
     }
 
     pub fn has_before_pseudo(&self) -> bool {
@@ -722,7 +741,7 @@ impl<'a> Iterator<ThreadSafeLayoutNode<'a>> for ThreadSafeLayoutNodeChildrenIter
 
         match node {
             Some(ref node) => {
-                if node.pseudo == After || node.pseudo == AfterBlock {
+                if node.pseudo.is_after() {
                     return None
                 }
 
@@ -745,12 +764,9 @@ impl<'a> Iterator<ThreadSafeLayoutNode<'a>> for ThreadSafeLayoutNodeChildrenIter
                 match self.parent_node {
                     Some(ref parent_node) => {
                         if parent_node.has_after_pseudo() {
-                            let pseudo_after_node = if parent_node.is_block(After) && parent_node.pseudo == Normal {
+                            let pseudo_after_node = if parent_node.pseudo == Normal {
-                                let pseudo_after_node = parent_node.with_pseudo(AfterBlock);
+                                let pseudo = After(parent_node.get_after_display());
-                                Some(pseudo_after_node)
+                                Some(parent_node.with_pseudo(pseudo))
-                            } else if parent_node.pseudo == Normal {
-                                let pseudo_after_node = parent_node.with_pseudo(After);
-                                Some(pseudo_after_node)
                             } else {
                                 None
                             };

tests/ref/basic.list

@@ -149,3 +149,4 @@ flaky_linux == acid2_noscroll.html acid2_ref_broken.html
 == margins_inside_floats_a.html margins_inside_floats_ref.html
 == block_formatting_context_complex_a.html block_formatting_context_complex_ref.html
 == block_formatting_context_containing_floats_a.html block_formatting_context_containing_floats_ref.html
+== clear_generated_content_table_a.html clear_generated_content_table_ref.html

tests/ref/clear_generated_content_table_a.html

@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.clearit:after {
+    content: "";
+    clear: both;
+    display: table;
+}
+</style>
+</head>
+<body>
+<div class=clearit>
+    <div style="float: left;">x</div>
+</div>
+<div style="float: left;">y</div>
+</body>
+</html>
+

tests/ref/clear_generated_content_table_ref.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div class=clearit>
+    <div style="float: left;">x</div>
+</div>
+<div style="display: table; clear: both;"></div>
+<div style="float: left;">y</div>
+</body>
+</html>
+
+