max-height property not respected in case of tables
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Nov 2012 19:52:43 +0000 (19:52 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Nov 2012 19:52:43 +0000 (19:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=98633

Patch by Pravin D <pravind.2k4@gmail.com> 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: https://svn.webkit.org/repository/webkit/trunk@135891 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/table/css-table-max-height-expected.txt [new file with mode: 0644]
LayoutTests/fast/table/css-table-max-height.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderTable.cpp
Source/WebCore/rendering/RenderTable.h

index ab1fdd9..c2eb304 100644 (file)
@@ -1,3 +1,13 @@
+2012-11-27  Pravin D  <pravind.2k4@gmail.com>
+
+        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  <roger_fong@apple.com>
 
         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
new file mode 100644 (file)
index 0000000..00d3b35
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..a8497cd
--- /dev/null
@@ -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>
index 45668fe..2c27455 100644 (file)
@@ -1,3 +1,32 @@
+2012-11-27  Pravin D  <pravind.2k4@gmail.com>
+
+        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  <roger_fong@apple.com>
 
         Windows specific implementation of usesTileCacheLayer needed after r133056.
index 7527c47..7d5cfe9 100644 (file)
@@ -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));
 
index 589b9de..55a4509 100644 (file)
@@ -291,6 +291,7 @@ private:
     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);