Diff
Modified: trunk/LayoutTests/ChangeLog (136106 => 136107)
--- trunk/LayoutTests/ChangeLog 2012-11-29 08:03:31 UTC (rev 136106)
+++ trunk/LayoutTests/ChangeLog 2012-11-29 08:35:50 UTC (rev 136107)
@@ -1,3 +1,20 @@
+2012-11-29 Andrei Bucur <[email protected]>
+
+ [CSS Regions] Fix content node renderers ordering inside the named flow thread
+ https://bugs.webkit.org/show_bug.cgi?id=103501
+
+ Reviewed by David Hyatt.
+
+ The first two ref tests cover the issue with incorrectly computing the insertion position for a content node renderer.
+ The third ref test covers the issue with nextRenderer and previousRenderer skipping valid nodes.
+
+ * fast/regions/named-flow-content-order-1-expected.html: Added.
+ * fast/regions/named-flow-content-order-1.html: Added.
+ * fast/regions/named-flow-content-order-2-expected.html: Added.
+ * fast/regions/named-flow-content-order-2.html: Added.
+ * fast/regions/named-flow-content-order-3-expected.html: Added.
+ * fast/regions/named-flow-content-order-3.html: Added.
+
2012-11-28 Mike West <[email protected]>
Add a test to ensure that 'seamless' iframes do not inherit contenteditable.
Added: trunk/LayoutTests/fast/regions/named-flow-content-order-1-expected.html (0 => 136107)
--- trunk/LayoutTests/fast/regions/named-flow-content-order-1-expected.html (rev 0)
+++ trunk/LayoutTests/fast/regions/named-flow-content-order-1-expected.html 2012-11-29 08:35:50 UTC (rev 136107)
@@ -0,0 +1,44 @@
+<!doctype html>
+<html>
+ <head>
+ <style>
+ .region {
+ height: 60px;
+ width: 120px;
+ border: 2px solid black;
+ }
+
+ .float {
+ float: left;
+ height: 60px;
+ width: 40px;
+ }
+
+ #red {
+ background-color: red;
+ }
+
+ #green {
+ background-color: green;
+ }
+
+ #blue {
+ background-color: blue;
+ }
+
+ div.content:hover p:first-letter {
+ font-size: 30px;
+ }
+ </style>
+ </head>
+ <body>
+ <p>Test for <a href="" content node renderers ordering inside the named flow thread</a>.</p>
+ <p>The test adds three colored content nodes (blocks) to a flow and then modifies the flow-into properties to force renderers to shuffle.</p>
+ <p>On success, you should see the blocks ordered: red, green, blue.</p>
+ <div class="region">
+ <div id="red" class="float">1</div>
+ <div id="green"class="float">2</div>
+ <div id="blue" class="float">3</div>
+ </div>
+ </body>
+</html>
Added: trunk/LayoutTests/fast/regions/named-flow-content-order-1.html (0 => 136107)
--- trunk/LayoutTests/fast/regions/named-flow-content-order-1.html (rev 0)
+++ trunk/LayoutTests/fast/regions/named-flow-content-order-1.html 2012-11-29 08:35:50 UTC (rev 136107)
@@ -0,0 +1,57 @@
+<!doctype html>
+<html>
+ <head>
+ <style>
+ .region {
+ -webkit-flow-from: content-flow;
+ height: 60px;
+ width: 120px;
+ border: 2px solid black;
+ }
+
+ .float {
+ float: left;
+ -webkit-flow-into: content-flow;
+ height: 60px;
+ width: 40px;
+ }
+
+ #red {
+ background-color: red;
+ }
+
+ #green {
+ background-color: green;
+ }
+
+ #blue {
+ background-color: blue;
+ }
+ </style>
+ </head>
+ <body _onload_="shuffle()">
+ <p>Test for <a href="" content node renderers ordering inside the named flow thread</a>.</p>
+ <p>The test adds three colored content nodes (blocks) to a flow and then modifies the flow-into properties to force renderers to shuffle.</p>
+ <p>On success, you should see the blocks ordered: red, green, blue.</p>
+
+ <div id="red" class="float">1</div>
+ <div id="green"class="float">2</div>
+ <div id="blue" class="float">3</div>
+ <div class="region"></div>
+
+ <script>
+ function readd(content) {
+ content.style.webkitFlowInto = "none";
+ document.body.offsetTop;
+ content.style.webkitFlowInto = "content-flow";
+ document.body.offsetTop;
+ }
+ function shuffle() {
+ var greenElement = document.getElementById("green");
+ readd(greenElement);
+ var redElement = document.getElementById("red");
+ readd(redElement);
+ }
+ </script>
+ </body>
+</html>
Added: trunk/LayoutTests/fast/regions/named-flow-content-order-2-expected.html (0 => 136107)
--- trunk/LayoutTests/fast/regions/named-flow-content-order-2-expected.html (rev 0)
+++ trunk/LayoutTests/fast/regions/named-flow-content-order-2-expected.html 2012-11-29 08:35:50 UTC (rev 136107)
@@ -0,0 +1,46 @@
+<!doctype html>
+<html>
+ <head>
+ <style>
+ .region {
+ height: 180px;
+ width: 80px;
+ border: 2px solid black;
+ }
+
+ .content {
+ height: 60px;
+ width: 40px;
+ }
+
+ #red {
+ background-color: red;
+ }
+
+ #green {
+ background-color: green;
+ display: inline-block;
+ }
+
+ #yellow {
+ background-color: yellow;
+ display: inline-block;
+ }
+
+ #blue {
+ background-color: blue;
+ }
+ </style>
+ </head>
+ <body _onload_="shuffle()">
+ <p>Test for <a href="" content node renderers ordering inside the named flow thread</a>.</p>
+ <p>The test adds four colored content nodes (mix of inline and block) to a flow and then modifies the flow-into properties to force renderers to shuffle.</p>
+ <p>On success, you should see the blocks ordered: red (top), green, yellow (middle), blue (bottom).</p>
+
+ <div class="region">
+ <div id="red" class="content">1</div>
+ <div id="green"class="content">2</div><div id="yellow"class="content">3</div>
+ <div id="blue" class="content">4</div>
+ </div>
+ </body>
+</html>
Added: trunk/LayoutTests/fast/regions/named-flow-content-order-2.html (0 => 136107)
--- trunk/LayoutTests/fast/regions/named-flow-content-order-2.html (rev 0)
+++ trunk/LayoutTests/fast/regions/named-flow-content-order-2.html 2012-11-29 08:35:50 UTC (rev 136107)
@@ -0,0 +1,66 @@
+<!doctype html>
+<html>
+ <head>
+ <style>
+ .region {
+ -webkit-flow-from: content-flow;
+ height: 180px;
+ width: 80px;
+ border: 2px solid black;
+ }
+
+ .content {
+ -webkit-flow-into: content-flow;
+ height: 60px;
+ width: 40px;
+ }
+
+ #red {
+ background-color: red;
+ }
+
+ #green {
+ background-color: green;
+ display: inline-block;
+ }
+
+ #yellow {
+ background-color: yellow;
+ display: inline-block;
+ }
+
+ #blue {
+ background-color: blue;
+ }
+ </style>
+ </head>
+ <body _onload_="shuffle()">
+ <p>Test for <a href="" content node renderers ordering inside the named flow thread</a>.</p>
+ <p>The test adds four colored content nodes (mix of inline and block) to a flow and then modifies the flow-into properties to force renderers to shuffle.</p>
+ <p>On success, you should see the blocks ordered: red (top), green, yellow (middle), blue (bottom).</p>
+
+ <div id="red" class="content">1</div>
+ <div id="green"class="content">2</div>
+ <div id="yellow"class="content">3</div>
+ <div id="blue" class="content">4</div>
+ <div class="region"></div>
+ <script>
+ function readd(content) {
+ content.style.webkitFlowInto = "none";
+ document.body.offsetTop;
+ content.style.webkitFlowInto = "content-flow";
+ document.body.offsetTop;
+ }
+ function shuffle() {
+ var greenElement = document.getElementById("green");
+ var redElement = document.getElementById("red");
+ var yellowElement = document.getElementById("yellow");
+ var blueElement = document.getElementById("blue");
+ readd(greenElement);
+ readd(redElement);
+ readd(blueElement);
+ readd(yellowElement);
+ }
+ </script>
+ </body>
+</html>
Added: trunk/LayoutTests/fast/regions/named-flow-content-order-3-expected.html (0 => 136107)
--- trunk/LayoutTests/fast/regions/named-flow-content-order-3-expected.html (rev 0)
+++ trunk/LayoutTests/fast/regions/named-flow-content-order-3-expected.html 2012-11-29 08:35:50 UTC (rev 136107)
@@ -0,0 +1,33 @@
+<!doctype html>
+<html>
+ <head>
+ <style>
+ .region {
+ height: 300px;
+ width: 500px;
+ border: 1px solid black;
+ }
+
+ p, div, body {
+ margin: 0px;
+ font-size: 20px;
+ }
+
+ p {
+ display: inline;
+ }
+ </style>
+ </head>
+ <body>
+ <p>Test for <a href="" content node renderers ordering inside the named flow thread</a>.</p>
+ <p>The test verifies the order of elements preserves when the text nodes have flow-into set because of style cloning.</p>
+ <p>On success, you should see the first inline, the paragraph and the second inline.</p>
+ <div class="region">
+ <div class="content">
+ First inline.
+ <p>A paragraph that can be block or inline.</p>
+ Second inline.
+ </div>
+ </div>
+ </body>
+</html>
Added: trunk/LayoutTests/fast/regions/named-flow-content-order-3.html (0 => 136107)
--- trunk/LayoutTests/fast/regions/named-flow-content-order-3.html (rev 0)
+++ trunk/LayoutTests/fast/regions/named-flow-content-order-3.html 2012-11-29 08:35:50 UTC (rev 136107)
@@ -0,0 +1,43 @@
+<!doctype html>
+<html>
+ <head>
+ <style>
+ .content { -webkit-flow-into: content-flow; }
+
+ .region {
+ -webkit-flow-from: content-flow;
+ height: 300px;
+ width: 500px;
+ border: 1px solid black;
+ }
+
+ p, div, body {
+ margin: 0px;
+ font-size: 20px;
+ }
+
+ p {
+ display: inline;
+ }
+ </style>
+ </head>
+ <body>
+ <p>Test for <a href="" content node renderers ordering inside the named flow thread</a>.</p>
+ <p>The test verifies the order of elements preserves when the text nodes have flow-into set because of style cloning.</p>
+ <p>On success, you should see the first inline, the paragraph and the second inline.</p>
+ <div class="content">
+ First inline.
+ <p id="para">A paragraph that can be block or inline.</p>
+ Second inline.
+ </div>
+ <div class="region"></div>
+
+ <script>
+ document.body.offsetTop;
+ var para = document.getElementById("para");
+ para.style.display = "block";
+ document.body.offsetTop;
+ para.style.display = "inline";
+ </script>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (136106 => 136107)
--- trunk/Source/WebCore/ChangeLog 2012-11-29 08:03:31 UTC (rev 136106)
+++ trunk/Source/WebCore/ChangeLog 2012-11-29 08:35:50 UTC (rev 136107)
@@ -1,3 +1,35 @@
+2012-11-29 Andrei Bucur <[email protected]>
+
+ [CSS Regions] Fix content node renderers ordering inside the named flow thread
+ https://bugs.webkit.org/show_bug.cgi?id=103501
+
+ Reviewed by David Hyatt.
+
+ This patch fixes two issues with how content nodes renderers are added to a named flow thread.
+ The first issue was about determining the insertion position of a renderer inside the children list of a named flow thread. Before this patch, the
+ insertion point was based on both the DOM ordering of the elements and insertion order of previous renderers.
+ The patch fixes this and makes the renderer position just a function of the DOM ordering of elements.
+ The second issue appeared when next/previousRenderer methods were skipping nodes because they had the flow-into property as a side effect
+ of copying the style of the parent element (e.g. Text nodes). The patch ensures the skipped nodes are also elements.
+
+ Tests: fast/regions/named-flow-content-order-1.html
+ fast/regions/named-flow-content-order-2.html
+ fast/regions/named-flow-content-order-3.html
+
+ * dom/NodeRenderingContext.cpp:
+ (WebCore::NodeRenderingContext::nextRenderer): Skip only elements.
+ (WebCore::NodeRenderingContext::previousRenderer): Skip only elements.
+ * rendering/RenderNamedFlowThread.cpp:
+ (WebCore::RenderNamedFlowThread::addFlowChild): Insert the renderer in the list based on the DOM position of the owner element.
+ * rendering/RenderNamedFlowThread.h:
+ (RenderNamedFlowThread):
+ * rendering/RenderObject.cpp:
+ (WebCore::RenderObject::renderNamedFlowThreadWrapper): Rename to eliminate the confusion with enclosingRenderFlowThread.
+ (WebCore::RenderObject::insertedIntoTree):
+ (WebCore::RenderObject::willBeRemovedFromTree):
+ * rendering/RenderObject.h:
+ (RenderObject):
+
2012-11-28 Mike West <[email protected]>
Add a test to ensure that 'seamless' iframes do not inherit contenteditable.
Modified: trunk/Source/WebCore/dom/NodeRenderingContext.cpp (136106 => 136107)
--- trunk/Source/WebCore/dom/NodeRenderingContext.cpp 2012-11-29 08:03:31 UTC (rev 136106)
+++ trunk/Source/WebCore/dom/NodeRenderingContext.cpp 2012-11-29 08:35:50 UTC (rev 136107)
@@ -86,8 +86,8 @@
ComposedShadowTreeWalker walker(m_node);
for (walker.nextSibling(); walker.get(); walker.nextSibling()) {
if (RenderObject* renderer = walker.get()->renderer()) {
- // Do not return elements that are attached to a different flow-thread.
- if (renderer->style() && !renderer->style()->flowThread().isEmpty())
+ // Renderers for elements attached to a flow thread should be skipped because they are parented differently.
+ if (renderer->node()->isElementNode() && renderer->style() && !renderer->style()->flowThread().isEmpty())
continue;
return renderer;
}
@@ -110,8 +110,8 @@
ComposedShadowTreeWalker walker(m_node);
for (walker.previousSibling(); walker.get(); walker.previousSibling()) {
if (RenderObject* renderer = walker.get()->renderer()) {
- // Do not return elements that are attached to a different flow-thread.
- if (renderer->style() && !renderer->style()->flowThread().isEmpty())
+ // Renderers for elements attached to a flow thread should be skipped because they are parented differently.
+ if (renderer->node()->isElementNode() && renderer->style() && !renderer->style()->flowThread().isEmpty())
continue;
return renderer;
}
Modified: trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp (136106 => 136107)
--- trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp 2012-11-29 08:03:31 UTC (rev 136106)
+++ trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp 2012-11-29 08:35:50 UTC (rev 136107)
@@ -113,15 +113,20 @@
return 0;
}
-void RenderNamedFlowThread::addFlowChild(RenderObject* newChild, RenderObject* beforeChild)
+void RenderNamedFlowThread::addFlowChild(RenderObject* newChild)
{
// The child list is used to sort the flow thread's children render objects
// based on their corresponding nodes DOM order. The list is needed to avoid searching the whole DOM.
+ Node* childNode = newChild->node();
+
// Do not add anonymous objects.
- if (!newChild->node())
+ if (!childNode)
return;
+ ASSERT(childNode->isElementNode());
+
+ RenderObject* beforeChild = nextRendererForNode(childNode);
if (beforeChild)
m_flowThreadChildList.insertBefore(beforeChild, newChild);
else
Modified: trunk/Source/WebCore/rendering/RenderNamedFlowThread.h (136106 => 136107)
--- trunk/Source/WebCore/rendering/RenderNamedFlowThread.h 2012-11-29 08:03:31 UTC (rev 136106)
+++ trunk/Source/WebCore/rendering/RenderNamedFlowThread.h 2012-11-29 08:35:50 UTC (rev 136107)
@@ -55,7 +55,7 @@
RenderObject* nextRendererForNode(Node*) const;
RenderObject* previousRendererForNode(Node*) const;
- void addFlowChild(RenderObject* newChild, RenderObject* beforeChild = 0);
+ void addFlowChild(RenderObject* newChild);
void removeFlowChild(RenderObject*);
bool hasChildren() const { return !m_flowThreadChildList.isEmpty(); }
#ifndef NDEBUG
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (136106 => 136107)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2012-11-29 08:03:31 UTC (rev 136106)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2012-11-29 08:35:50 UTC (rev 136107)
@@ -604,7 +604,7 @@
return 0;
}
-RenderNamedFlowThread* RenderObject::enclosingRenderNamedFlowThread() const
+RenderNamedFlowThread* RenderObject::renderNamedFlowThreadWrapper() const
{
RenderObject* object = const_cast<RenderObject*>(this);
while (object && object->isAnonymousBlock() && !object->isRenderNamedFlowThread())
@@ -2422,7 +2422,7 @@
if (!isFloating() && parent()->childrenInline())
parent()->dirtyLinesFromChangedChild(this);
- if (RenderNamedFlowThread* containerFlowThread = parent()->enclosingRenderNamedFlowThread())
+ if (RenderNamedFlowThread* containerFlowThread = parent()->renderNamedFlowThreadWrapper())
containerFlowThread->addFlowChild(this);
}
@@ -2450,7 +2450,7 @@
if (inRenderFlowThread())
removeFromRenderFlowThread();
- if (RenderNamedFlowThread* containerFlowThread = parent()->enclosingRenderNamedFlowThread())
+ if (RenderNamedFlowThread* containerFlowThread = parent()->renderNamedFlowThreadWrapper())
containerFlowThread->removeFlowChild(this);
#if ENABLE(SVG)
Modified: trunk/Source/WebCore/rendering/RenderObject.h (136106 => 136107)
--- trunk/Source/WebCore/rendering/RenderObject.h 2012-11-29 08:03:31 UTC (rev 136106)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2012-11-29 08:35:50 UTC (rev 136107)
@@ -241,7 +241,7 @@
// Function to return our enclosing flow thread if we are contained inside one.
RenderFlowThread* enclosingRenderFlowThread() const;
- RenderNamedFlowThread* enclosingRenderNamedFlowThread() const;
+ RenderNamedFlowThread* renderNamedFlowThreadWrapper() const;
virtual bool isEmpty() const { return firstChild() == 0; }
