Title: [165873] trunk
Revision
165873
Author
abu...@adobe.com
Date
2014-03-18 23:57:17 -0700 (Tue, 18 Mar 2014)

Log Message

[CSS Regions] Strange layout for content with region breaks
https://bugs.webkit.org/show_bug.cgi?id=121318

Reviewed by Mihnea Ovidenie.

Source/WebCore:

When break-inside: avoid is used on an element, the implementation tries to
force it inside a single fragmentation container even though the descendants
of the element specify forced breaks. This leads to unexpected results and
unstable layout.

The change extends the definition of unsplittable elements for flow threads
using the same conditions found inside RenderBlockFlow::adjustForUnsplittableChild.
This change forces elements that require break avoidance to have a single region
in the region range. As a result, forced breaks have no effect inside them.

It should be noted this may not the behavior defined by the CSS3 Fragmentation spec.
>From my understanding of the text, forced breaks override any avoidance specified
by ancestor elements. However, implementing this is a larger change that will require
a fundamental new approach in handling break avoidance. This patch just brings
consistency to the API.

Test: fast/regions/forced-break-inside-avoid-break.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::childBoxIsUnsplittableForFragmentation):
(WebCore::RenderBlock::computeRegionRangeForBoxChild):
(WebCore::RenderBlock::estimateRegionRangeForBoxChild):
* rendering/RenderBlock.h:
* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::adjustForUnsplittableChild): Move the avoidance conditions inside
a helper method RenderBlock::childBoxIsUnsplittableForFragmentation.

LayoutTests:

The test verifies that elements specifying break-inside: avoid can't
be splitted by forced breaks applied inside of them.

* fast/regions/forced-break-inside-avoid-break-expected.txt: Added.
* fast/regions/forced-break-inside-avoid-break.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (165872 => 165873)


--- trunk/LayoutTests/ChangeLog	2014-03-19 06:55:29 UTC (rev 165872)
+++ trunk/LayoutTests/ChangeLog	2014-03-19 06:57:17 UTC (rev 165873)
@@ -1,3 +1,16 @@
+2014-03-18  Andrei Bucur  <abu...@adobe.com>
+
+        [CSS Regions] Strange layout for content with region breaks
+        https://bugs.webkit.org/show_bug.cgi?id=121318
+
+        Reviewed by Mihnea Ovidenie.
+
+        The test verifies that elements specifying break-inside: avoid can't
+        be splitted by forced breaks applied inside of them.
+
+        * fast/regions/forced-break-inside-avoid-break-expected.txt: Added.
+        * fast/regions/forced-break-inside-avoid-break.html: Added.
+
 2014-03-18  Samuel White  <samuel_wh...@apple.com>
 
         AX: Not able to use arrow keys to read text with VoiceOver before selection is set someplace (anyplace).

Added: trunk/LayoutTests/fast/regions/forced-break-inside-avoid-break-expected.txt (0 => 165873)


--- trunk/LayoutTests/fast/regions/forced-break-inside-avoid-break-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/regions/forced-break-inside-avoid-break-expected.txt	2014-03-19 06:57:17 UTC (rev 165873)
@@ -0,0 +1,8 @@
+Test for [CSS Regions] Strange layout for content with region breaks.
+
+The green rectangle is flowed inside three regions with blue borders. The first region should contain the whole green rectangle. The second and the third regions should have a height of 0px.
+
+PASS
+PASS
+PASS
+

Added: trunk/LayoutTests/fast/regions/forced-break-inside-avoid-break.html (0 => 165873)


--- trunk/LayoutTests/fast/regions/forced-break-inside-avoid-break.html	                        (rev 0)
+++ trunk/LayoutTests/fast/regions/forced-break-inside-avoid-break.html	2014-03-19 06:57:17 UTC (rev 165873)
@@ -0,0 +1,82 @@
+<!DOCTYPE html>
+<html>
+    <head>
+    <style>
+        .region {
+            -webkit-flow-from: article;
+            border: 1px solid blue;
+            float: left;
+            height: auto;
+        }
+
+        #article {
+            -webkit-flow-into: article;
+            width: 100%;
+            margin: 0px;
+        }
+
+        #region1 {
+            width: 20%;
+        }
+
+        #region2 {
+            width: 20%;
+        }
+
+        #region3 {
+            width: 30%;
+        }
+
+        #container {
+            margin: auto;
+            margin-top: 50px;
+            width: 50%;
+            background-color: rgb(42, 137, 57);
+            -webkit-region-break-inside: avoid;
+        }
+
+        #firstChild {
+            height: 31px;
+            margin-left: auto;
+            margin-top: 25px;
+            width: 70%;
+            background-color: rgb(255, 157, 0);
+            margin-right: auto;
+        }
+
+        #secondChild {
+            height: 27px;
+            margin-left: auto;
+            margin-top: 34px;
+            width: 70%;
+            background-color: rgb(255, 0, 0);
+            margin-right: auto;
+            -webkit-region-break-before: always;
+            -webkit-region-break-after: always;
+        }
+
+        #thirdChild {
+            height: 31px;
+            margin: auto;
+            width: 70%;
+            background-color: rgb(216, 0, 255);
+        }
+
+    </style>
+    <script src=""
+    </head>
+    <body _onload_="checkLayout('.region')">
+        <p>Test for <a href="" Regions] Strange layout for content with region breaks</a>.</p>
+        <p>The green rectangle is flowed inside three regions with blue borders. The first region should contain the whole green rectangle. The second and the third regions should have a height of 0px.</p>
+        <div id='region1' class='region' data-expected-height='175px'></div>
+        <div id='region2' class='region' data-expected-height='2px'></div>
+        <div id='region3' class='region' data-expected-height='2px'></div>
+        <div id='article'>
+            <div id='container'>
+                <div id='firstChild'></div>
+                <div id='secondChild'></div>
+                <div id='thirdChild'></div>
+            </div>
+        </div>
+    </body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (165872 => 165873)


--- trunk/Source/WebCore/ChangeLog	2014-03-19 06:55:29 UTC (rev 165872)
+++ trunk/Source/WebCore/ChangeLog	2014-03-19 06:57:17 UTC (rev 165873)
@@ -1,3 +1,37 @@
+2014-03-18  Andrei Bucur  <abu...@adobe.com>
+
+        [CSS Regions] Strange layout for content with region breaks
+        https://bugs.webkit.org/show_bug.cgi?id=121318
+
+        Reviewed by Mihnea Ovidenie.
+
+        When break-inside: avoid is used on an element, the implementation tries to
+        force it inside a single fragmentation container even though the descendants
+        of the element specify forced breaks. This leads to unexpected results and
+        unstable layout.
+
+        The change extends the definition of unsplittable elements for flow threads
+        using the same conditions found inside RenderBlockFlow::adjustForUnsplittableChild.
+        This change forces elements that require break avoidance to have a single region
+        in the region range. As a result, forced breaks have no effect inside them.
+
+        It should be noted this may not the behavior defined by the CSS3 Fragmentation spec.
+        From my understanding of the text, forced breaks override any avoidance specified
+        by ancestor elements. However, implementing this is a larger change that will require
+        a fundamental new approach in handling break avoidance. This patch just brings
+        consistency to the API.
+
+        Test: fast/regions/forced-break-inside-avoid-break.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::childBoxIsUnsplittableForFragmentation):
+        (WebCore::RenderBlock::computeRegionRangeForBoxChild):
+        (WebCore::RenderBlock::estimateRegionRangeForBoxChild):
+        * rendering/RenderBlock.h:
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::adjustForUnsplittableChild): Move the avoidance conditions inside
+        a helper method RenderBlock::childBoxIsUnsplittableForFragmentation.
+
 2014-03-18  Samuel White  <samuel_wh...@apple.com>
 
         AX: Not able to use arrow keys to read text with VoiceOver before selection is set someplace (anyplace).

Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (165872 => 165873)


--- trunk/Source/WebCore/rendering/RenderBlock.cpp	2014-03-19 06:55:29 UTC (rev 165872)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp	2014-03-19 06:57:17 UTC (rev 165873)
@@ -5054,6 +5054,18 @@
     return flowThreadContainingBlock->hasRegionRangeForBox(parentBlock);
 }
 
+bool RenderBlock::childBoxIsUnsplittableForFragmentation(const RenderBox& child) const
+{
+    RenderFlowThread* flowThread = flowThreadContainingBlock();
+    bool isInsideMulticolFlowThread = flowThread && !flowThread->isRenderNamedFlowThread();
+    bool checkColumnBreaks = isInsideMulticolFlowThread || view().layoutState()->isPaginatingColumns();
+    bool checkPageBreaks = !checkColumnBreaks && view().layoutState()->m_pageLogicalHeight;
+    bool checkRegionBreaks = flowThread && flowThread->isRenderNamedFlowThread();
+    return child.isUnsplittableForPagination() || (checkColumnBreaks && child.style().columnBreakInside() == PBAVOID)
+        || (checkPageBreaks && child.style().pageBreakInside() == PBAVOID)
+        || (checkRegionBreaks && child.style().regionBreakInside() == PBAVOID);
+}
+
 void RenderBlock::computeRegionRangeForBoxChild(const RenderBox& box) const
 {
     RenderFlowThread* flowThread = flowThreadContainingBlock();
@@ -5062,7 +5074,7 @@
     RenderRegion* startRegion;
     RenderRegion* endRegion;
     LayoutUnit offsetFromLogicalTopOfFirstRegion = box.offsetFromLogicalTopOfFirstPage();
-    if (box.isUnsplittableForPagination())
+    if (childBoxIsUnsplittableForFragmentation(box))
         startRegion = endRegion = flowThread->regionAtBlockOffset(this, offsetFromLogicalTopOfFirstRegion, true);
     else {
         startRegion = flowThread->regionAtBlockOffset(this, offsetFromLogicalTopOfFirstRegion, true);
@@ -5078,7 +5090,7 @@
     if (!canComputeRegionRangeForBox(this, box, flowThread))
         return;
 
-    if (box.isUnsplittableForPagination()) {
+    if (childBoxIsUnsplittableForFragmentation(box)) {
         computeRegionRangeForBoxChild(box);
         return;
     }

Modified: trunk/Source/WebCore/rendering/RenderBlock.h (165872 => 165873)


--- trunk/Source/WebCore/rendering/RenderBlock.h	2014-03-19 06:55:29 UTC (rev 165872)
+++ trunk/Source/WebCore/rendering/RenderBlock.h	2014-03-19 06:57:17 UTC (rev 165873)
@@ -421,6 +421,8 @@
     // FIXME: Can de-virtualize this once old columns go away.
     virtual void setComputedColumnCountAndWidth(int, LayoutUnit);
 
+    bool childBoxIsUnsplittableForFragmentation(const RenderBox& child) const;
+
 public:
     virtual void computeOverflow(LayoutUnit oldClientAfterEdge, bool recomputeFloats = false);
     void clearLayoutOverflow();

Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.cpp (165872 => 165873)


--- trunk/Source/WebCore/rendering/RenderBlockFlow.cpp	2014-03-19 06:55:29 UTC (rev 165872)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.cpp	2014-03-19 06:57:17 UTC (rev 165873)
@@ -1689,16 +1689,10 @@
 
 LayoutUnit RenderBlockFlow::adjustForUnsplittableChild(RenderBox& child, LayoutUnit logicalOffset, bool includeMargins)
 {
-    RenderFlowThread* flowThread = flowThreadContainingBlock();
-    bool isInsideMulticolFlowThread = flowThread && !flowThread->isRenderNamedFlowThread();
-    bool checkColumnBreaks = isInsideMulticolFlowThread || view().layoutState()->isPaginatingColumns();
-    bool checkPageBreaks = !checkColumnBreaks && view().layoutState()->m_pageLogicalHeight;
-    bool checkRegionBreaks = flowThread && flowThread->isRenderNamedFlowThread();
-    bool isUnsplittable = child.isUnsplittableForPagination() || (checkColumnBreaks && child.style().columnBreakInside() == PBAVOID)
-        || (checkPageBreaks && child.style().pageBreakInside() == PBAVOID)
-        || (checkRegionBreaks && child.style().regionBreakInside() == PBAVOID);
-    if (!isUnsplittable)
+    if (!childBoxIsUnsplittableForFragmentation(child))
         return logicalOffset;
+
+    RenderFlowThread* flowThread = flowThreadContainingBlock();
     LayoutUnit childLogicalHeight = logicalHeightForChild(child) + (includeMargins ? marginBeforeForChild(child) + marginAfterForChild(child) : LayoutUnit());
     LayoutUnit pageLogicalHeight = pageLogicalHeightForOffset(logicalOffset);
     bool hasUniformPageLogicalHeight = !flowThread || flowThread->regionsHaveUniformLogicalHeight();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to