[CSSRegions] Incorrect computed height for content with region-break-before
authormihnea@adobe.com <mihnea@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Nov 2012 11:39:12 +0000 (11:39 +0000)
committermihnea@adobe.com <mihnea@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Nov 2012 11:39:12 +0000 (11:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=101862

Reviewed by Julien Chaffraix.

Source/WebCore:

When processing the region breaks for auto-height regions, we skipped the case
when the region-break-before occurred in the first region in the chain which was
an auto-height region. Because of that, the region computed height was not 0
as it was supposed to be, but rather LayoutUnit::max() / 2.

Test: fast/regions/autoheight-breakbefore-wrongheight.html

* rendering/RenderFlowThread.cpp:
(WebCore::RenderFlowThread::addForcedRegionBreak):
Make sure we process also the case when the region-break occurs at offset 0 in the flow thread
and the first region in chain is an auto-height region.
* rendering/RenderRegion.cpp:
(WebCore::RenderRegion::updateLogicalHeight):
Add an assert to make sure that the computed height for auto-height regions is always less than LayoutUnit::max() / 2.

LayoutTests:

Add a test showing that when content having -webkit-region-break-before:always is flowed
into a region chain with the first region being an auto-height region, the auto-height region
has a computed height of 0 and the content is flowed into the second region in the chain.

* fast/regions/autoheight-breakbefore-wrongheight-expected.txt: Added.
* fast/regions/autoheight-breakbefore-wrongheight.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@134395 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/regions/autoheight-breakbefore-wrongheight-expected.txt [new file with mode: 0644]
LayoutTests/fast/regions/autoheight-breakbefore-wrongheight.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlowThread.cpp
Source/WebCore/rendering/RenderRegion.cpp

index e3b9831..d6b8b8a 100644 (file)
@@ -1,3 +1,17 @@
+2012-11-13  Mihnea Ovidenie  <mihnea@adobe.com>
+
+        [CSSRegions] Incorrect computed height for content with region-break-before
+        https://bugs.webkit.org/show_bug.cgi?id=101862
+
+        Reviewed by Julien Chaffraix.
+
+        Add a test showing that when content having -webkit-region-break-before:always is flowed
+        into a region chain with the first region being an auto-height region, the auto-height region
+        has a computed height of 0 and the content is flowed into the second region in the chain.
+
+        * fast/regions/autoheight-breakbefore-wrongheight-expected.txt: Added.
+        * fast/regions/autoheight-breakbefore-wrongheight.html: Added.
+
 2012-11-12  Kent Tamura  <tkent@chromium.org>
 
         Unable to set valid time value to input[type=time] with user interaction in some cases
diff --git a/LayoutTests/fast/regions/autoheight-breakbefore-wrongheight-expected.txt b/LayoutTests/fast/regions/autoheight-breakbefore-wrongheight-expected.txt
new file mode 100644 (file)
index 0000000..72f144b
--- /dev/null
@@ -0,0 +1,6 @@
+Test for [CSSRegions] Incorrect computed height for content with region-break-before.
+
+The flow has two regions, the first region has height auto. The region-break before the content should set the height for the auto height region to 0.
+
+PASS
+
diff --git a/LayoutTests/fast/regions/autoheight-breakbefore-wrongheight.html b/LayoutTests/fast/regions/autoheight-breakbefore-wrongheight.html
new file mode 100644 (file)
index 0000000..bd81588
--- /dev/null
@@ -0,0 +1,23 @@
+<!doctype html>
+<html>
+    <head>
+        <style>
+            .flowContentSize  { width: 50px; height: 50px; }
+            #article { -webkit-flow-into: flow; background-color: green; }
+
+            .region { -webkit-flow-from: flow; position: absolute; background-color: red; }
+            .regionSize { width: 50px; max-height: 50px; top: 200px; }
+            .regionSize2 { width: 50px; height: 50px; top: 200px; left: 100px; }
+
+            #article { -webkit-region-break-before: always; }
+        </style>
+    </head>
+    <script src="../../resources/check-layout.js"></script>
+    <body onload="checkLayout('#regionZeroHeight')">
+        <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=101862"> [CSSRegions] Incorrect computed height for content with region-break-before</a>.</p>
+        <p>The flow has two regions, the first region has height auto. The region-break before the content should set the height for the auto height region to 0.</p>
+        <div id="article" class="flowContentSize"></div>
+        <div id="regionZeroHeight" class="region regionSize" data-expected-height=0></div>
+        <div class="region regionSize2"></div>
+    </body>
+</html>
index b664703..02f9b79 100644 (file)
@@ -1,3 +1,25 @@
+2012-11-13  Mihnea Ovidenie  <mihnea@adobe.com>
+
+        [CSSRegions] Incorrect computed height for content with region-break-before
+        https://bugs.webkit.org/show_bug.cgi?id=101862
+
+        Reviewed by Julien Chaffraix.
+
+        When processing the region breaks for auto-height regions, we skipped the case
+        when the region-break-before occurred in the first region in the chain which was
+        an auto-height region. Because of that, the region computed height was not 0
+        as it was supposed to be, but rather LayoutUnit::max() / 2.
+
+        Test: fast/regions/autoheight-breakbefore-wrongheight.html
+
+        * rendering/RenderFlowThread.cpp:
+        (WebCore::RenderFlowThread::addForcedRegionBreak):
+        Make sure we process also the case when the region-break occurs at offset 0 in the flow thread
+        and the first region in chain is an auto-height region.
+        * rendering/RenderRegion.cpp:
+        (WebCore::RenderRegion::updateLogicalHeight):
+        Add an assert to make sure that the computed height for auto-height regions is always less than LayoutUnit::max() / 2.
+
 2012-11-13  Yury Semikhatsky  <yurys@chromium.org>
 
         Memory instrumentation: remove reportMemoryUsage method from ImageObserver
index ce182cd..13809b0 100644 (file)
@@ -856,7 +856,7 @@ bool RenderFlowThread::addForcedRegionBreak(LayoutUnit offsetBreakInFlowThread,
 
     RenderRegionList::iterator regionIter = m_regionList.find(region);
     ASSERT(regionIter != m_regionList.end());
-    for (; (regionIter != m_regionList.end()) && (currentRegionOffsetInFlowThread < offsetBreakInFlowThread); ++regionIter) {
+    for (; regionIter != m_regionList.end(); ++regionIter) {
         RenderRegion* region = *regionIter;
         if (region->needsOverrideLogicalContentHeightComputation()) {
             mapToUse.set(breakChild, region);
@@ -871,6 +871,12 @@ bool RenderFlowThread::addForcedRegionBreak(LayoutUnit offsetBreakInFlowThread,
             currentRegionOffsetInFlowThread += regionOverrideLogicalContentHeight;
         } else
             currentRegionOffsetInFlowThread += isHorizontalWritingMode() ? region->flowThreadPortionRect().height() : region->flowThreadPortionRect().width();
+
+        // If the current offset if greater than the break offset, bail out and skip the current region.
+        if (currentRegionOffsetInFlowThread >= offsetBreakInFlowThread) {
+            ++regionIter;
+            break;
+        }
     }
 
     // The remaining auto logical height regions in the chain that were unable to receive content
index a59cba3..753a53c 100644 (file)
@@ -631,6 +631,7 @@ void RenderRegion::updateLogicalHeight()
         return;
 
     LayoutUnit newLogicalHeight = overrideLogicalContentHeight() + borderAndPaddingLogicalHeight();
+    ASSERT(newLogicalHeight < LayoutUnit::max() / 2);
     if (newLogicalHeight > logicalHeight())
         setLogicalHeight(newLogicalHeight);
 }