Attachment 185569 Details for Bug 108382 – Patch

[patch] Patch

bug-108382-20130130141449.patch (text/plain), 12.21 KB, created by Ojan Vafai on 2013-01-30 14:18:04 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ojan Vafai

Created: 2013-01-30 14:18:04 PST

Size: 12.21 KB

patch

obsolete

>Subversion Revision: 141137
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 34687aa21f729e877af0d17830d85f94169a4c3a..26af917feb68b3474b60f266f0a18ade45678f5a 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2013-01-30  Ojan Vafai  <ojan@chromium.org>
>+
>+        [REGRESSION: r128517] Percentage heights in quirks mode collapse when printing
>+        https://bugs.webkit.org/show_bug.cgi?id=108382
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        r128517 clean up our containing block finding logic, but broke percentage
>+        heights in quirks mode during printing since the RenderView would have 0 height.
>+        Turns out we already had a long-standing bug where we'd incorrectly
>+        treat collapse percentage heights on the body when printing as well.
>+
>+        Fix both bugs by changing the way we grab the logical height on the RenderView.
>+        RenderView::computeLogicalHeight returns 0 when printing. For the purposes of
>+        stretching and percentage heights, we instead need to return the pageLogicalHeight.
>+
>+        Tests: printing/quirks-percentage-height-body.html
>+               printing/quirks-percentage-height.html
>+               printing/standards-percentage-heights.html
>+
>+        * rendering/RenderBox.cpp:
>+        (WebCore::RenderBox::computeLogicalHeight):
>+        This FIXME is outdated and already fixed. Also, call viewLogicalHeightForPercentages
>+        which does the same logic except also correctly handles column RenderViews.
>+        (WebCore::RenderBox::viewLogicalHeightForPercentages):
>+        (WebCore::RenderBox::computePercentageLogicalHeight):
>+        * rendering/RenderBox.h:
>+
> 2013-01-29  Nate Chapin  <japhet@chromium.org>
> 
>         Enable reuse of cached main resources
>diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
>index d21e408f0fb49d32fadca4075fef52db74a00ea1..1fd1ad93a998926c453c6397e3dc080a1f778f46 100644
>--- a/Source/WebCore/rendering/RenderBox.cpp
>+++ b/Source/WebCore/rendering/RenderBox.cpp
>@@ -2416,27 +2416,24 @@ void RenderBox::computeLogicalHeight(LayoutUnit logicalHeight, LayoutUnit logica
>     bool paginatedContentNeedsBaseHeight = document()->printing() && h.isPercent()
>         && (isRoot() || (isBody() && document()->documentElement()->renderer()->style()->logicalHeight().isPercent()));
>     if (stretchesToViewport() || paginatedContentNeedsBaseHeight) {
>-        // FIXME: Finish accounting for block flow here.
>-        // https://bugs.webkit.org/show_bug.cgi?id=46603
>         LayoutUnit margins = collapsedMarginBefore() + collapsedMarginAfter();
>-        LayoutUnit visHeight;
>-        if (document()->printing())
>-            visHeight = static_cast<LayoutUnit>(view()->pageLogicalHeight());
>-        else  {
>-            if (isHorizontalWritingMode())
>-                visHeight = view()->viewHeight();
>-            else
>-                visHeight = view()->viewWidth();
>-        }
>+        LayoutUnit visibleHeight = viewLogicalHeightForPercentages();
>         if (isRoot())
>-            computedValues.m_extent = max(computedValues.m_extent, visHeight - margins);
>+            computedValues.m_extent = max(computedValues.m_extent, visibleHeight - margins);
>         else {
>             LayoutUnit marginsBordersPadding = margins + parentBox()->marginBefore() + parentBox()->marginAfter() + parentBox()->borderAndPaddingLogicalHeight();
>-            computedValues.m_extent = max(computedValues.m_extent, visHeight - marginsBordersPadding);
>+            computedValues.m_extent = max(computedValues.m_extent, visibleHeight - marginsBordersPadding);
>         }
>     }
> }
> 
>+LayoutUnit RenderBox::viewLogicalHeightForPercentages() const
>+{
>+    if (document()->printing())
>+        return static_cast<LayoutUnit>(view()->pageLogicalHeight());
>+    return view()->viewLogicalHeight();
>+}
>+
> LayoutUnit RenderBox::computeLogicalHeightUsing(SizeType heightType, const Length& height) const
> {
>     LayoutUnit logicalHeight = computeContentAndScrollbarLogicalHeightUsing(heightType, height);
>@@ -2542,13 +2539,14 @@ LayoutUnit RenderBox::computePercentageLogicalHeight(const Length& height) const
>             LayoutUnit contentBoxHeight = cb->constrainContentBoxLogicalHeightByMinMax(contentBoxHeightWithScrollbar - cb->scrollbarLogicalHeight());
>             availableHeight = max<LayoutUnit>(0, contentBoxHeight);
>         }
>-    } else if (cb->isRenderView() || isOutOfFlowPositionedWithSpecifiedHeight) {
>+    } else if (isOutOfFlowPositionedWithSpecifiedHeight) {
>         // Don't allow this to affect the block' height() member variable, since this
>         // can get called while the block is still laying out its kids.
>         LogicalExtentComputedValues computedValues;
>         cb->computeLogicalHeight(cb->logicalHeight(), 0, computedValues);
>         availableHeight = computedValues.m_extent - cb->borderAndPaddingLogicalHeight() - cb->scrollbarLogicalHeight();
>-    }
>+    } else if (cb->isRenderView())
>+        availableHeight = viewLogicalHeightForPercentages();
> 
>     if (availableHeight == -1)
>         return availableHeight;
>diff --git a/Source/WebCore/rendering/RenderBox.h b/Source/WebCore/rendering/RenderBox.h
>index cad21ff059de14f9fff162b616b5ca783cb9a3b2..35923efbd0d07c025f1c237feff9f0330633ea20 100644
>--- a/Source/WebCore/rendering/RenderBox.h
>+++ b/Source/WebCore/rendering/RenderBox.h
>@@ -414,6 +414,8 @@ public:
> 
>     LayoutUnit shrinkLogicalWidthToAvoidFloats(LayoutUnit childMarginStart, LayoutUnit childMarginEnd, const RenderBlock* cb, RenderRegion*, LayoutUnit offsetFromLogicalTopOfFirstPage) const;
> 
>+    LayoutUnit viewLogicalHeightForPercentages() const;
>+
>     LayoutUnit computeLogicalWidthInRegionUsing(SizeType, LayoutUnit availableLogicalWidth, const RenderBlock* containingBlock, RenderRegion*, LayoutUnit offsetFromLogicalTopOfFirstPage) const;
>     LayoutUnit computeLogicalHeightUsing(SizeType, const Length& height) const;
>     LayoutUnit computeContentLogicalHeight(SizeType, const Length& height) const;
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ad458ac14457f4c066008b1e749d185367778c3b..a74755b20efb7f4e179999db741f046f186b3935 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,22 @@
>+2013-01-30  Ojan Vafai  <ojan@chromium.org>
>+
>+        [REGRESSION: r128517] Percentage heights in quirks mode collapse when printing
>+        https://bugs.webkit.org/show_bug.cgi?id=108382
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * platform/chromium/fast/multicol/shrink-to-column-height-for-pagination-expected.txt:
>+        We're just clipping more content that you can't scroll to anyways. This looks
>+        like an improvement to me.
>+        * printing/css2.1/page-break-inside-000-expected.txt:
>+        We pass this test now!
>+        * printing/quirks-percentage-height-body-expected.html: Added.
>+        * printing/quirks-percentage-height-body.html: Added.
>+        * printing/quirks-percentage-height-expected.html: Added.
>+        * printing/quirks-percentage-height.html: Added.
>+        * printing/standards-percentage-heights-expected.html: Added.
>+        * printing/standards-percentage-heights.html: Added.
>+
> 2013-01-29  Nate Chapin  <japhet@chromium.org>
> 
>         Enable reuse of cached main resources
>diff --git a/LayoutTests/platform/chromium/fast/multicol/shrink-to-column-height-for-pagination-expected.txt b/LayoutTests/platform/chromium/fast/multicol/shrink-to-column-height-for-pagination-expected.txt
>index c93a3f3f4050ff9555b7fa39faf887842adda1f3..ded30a7df0ebf2845e1e77875f5788bcb0cf90b3 100644
>--- a/LayoutTests/platform/chromium/fast/multicol/shrink-to-column-height-for-pagination-expected.txt
>+++ b/LayoutTests/platform/chromium/fast/multicol/shrink-to-column-height-for-pagination-expected.txt
>@@ -1,7 +1,7 @@
> layer at (0,0) size 800x600
>   RenderView at (0,0) size 400x600
>-layer at (-400,0) size 800x600 backgroundClip at (0,0) size 800x600 clip at (0,0) size 800x600 outlineClip at (0,0) size 800x600
>-  RenderBlock {HTML} at (0,0) size 800x600
>+layer at (-16,0) size 416x600 backgroundClip at (0,0) size 800x600 clip at (0,0) size 800x600 outlineClip at (0,0) size 800x600
>+  RenderBlock {HTML} at (0,0) size 416x600
>     RenderBody {BODY} at (8,8) size 400x600
>       RenderBlock {DIV} at (0,0) size 400x600
>         RenderImage {IMG} at (392,0) size 400x600
>diff --git a/LayoutTests/printing/css2.1/page-break-inside-000-expected.txt b/LayoutTests/printing/css2.1/page-break-inside-000-expected.txt
>index dc28776a907703ae41e52fe52c22379c2fa278c6..b353f06b5d0747c2f2830c81065ccd54de3345bb 100644
>--- a/LayoutTests/printing/css2.1/page-break-inside-000-expected.txt
>+++ b/LayoutTests/printing/css2.1/page-break-inside-000-expected.txt
>@@ -1,5 +1,6 @@
> There must be a page break below between "FIRST" and "LAST"
> FIRST dummy dummy dummy dummy dummy dummy dummy dummy dummy FIRST
> 
>-FAIL: expected page number of "test1" is 0. Was -1
>-FAIL: expected page number of "test2" is 1. Was 0
>+PASS: page number of "test1" is 0
>+PASS: page number of "test2" is 1
>+All tests passed
>diff --git a/LayoutTests/printing/quirks-percentage-height-body-expected.html b/LayoutTests/printing/quirks-percentage-height-body-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..234dbf4d3652a2026c253986a4b76ac060599898
>--- /dev/null
>+++ b/LayoutTests/printing/quirks-percentage-height-body-expected.html
>@@ -0,0 +1,2 @@
>+
>+<body style="margin: 0; height: 50%; width: 50%; border: 4px solid salmon;"></body>
>diff --git a/LayoutTests/printing/quirks-percentage-height-body.html b/LayoutTests/printing/quirks-percentage-height-body.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..b3d05d9a581df1a98537b0ff6af55bacf13c7d21
>--- /dev/null
>+++ b/LayoutTests/printing/quirks-percentage-height-body.html
>@@ -0,0 +1,5 @@
>+<script>
>+if (window.testRunner)
>+    testRunner.setPrinting();
>+</script>
>+<body style="margin: 0; height: 50%; width: 50%; border: 5px solid salmon;"></body>
>diff --git a/LayoutTests/printing/quirks-percentage-height-expected.html b/LayoutTests/printing/quirks-percentage-height-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..a62d974c7e5cda6f678e540f8607ba221d7b3c5a
>--- /dev/null
>+++ b/LayoutTests/printing/quirks-percentage-height-expected.html
>@@ -0,0 +1,4 @@
>+
>+<body style="margin: 0;">
>+    <div style="height: 50%; width: 50%; border: 4px solid salmon;"></div>
>+</body>
>diff --git a/LayoutTests/printing/quirks-percentage-height.html b/LayoutTests/printing/quirks-percentage-height.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..a60fd6e8e19dcec686d637025b70c0acef8a2095
>--- /dev/null
>+++ b/LayoutTests/printing/quirks-percentage-height.html
>@@ -0,0 +1,7 @@
>+<script>
>+if (window.testRunner)
>+    testRunner.setPrinting();
>+</script>
>+<body style="margin: 0;">
>+    <div style="height: 50%; width: 50%; border: 5px solid salmon;"></div>
>+</body>
>diff --git a/LayoutTests/printing/standards-percentage-heights-expected.html b/LayoutTests/printing/standards-percentage-heights-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..5dd668fdadf8732a781b650d13cb8262f464ba0d
>--- /dev/null
>+++ b/LayoutTests/printing/standards-percentage-heights-expected.html
>@@ -0,0 +1,4 @@
>+<!DOCTYPE html>
>+
>+<html style="overflow: hidden; height: 400px; width: 400px; border: 4px solid pink">
>+<body style="margin: 0; height: 50%; width: 50%; border: 4px solid salmon;"></body>
>diff --git a/LayoutTests/printing/standards-percentage-heights.html b/LayoutTests/printing/standards-percentage-heights.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..09bea6fc4d447a61fc47957b28e0815897a2c61a
>--- /dev/null
>+++ b/LayoutTests/printing/standards-percentage-heights.html
>@@ -0,0 +1,7 @@
>+<!DOCTYPE html>
>+<html style="overflow: hidden; height: 500px; width: 500px; border: 5px solid pink">
>+<script>
>+if (window.testRunner)
>+    testRunner.setPrinting();
>+</script>
>+<body style="margin: 0; height: 50%; width: 50%; border: 5px solid salmon;"></body>

Actions: View | Formatted Diff | Diff

Attachments on bug 108382: 185569 | 185606 | 185823