In vertical writing modes, pagination may split a line after a block shifts
authormitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 May 2012 20:01:50 +0000 (20:01 +0000)
committermitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 May 2012 20:01:50 +0000 (20:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=86763

Reviewed by Sam Weinig.

Source/WebCore:

Test: fast/multicol/pageLogicalOffset-vertical.html

LayoutState::pageLogicalOffset() was returning bogus results in vertical writing modes,
because it was always using physical heights. Changed it to take a RenderBox and use its
writing mode to choose between heights and widths.

* rendering/LayoutState.cpp:
(WebCore::LayoutState::pageLogicalOffset):
(WebCore::LayoutState::addForcedColumnBreak):
* rendering/LayoutState.h:
(LayoutState):
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::layoutBlock):
(WebCore::RenderBlock::markForPaginationRelayoutIfNeeded):
(WebCore::RenderBlock::layoutColumns):
(WebCore::RenderBlock::applyBeforeBreak):
(WebCore::RenderBlock::applyAfterBreak):
* rendering/RenderDeprecatedFlexibleBox.cpp:
(WebCore::RenderDeprecatedFlexibleBox::layoutBlock):
* rendering/RenderTable.cpp:
(WebCore::RenderTable::layout):
* rendering/RenderTableRow.cpp:
(WebCore::RenderTableRow::layout):
* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::layoutRows):

LayoutTests:

* fast/multicol/pageLogicalOffset-vertical-expected.html: Added.
* fast/multicol/pageLogicalOffset-vertical.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/multicol/pageLogicalOffset-vertical-expected.html [new file with mode: 0644]
LayoutTests/fast/multicol/pageLogicalOffset-vertical.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/LayoutState.cpp
Source/WebCore/rendering/LayoutState.h
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp
Source/WebCore/rendering/RenderTable.cpp
Source/WebCore/rendering/RenderTableRow.cpp
Source/WebCore/rendering/RenderTableSection.cpp

index 4f9be3a..ddf47b7 100644 (file)
@@ -1,3 +1,13 @@
+2012-05-17  Dan Bernstein  <mitz@apple.com>
+
+        In vertical writing modes, pagination may split a line after a block shifts
+        https://bugs.webkit.org/show_bug.cgi?id=86763
+
+        Reviewed by Sam Weinig.
+
+        * fast/multicol/pageLogicalOffset-vertical-expected.html: Added.
+        * fast/multicol/pageLogicalOffset-vertical.html: Added.
+
 2012-05-17  Abhishek Arya  <inferno@chromium.org>
 
         Move run-in handling to addChild, instead of in layout.
diff --git a/LayoutTests/fast/multicol/pageLogicalOffset-vertical-expected.html b/LayoutTests/fast/multicol/pageLogicalOffset-vertical-expected.html
new file mode 100644 (file)
index 0000000..c5d4eef
--- /dev/null
@@ -0,0 +1,32 @@
+<style>\r
+    .container {\r
+        background-color: lightblue;\r
+        height: 200px;\r
+        width: 100px;\r
+        -webkit-columns: 2;\r
+        -webkit-column-gap: 0;\r
+        margin: 8px;\r
+    }\r
+\r
+    .margin-before {\r
+        width: 90px;\r
+    }\r
+\r
+    .test {\r
+        font: 20px ahem;\r
+        background-color: yellow;\r
+    }\r
+</style>\r
+<div class="container" style="-webkit-writing-mode: vertical-rl;">\r
+    <div>\r
+        <div class="margin-before"></div>\r
+        <div class="test">Test</div>\r
+    </div>\r
+</div>\r
+\r
+<div class="container" style="-webkit-writing-mode: vertical-lr;">\r
+    <div>\r
+        <div class="margin-before"></div>\r
+        <div class="test">Test</div>\r
+    </div>\r
+</div>\r
diff --git a/LayoutTests/fast/multicol/pageLogicalOffset-vertical.html b/LayoutTests/fast/multicol/pageLogicalOffset-vertical.html
new file mode 100644 (file)
index 0000000..e4bf9e9
--- /dev/null
@@ -0,0 +1,33 @@
+<style>\r
+    .container {\r
+        background-color: lightblue;\r
+        height: 200px;\r
+        width: 100px;\r
+        -webkit-columns: 2;\r
+        -webkit-column-gap: 0;\r
+        margin: 8px;\r
+    }\r
+\r
+    .margin-before {\r
+        -webkit-margin-before: 30px;\r
+        width: 60px;\r
+    }\r
+\r
+    .test {\r
+        font: 20px ahem;\r
+        background-color: yellow;\r
+    }\r
+</style>\r
+<div class="container" style="-webkit-writing-mode: vertical-rl;">\r
+    <div>\r
+        <div class="margin-before"></div>\r
+        <div class="test">Test</div>\r
+    </div>\r
+</div>\r
+\r
+<div class="container" style="-webkit-writing-mode: vertical-lr;">\r
+    <div>\r
+        <div class="margin-before"></div>\r
+        <div class="test">Test</div>\r
+    </div>\r
+</div>\r
index 60d3cb6..7600013 100644 (file)
@@ -1,3 +1,36 @@
+2012-05-17  Dan Bernstein  <mitz@apple.com>
+
+        In vertical writing modes, pagination may split a line after a block shifts
+        https://bugs.webkit.org/show_bug.cgi?id=86763
+
+        Reviewed by Sam Weinig.
+
+        Test: fast/multicol/pageLogicalOffset-vertical.html
+
+        LayoutState::pageLogicalOffset() was returning bogus results in vertical writing modes,
+        because it was always using physical heights. Changed it to take a RenderBox and use its
+        writing mode to choose between heights and widths.
+
+        * rendering/LayoutState.cpp:
+        (WebCore::LayoutState::pageLogicalOffset):
+        (WebCore::LayoutState::addForcedColumnBreak):
+        * rendering/LayoutState.h:
+        (LayoutState):
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::layoutBlock):
+        (WebCore::RenderBlock::markForPaginationRelayoutIfNeeded):
+        (WebCore::RenderBlock::layoutColumns):
+        (WebCore::RenderBlock::applyBeforeBreak):
+        (WebCore::RenderBlock::applyAfterBreak):
+        * rendering/RenderDeprecatedFlexibleBox.cpp:
+        (WebCore::RenderDeprecatedFlexibleBox::layoutBlock):
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::layout):
+        * rendering/RenderTableRow.cpp:
+        (WebCore::RenderTableRow::layout):
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::layoutRows):
+
 2012-05-17  Abhishek Arya  <inferno@chromium.org>
 
         Move run-in handling to addChild, instead of in layout.
index 1fbfc0c..5f780dc 100644 (file)
@@ -196,16 +196,18 @@ void LayoutState::clearPaginationInformation()
     m_columnInfo = m_next->m_columnInfo;
 }
 
-LayoutUnit LayoutState::pageLogicalOffset(LayoutUnit childLogicalOffset) const
+LayoutUnit LayoutState::pageLogicalOffset(RenderBox* child, LayoutUnit childLogicalOffset) const
 {
-    return m_layoutOffset.height() + childLogicalOffset - m_pageOffset.height();
+    if (child->isHorizontalWritingMode())
+        return m_layoutOffset.height() + childLogicalOffset - m_pageOffset.height();
+    return m_layoutOffset.width() + childLogicalOffset - m_pageOffset.width();
 }
 
-void LayoutState::addForcedColumnBreak(LayoutUnit childLogicalOffset)
+void LayoutState::addForcedColumnBreak(RenderBox* child, LayoutUnit childLogicalOffset)
 {
     if (!m_columnInfo || m_columnInfo->columnHeight())
         return;
-    m_columnInfo->addForcedBreak(pageLogicalOffset(childLogicalOffset));
+    m_columnInfo->addForcedBreak(pageLogicalOffset(child, childLogicalOffset));
 }
 
 void LayoutState::propagateLineGridInfo(RenderBox* renderer)
index e1df24b..81d61e9 100644 (file)
@@ -74,9 +74,9 @@ public:
     
     // The page logical offset is the object's offset from the top of the page in the page progression
     // direction (so an x-offset in vertical text and a y-offset for horizontal text).
-    LayoutUnit pageLogicalOffset(LayoutUnit childLogicalOffset) const;
+    LayoutUnit pageLogicalOffset(RenderBox*, LayoutUnit childLogicalOffset) const;
 
-    void addForcedColumnBreak(LayoutUnit childLogicalOffset);
+    void addForcedColumnBreak(RenderBox*, LayoutUnit childLogicalOffset);
     
     LayoutUnit pageLogicalHeight() const { return m_pageLogicalHeight; }
     bool pageLogicalHeightChanged() const { return m_pageLogicalHeightChanged; }
index 307e0a5..9f37414 100755 (executable)
@@ -1518,7 +1518,7 @@ void RenderBlock::layoutBlock(bool relayoutChildren, LayoutUnit pageLogicalHeigh
     statePusher.pop();
 
     if (renderView->layoutState()->m_pageLogicalHeight)
-        setPageLogicalOffset(renderView->layoutState()->pageLogicalOffset(logicalTop()));
+        setPageLogicalOffset(renderView->layoutState()->pageLogicalOffset(this, logicalTop()));
 
     updateLayerTransform();
 
@@ -2530,7 +2530,7 @@ void RenderBlock::markForPaginationRelayoutIfNeeded()
     if (needsLayout())
         return;
 
-    if (view()->layoutState()->pageLogicalHeightChanged() || (view()->layoutState()->pageLogicalHeight() && view()->layoutState()->pageLogicalOffset(logicalTop()) != pageLogicalOffset()))
+    if (view()->layoutState()->pageLogicalHeightChanged() || (view()->layoutState()->pageLogicalHeight() && view()->layoutState()->pageLogicalOffset(this, logicalTop()) != pageLogicalOffset()))
         setChildNeedsLayout(true, MarkOnlyThis);
 }
 
@@ -4990,7 +4990,7 @@ bool RenderBlock::layoutColumns(bool hasSpecifiedPageLogicalHeight, LayoutUnit p
             // maximum page break distance.
             if (!pageLogicalHeight) {
                 LayoutUnit distanceBetweenBreaks = max<LayoutUnit>(colInfo->maximumDistanceBetweenForcedBreaks(),
-                                                                   view()->layoutState()->pageLogicalOffset(borderBefore() + paddingBefore() + contentLogicalHeight()) - colInfo->forcedBreakOffset());
+                                                                   view()->layoutState()->pageLogicalOffset(this, borderBefore() + paddingBefore() + contentLogicalHeight()) - colInfo->forcedBreakOffset());
                 columnHeight = max(colInfo->minimumColumnHeight(), distanceBetweenBreaks);
             }
         } else if (contentLogicalHeight() > boundedMultiply(pageLogicalHeight, desiredColumnCount)) {
@@ -6654,7 +6654,7 @@ LayoutUnit RenderBlock::applyBeforeBreak(RenderBox* child, LayoutUnit logicalOff
                              || (checkRegionBreaks && child->style()->regionBreakBefore() == PBALWAYS);
     if (checkBeforeAlways && inNormalFlow(child) && hasNextPage(logicalOffset, IncludePageBoundary)) {
         if (checkColumnBreaks)
-            view()->layoutState()->addForcedColumnBreak(logicalOffset);
+            view()->layoutState()->addForcedColumnBreak(child, logicalOffset);
         return nextPageLogicalTop(logicalOffset, IncludePageBoundary);
     }
     return logicalOffset;
@@ -6671,7 +6671,7 @@ LayoutUnit RenderBlock::applyAfterBreak(RenderBox* child, LayoutUnit logicalOffs
     if (checkAfterAlways && inNormalFlow(child) && hasNextPage(logicalOffset, IncludePageBoundary)) {
         marginInfo.setMarginAfterQuirk(true); // Cause margins to be discarded for any following content.
         if (checkColumnBreaks)
-            view()->layoutState()->addForcedColumnBreak(logicalOffset);
+            view()->layoutState()->addForcedColumnBreak(child, logicalOffset);
         return nextPageLogicalTop(logicalOffset, IncludePageBoundary);
     }
     return logicalOffset;
index b956d39..7fae3c4 100644 (file)
@@ -301,7 +301,7 @@ void RenderDeprecatedFlexibleBox::layoutBlock(bool relayoutChildren, LayoutUnit)
     updateLayerTransform();
 
     if (view()->layoutState()->pageLogicalHeight())
-        setPageLogicalOffset(view()->layoutState()->pageLogicalOffset(logicalTop()));
+        setPageLogicalOffset(view()->layoutState()->pageLogicalOffset(this, logicalTop()));
 
     // Update our scrollbars if we're overflow:auto/scroll/hidden now that we know if
     // we overflow or not.
index 5e62ecc..28172ef 100644 (file)
@@ -456,7 +456,7 @@ void RenderTable::layout()
     statePusher.pop();
 
     if (view()->layoutState()->pageLogicalHeight())
-        setPageLogicalOffset(view()->layoutState()->pageLogicalOffset(logicalTop()));
+        setPageLogicalOffset(view()->layoutState()->pageLogicalOffset(this, logicalTop()));
 
     bool didFullRepaint = repainter.repaintAfterLayout();
     // Repaint with our new bounds if they are different from our old bounds.
index d988900..3caa7b4 100644 (file)
@@ -148,7 +148,7 @@ void RenderTableRow::layout()
     for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
         if (child->isTableCell()) {
             RenderTableCell* cell = toRenderTableCell(child);
-            if (!cell->needsLayout() && paginated && view()->layoutState()->pageLogicalHeight() && view()->layoutState()->pageLogicalOffset(cell->logicalTop()) != cell->pageLogicalOffset())
+            if (!cell->needsLayout() && paginated && view()->layoutState()->pageLogicalHeight() && view()->layoutState()->pageLogicalOffset(cell, cell->logicalTop()) != cell->pageLogicalOffset())
                 cell->setChildNeedsLayout(true, MarkOnlyThis);
 
             if (child->needsLayout()) {
index 8d8b355..7f44a75 100644 (file)
@@ -662,7 +662,7 @@ void RenderTableSection::layoutRows()
             if (intrinsicPaddingBefore != oldIntrinsicPaddingBefore || intrinsicPaddingAfter != oldIntrinsicPaddingAfter)
                 cell->setNeedsLayout(true, MarkOnlyThis);
 
-            if (!cell->needsLayout() && view()->layoutState()->pageLogicalHeight() && view()->layoutState()->pageLogicalOffset(cell->logicalTop()) != cell->pageLogicalOffset())
+            if (!cell->needsLayout() && view()->layoutState()->pageLogicalHeight() && view()->layoutState()->pageLogicalOffset(cell, cell->logicalTop()) != cell->pageLogicalOffset())
                 cell->setChildNeedsLayout(true, MarkOnlyThis);
 
             cell->layoutIfNeeded();