max-height property not respected in case of tables

https://bugs.webkit.org/show_bug.cgi?id=98633 Patch by Pravin D <[email protected]> on 2012-11-27 Reviewed by Julien Chaffraix. Source/WebCore: The max-height property determines the maximum computed height an element can have. In case of tables the computed height was not being limited by the max-height property. The current patch fixes the same. Test: fast/table/css-table-max-height.html * rendering/RenderTable.cpp: (WebCore::RenderTable::convertStyleLogicalHeightToComputedHeight): Helper function to compute height from the given style height. This function handles style height of type fixed, percent and viewport percent. As height of type 'calculated' gets internally resolved to either fixed or percent there is no special handling required for the same. (WebCore): (WebCore::RenderTable::layout): Logic to compute the logical height of an element such that it does not exceed the max-height value given that min-width < Content height < max-height, when min-height < max-height. However max-height value is not respected if either min-height > max-height or Content height > max-height. * rendering/RenderTable.h: (RenderTable): Function definition for the newly add function convertStyleLogicalHeightToComputedHeight(). LayoutTests: * fast/table/css-table-max-height-expected.txt: Added. * fast/table/css-table-max-height.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@135891 268f45cc-cd09-0410-ab3c-d52691b4dbfc
ziswiler · Nov 27, 2012 · cc747f4 · cc747f4
1 parent f8c632e
commit cc747f4
Show file tree

Hide file tree

Showing 6 changed files with 231 additions and 11 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2012-11-27  Pravin D  <[email protected]>
+
+        max-height property not respected in case of tables
+        https://bugs.webkit.org/show_bug.cgi?id=98633
+
+        Reviewed by Julien Chaffraix.
+
+        * fast/table/css-table-max-height-expected.txt: Added.
+        * fast/table/css-table-max-height.html: Added.
+
 2012-11-27  Roger Fong  <[email protected]>
 
         Windows specific implementation of usesTileCacheLayer needed after r133056.

diff --git a/LayoutTests/fast/table/css-table-max-height-expected.txt b/LayoutTests/fast/table/css-table-max-height-expected.txt
@@ -0,0 +1,38 @@
+Testcase for Bug http://wkbug.com/98633. The testcase checks if the height of a css table does not exceed the max-height value. The height of the table can be greater than max-height value when either min-height is greater than max-height or computed height of the content is greater than the max-height.
+
+This sub-test checks that when min-height is lesser than max-height, max-height is applied to a table with auto layout.
+PASS
+This sub-test checks that when min-height is greater than max-height, min-height is applied to a table with auto layout.
+PASS
+This sub-test checks that max-height with fixed value is applied to a table with auto layout.
+PASS
+This sub-test checks that max-height with percent value is applied to a table with auto layout.
+PASS
+This sub-test checks that max-height with viewport percent height value is applied to a table with auto layout.
+PASS
+This sub-test checks that max-height with viewport percent width value is applied to a table with auto layout.
+PASS
+This sub-test checks that when content height is greater than max-height, content height is applied to the table with auto layout. 
+
+FILLER TEXT TO INCREASE CONTENT HEIGHT.
+PASS
+This test checks that when height of an auto-table is auto, there is are no asserts. No crash means the test passed.
+PASS
+This sub-test checks that when min-height is lesser than max-height, max-height is applied to a table with fixed layout.
+PASS
+This sub-test checks that when min-height is greater than max-height, min-height is applied to a table with fixed layout.
+PASS
+This sub-test checks that max-height with fixed value is applied to a table with fixed layout.
+PASS
+This sub-test checks that max-height with percent value is applied to a table with fixed layout.
+PASS
+This sub-test checks that max-height with viewport percent height value is applied to a table with fixed layout.
+PASS
+This sub-test checks that max-height with viewport percent width value is applied to a table with fixed layout.
+PASS
+This sub-test checks that when content height is greater than max-height, content height is applied to a table with fixed layout. 
+
+FILLER TEXT TO INCREASE CONTENT HEIGHT.
+PASS
+This test checks that when height of a fixed table is auto, there is are no asserts. No crash means the test passed.
+PASS
diff --git a/LayoutTests/fast/table/css-table-max-height.html b/LayoutTests/fast/table/css-table-max-height.html
@@ -0,0 +1,115 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style type="text/css">
+.container
+{
+    width:300px;
+    height:400px;
+    font-family:ahem;
+    background-color:#999999;
+}
+
+.child
+{
+    display:table;
+    height:100%;
+    background-color:green;
+}
+.fixed-table
+{
+    table-layout:fixed;
+}
+</style>
+<script src="../../resources/check-layout.js"></script>
+</head>
+<body onload="checkLayout('.child')">
+<div> Testcase for Bug <a href="http://wkbug.com/98633">http://wkbug.com/98633</a>. The testcase checks if the height of a 
+css table does not exceed the max-height value. The height of the table can be greater than max-height value when either
+min-height is greater than max-height or computed height of the content is greater than the max-height.
+</div>
+<br>
+<div class="container">
+    <div id="maxGreatThanMinHeightAutoLayout" class="child" style="min-height:100px; max-height:200px;" data-expected-height=200>
+        This sub-test checks that when min-height is lesser than max-height, max-height is applied to a table with auto layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child" style="min-height:200px; max-height:100px;" data-expected-height=200>
+        This sub-test checks that when min-height is greater than max-height, min-height is applied to a table with auto layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child" style="max-height:200px;" data-expected-height=200>
+        This sub-test checks that max-height with fixed value is applied to a table with auto layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child" style="max-height:50%;" data-expected-height=200>
+        This sub-test checks that max-height with percent value is applied to a table with auto layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child" style="max-height:50vh;" data-expected-height=300>
+        This sub-test checks that max-height with viewport percent height value is applied to a table with auto layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child" style="max-height:25vw;" data-expected-height=200>
+        This sub-test checks that max-height with viewport percent width value is applied to a table with auto layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child" style="max-height:100px;" data-expected-height=192>
+        This sub-test checks that when content height is greater than max-height, content height is applied to the table with auto layout.
+        <br><br>FILLER TEXT TO INCREASE CONTENT HEIGHT.
+    </div>
+</div>
+<div class="container">
+    <div class="child fixed-table" style="height:auto;">
+        This test checks that when height of an auto-table is auto, there is are no asserts. No crash means the test passed.
+    </div>
+</div>
+<div class="container">
+    <div class="child fixed-table" style="min-height:100px; max-height:200px;" data-expected-height=200>
+        This sub-test checks that when min-height is lesser than max-height, max-height is applied to a table with fixed layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child fixed-table" style="min-height:200px; max-height:100px;" data-expected-height=200>
+        This sub-test checks that when min-height is greater than max-height, min-height is applied to a table with fixed layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child fixed-table" style="max-height:200px;" data-expected-height=200>
+        This sub-test checks that max-height with fixed value is applied to a table with fixed layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child fixed-table" style="max-height:50%;" data-expected-height=200>
+        This sub-test checks that max-height with percent value is applied to a table with fixed layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child fixed-table" style="max-height:50vh;" data-expected-height=300>
+        This sub-test checks that max-height with viewport percent height value is applied to a table with fixed layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child fixed-table" style="max-height:25vw;" data-expected-height=200>
+        This sub-test checks that max-height with viewport percent width value is applied to a table with fixed layout.
+    </div>
+</div>
+<div class="container">
+    <div class="child fixed-table" style="max-height:100px;" data-expected-height=192>
+        This sub-test checks that when content height is greater than max-height, content height is applied to a table with fixed layout.
+        <br><br>FILLER TEXT TO INCREASE CONTENT HEIGHT.
+    </div>
+</div>
+<div class="container">
+    <div class="child fixed-table" style="height:auto;">
+        This test checks that when height of a fixed table is auto, there is are no asserts. No crash means the test passed.
+    </div>
+</div>
+<body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2012-11-27  Pravin D  <[email protected]>
+
+        max-height property not respected in case of tables
+        https://bugs.webkit.org/show_bug.cgi?id=98633
+
+        Reviewed by Julien Chaffraix.
+
+        The max-height property determines the maximum computed height an element can have. In case of tables
+        the computed height was not being limited by the max-height property. The current patch fixes the same.
+
+        Test: fast/table/css-table-max-height.html
+
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::convertStyleLogicalHeightToComputedHeight):
+          Helper function to compute height from the given style height.
+          This function handles style height of type fixed, percent and viewport percent.
+          As height of type 'calculated' gets internally resolved to either fixed or percent
+          there is no special handling required for the same.
+
+        (WebCore):
+        (WebCore::RenderTable::layout):
+          Logic to compute the logical height of an element such that it does not exceed the max-height value given that
+          min-width < Content height < max-height, when min-height < max-height.
+          However max-height value is not respected if either min-height > max-height or Content height > max-height.
+
+        * rendering/RenderTable.h:
+        (RenderTable):
+          Function definition for the newly add function convertStyleLogicalHeightToComputedHeight().
+
 2012-11-27  Roger Fong  <[email protected]>
 
         Windows specific implementation of usesTileCacheLayer needed after r133056.

diff --git a/Source/WebCore/rendering/RenderTable.cpp b/Source/WebCore/rendering/RenderTable.cpp
@@ -326,6 +326,28 @@ LayoutUnit RenderTable::convertStyleLogicalWidthToComputedWidth(const Length& st
     return minimumValueForLength(styleLogicalWidth, availableWidth, view()) + borders;
 }
 
+LayoutUnit RenderTable::convertStyleLogicalHeightToComputedHeight(const Length& styleLogicalHeight)
+{
+    LayoutUnit computedLogicalHeight = 0;
+    if (styleLogicalHeight.isFixed()) {
+        // HTML tables size as though CSS height includes border/padding, CSS tables do not.
+        LayoutUnit borders = LayoutUnit();
+        // FIXME: We cannot apply box-sizing: content-box on <table> which other browsers allow.
+        if ((node() && node()->hasTagName(tableTag)) || style()->boxSizing() == BORDER_BOX) {
+            LayoutUnit borderAndPaddingBefore = borderBefore() + (collapseBorders() ? LayoutUnit() : paddingBefore());
+            LayoutUnit borderAndPaddingAfter = borderAfter() + (collapseBorders() ? LayoutUnit() : paddingAfter());
+            borders = borderAndPaddingBefore + borderAndPaddingAfter;
+        }
+        computedLogicalHeight = styleLogicalHeight.value() - borders;
+    } else if (styleLogicalHeight.isPercent())
+        computedLogicalHeight = computePercentageLogicalHeight(styleLogicalHeight);
+    else if (styleLogicalHeight.isViewportPercentage())
+        computedLogicalHeight = minimumValueForLength(styleLogicalHeight, 0, view());
+    else
+        ASSERT_NOT_REACHED();
+    return max<LayoutUnit>(0, computedLogicalHeight);
+}
+
 void RenderTable::layoutCaption(RenderTableCaption* caption)
 {
     LayoutRect captionRect(caption->frameRect());
@@ -442,18 +464,23 @@ void RenderTable::layout()
     if (!isOutOfFlowPositioned())
         updateLogicalHeight();
 
-    Length logicalHeightLength = style()->logicalHeight();
     LayoutUnit computedLogicalHeight = 0;
-    if (logicalHeightLength.isFixed()) {
-        // HTML tables size as though CSS height includes border/padding, CSS tables do not.
-        LayoutUnit borders = 0;
-        // FIXME: We cannot apply box-sizing: content-box on <table> which other browsers allow.
-        if ((node() && node()->hasTagName(tableTag)) || style()->boxSizing() == BORDER_BOX)
-            borders = borderAndPaddingBefore + borderAndPaddingAfter;
-        computedLogicalHeight = logicalHeightLength.value() - borders;
-    } else if (logicalHeightLength.isPercent())
-        computedLogicalHeight = computePercentageLogicalHeight(logicalHeightLength);
-    computedLogicalHeight = max<LayoutUnit>(0, computedLogicalHeight);
+
+    Length logicalHeightLength = style()->logicalHeight();
+    if (logicalHeightLength.isSpecified() && logicalHeightLength.isPositive())
+        computedLogicalHeight = convertStyleLogicalHeightToComputedHeight(logicalHeightLength);
+
+    Length logicalMaxHeightLength = style()->logicalMaxHeight();
+    if (logicalMaxHeightLength.isSpecified() && !logicalMaxHeightLength.isNegative()) {
+        LayoutUnit computedMaxLogicalHeight = convertStyleLogicalHeightToComputedHeight(logicalMaxHeightLength);
+        computedLogicalHeight = min(computedLogicalHeight, computedMaxLogicalHeight);
+    }
+
+    Length logicalMinHeightLength = style()->logicalMinHeight();
+    if (logicalMinHeightLength.isSpecified() && !logicalMinHeightLength.isNegative()) {
+        LayoutUnit computedMinLogicalHeight = convertStyleLogicalHeightToComputedHeight(logicalMinHeightLength);
+        computedLogicalHeight = max(computedLogicalHeight, computedMinLogicalHeight);
+    }
 
     distributeExtraLogicalHeight(floorToInt(computedLogicalHeight - totalSectionLogicalHeight));
 

diff --git a/Source/WebCore/rendering/RenderTable.h b/Source/WebCore/rendering/RenderTable.h
@@ -291,6 +291,7 @@ class RenderTable : public RenderBlock {
     virtual void updateLogicalWidth() OVERRIDE;
 
     LayoutUnit convertStyleLogicalWidthToComputedWidth(const Length& styleLogicalWidth, LayoutUnit availableWidth);
+    LayoutUnit convertStyleLogicalHeightToComputedHeight(const Length& styleLogicalHeight);
 
     virtual LayoutRect overflowClipRect(const LayoutPoint& location, RenderRegion*, OverlayScrollbarSizeRelevancy = IgnoreOverlayScrollbarSize);