[CSS Regions] Elements in a region should be assignable to a named flow

Source/core/dom/NodeRenderingContext.cpp

Index: Source/core/dom/NodeRenderingContext.cpp
diff --git a/Source/core/dom/NodeRenderingContext.cpp b/Source/core/dom/NodeRenderingContext.cpp
index 984dfb8de32a0fb39c3c389d53bd3afd829a6a97..a588dc963b5ccc92947afed2fb8d949de8ae6a0a 100644
--- a/Source/core/dom/NodeRenderingContext.cpp
+++ b/Source/core/dom/NodeRenderingContext.cpp
@@ -187,34 +187,32 @@ bool NodeRenderingContext::shouldCreateRenderer() const
 void NodeRenderingContext::moveToFlowThreadIfNeeded()
 {
     ASSERT(m_node->isElementNode());
-    ASSERT(m_style);
-    if (!RuntimeEnabledFeatures::cssRegionsEnabled())
-        return;
+    Element* element = toElement(m_node);

[esprehn, 2013/06/28 20:29:36]

You don't need the ASSERT( before this since toElement does it for you.

 
-    if (m_style->flowThread().isEmpty())
+    if (!RuntimeEnabledFeatures::cssRegionsEnabled())
         return;
 
-    // As per http://dev.w3.org/csswg/css3-regions/#flow-into, pseudo-elements such as ::first-line, ::first-letter, ::before or ::after
-    // cannot be directly collected into a named flow.
-    if (m_node->isPseudoElement())
-        return;
+    if (!m_style)
+        m_style = element->styleForRenderer();
 
-    // FIXME: Do not collect elements if they are in shadow tree.
-    if (m_node->isInShadowTree())
+    if (!element->shouldMoveToFlowThread(m_style.get()))
         return;
 
-    if (m_node->isElementNode() && FullscreenController::isActiveFullScreenElement(toElement(m_node)))
-        return;
+    moveToFlowThread();
+}
 
-    // Allow only svg root elements to be directly collected by a render flow thread.
-    if (m_node->isSVGElement()
-        && (!(m_node->hasTagName(SVGNames::svgTag) && m_node->parentNode() && !m_node->parentNode()->isSVGElement())))
-        return;
+void NodeRenderingContext::moveToFlowThread()

[esprehn, 2013/06/28 20:29:36]

Why is this a separate method now? Lets just merge it into
mergeToFlowThreadIfNeeded()

+{
+    ASSERT(m_node->isElementNode());
+    Element* element = toElement(m_node);
 
-    m_flowThread = m_style->flowThread();
+    if (!m_style)
+        m_style = element->styleForRenderer();
+    ASSERT(m_style);
     ASSERT(m_node->document()->renderView());
+    ASSERT(element->shouldMoveToFlowThread(m_style.get()));
     FlowThreadController* flowThreadController = m_node->document()->renderView()->flowThreadController();
-    m_parentFlowRenderer = flowThreadController->ensureRenderFlowThreadWithName(m_flowThread);
+    m_parentFlowRenderer = flowThreadController->ensureRenderFlowThreadWithName(m_style->flowThread());
     flowThreadController->registerNamedFlowContentNode(m_node, m_parentFlowRenderer);
 }
 
@@ -234,8 +232,26 @@ void NodeRenderingContext::createRendererForElementIfNeeded()
 
     Element* element = toElement(m_node);
 
-    if (!shouldCreateRenderer())
-        return;
+    if (!shouldCreateRenderer()) {
+        // Check the specific case of elements that are children of regions but are flowed into a flow thread themselves.

[esprehn, 2013/06/28 20:29:36]

This whole check should be in a method.

+        bool shouldBeInNamedFlow = false;
+        RenderObject* parentRenderer = this->parentRenderer();
+        if ((parentRenderer && !parentRenderer->canHaveChildren() && parentRenderer->isRenderRegion())
+            || (!parentRenderer && element->parentElement() && element->parentElement()->isDOMChildOfRegion())) {
+            if (!m_style)
+                m_style = element->styleForRenderer();

[esprehn, 2013/06/28 20:29:36]

This doesn't seem right, We should put  if (!m_style) m_style =
element->styleForRenderer(); in one single place, not in lots of methods.

+
+            shouldBeInNamedFlow = element && element->shouldMoveToFlowThread(m_style.get());

[esprehn, 2013/06/28 20:29:36]

element can't be null, you don't need element &&

+
+            // Children of this element will only be allowed to be flowed into other flow-threads if display is NOT none.
+            if (element->rendererIsNeeded(*this))
+                element->setIsDOMChildOfRegion(true);

[esprehn, 2013/06/28 20:29:36]

You never reset this to false so if I remove a child and put it somewhere else
in the tree it's still marked inside regions.

+        }
+
+        if (!shouldBeInNamedFlow)
+            return;
+    }
+
     if (!m_style)
         m_style = element->styleForRenderer();
     ASSERT(m_style);