Title: [127915] trunk
Revision
127915
Author
o...@chromium.org
Date
2012-09-07 13:54:56 -0700 (Fri, 07 Sep 2012)

Log Message

Fix RenderBox::availableHeight to subtract scrollbars in the right places
https://bugs.webkit.org/show_bug.cgi?id=96031

Reviewed by Tony Chang.

Source/WebCore:

This matches Firefox 15 and IE9 rendering for the two new tests.

Tests: fast/block/positioning/percent-top-left-on-relative-position.html
       fast/css/nested-percent-height-on-replaced.html

* rendering/RenderBox.cpp:
(WebCore::RenderBox::computePercentageLogicalHeight):
Subtract scrollbars when recurring on percentage heights.
(WebCore::RenderBox::computeReplacedLogicalHeightUsing):
This is the wrong place to subtract scrollbars. availableLogicalHeight
should return a value that doesn't include scrollbars.
(WebCore::RenderBox::availableLogicalHeightUsing):
Subtract scrollbars from specified heights. Also, consolidate the code to
use computeContentLogicalHeightUsing. This makes percentage heights use
the right containingBlock in quirks mode and makes viewport percentage heights work.

LayoutTests:

* fast/block/positioning/percent-top-left-on-relative-position-expected.html: Added.
* fast/block/positioning/percent-top-left-on-relative-position.html: Added.
* fast/css/nested-percent-height-on-replaced-expected.txt: Added.
* fast/css/nested-percent-height-on-replaced.html: Added.
* platform/chromium-linux/fast/css/percent-top-value-with-relative-position-expected.png:
* platform/chromium-win/fast/css/percent-top-value-with-relative-position-expected.txt:
This is now correctly centering the 50%. It moved up 8px because that is half of the body's margin.
This matches other Firefox 15 and IE9.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (127914 => 127915)


--- trunk/LayoutTests/ChangeLog	2012-09-07 20:50:41 UTC (rev 127914)
+++ trunk/LayoutTests/ChangeLog	2012-09-07 20:54:56 UTC (rev 127915)
@@ -1,3 +1,19 @@
+2012-09-07  Ojan Vafai  <o...@chromium.org>
+
+        Fix RenderBox::availableHeight to subtract scrollbars in the right places
+        https://bugs.webkit.org/show_bug.cgi?id=96031
+
+        Reviewed by Tony Chang.
+
+        * fast/block/positioning/percent-top-left-on-relative-position-expected.html: Added.
+        * fast/block/positioning/percent-top-left-on-relative-position.html: Added.
+        * fast/css/nested-percent-height-on-replaced-expected.txt: Added.
+        * fast/css/nested-percent-height-on-replaced.html: Added.
+        * platform/chromium-linux/fast/css/percent-top-value-with-relative-position-expected.png:
+        * platform/chromium-win/fast/css/percent-top-value-with-relative-position-expected.txt:
+        This is now correctly centering the 50%. It moved up 8px because that is half of the body's margin.
+        This matches other Firefox 15 and IE9.
+
 2012-09-07  Tim Horton  <timothy_hor...@apple.com>
 
         Unreviewed, fix incorrect comment character(s) in Skipped list.

Added: trunk/LayoutTests/fast/block/positioning/percent-top-left-on-relative-position-expected.html (0 => 127915)


--- trunk/LayoutTests/fast/block/positioning/percent-top-left-on-relative-position-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/block/positioning/percent-top-left-on-relative-position-expected.html	2012-09-07 20:54:56 UTC (rev 127915)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<style>
+html,body {
+    margin: 0;
+    padding: 0;
+}
+.container {
+    height: 50px;
+    width: 50px;
+    background-color: orange;
+    border: 1px solid;
+}
+.child {
+    margin: 20%;
+    height: 10px;
+    width: 10px;
+    background-color: salmon;
+}
+</style>
+<div class="container">
+    <div class="child"></div>
+</div>
+<br>
+<div class="container" style="overflow:scroll">
+    <div class="child"></div>
+</div>

Added: trunk/LayoutTests/fast/block/positioning/percent-top-left-on-relative-position.html (0 => 127915)


--- trunk/LayoutTests/fast/block/positioning/percent-top-left-on-relative-position.html	                        (rev 0)
+++ trunk/LayoutTests/fast/block/positioning/percent-top-left-on-relative-position.html	2012-09-07 20:54:56 UTC (rev 127915)
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<style>
+body {
+    margin: 0;
+}
+.container {
+    border: 1px solid;
+    height: 50px;
+    width: 50px;
+    background-color: orange;
+}
+.child {
+    position: relative;
+    top: 20%;
+    left: 20%;
+    height: 10px;
+    width: 10px;
+    background-color: salmon;
+}
+</style>
+<div class="container">
+    <div class="child"></div>
+</div>
+<br>
+<div class="container" style="overflow:scroll">
+    <div class="child"></div>
+</div>

Added: trunk/LayoutTests/fast/css/nested-percent-height-on-replaced-expected.txt (0 => 127915)


--- trunk/LayoutTests/fast/css/nested-percent-height-on-replaced-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/css/nested-percent-height-on-replaced-expected.txt	2012-09-07 20:54:56 UTC (rev 127915)
@@ -0,0 +1,3 @@
+PASS
+
+PASS

Added: trunk/LayoutTests/fast/css/nested-percent-height-on-replaced.html (0 => 127915)


--- trunk/LayoutTests/fast/css/nested-percent-height-on-replaced.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css/nested-percent-height-on-replaced.html	2012-09-07 20:54:56 UTC (rev 127915)
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<script src=""
+<body>
+
+<div style="overflow: scroll; height: 200px; width: 400px; background-color: salmon">
+    <div style="overflow: scroll; height: 100%; background-color: purple">
+        <div style="overflow: scroll; height: 100%; background-color: purple">
+            <div style="border: 0; height: 100%; width: 100%; background-color: orange"></div>
+        </div>
+    </div>
+</div>
+
+<div style="overflow: scroll; height: 200px; width: 400px; background-color: salmon">
+    <div style="overflow: scroll; height: 100%; background-color: purple">
+        <div style="overflow: scroll; height: 100%; background-color: purple;line-height: 0">
+            <iframe style="border: 0; height: 100%; width: 100%; background-color: orange"></iframe>
+        </div>
+    </div>
+</div>
+
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+var dummy = document.createElement('div');
+dummy.style.overflow = "scroll";
+document.body.appendChild(dummy);
+var scrollbarWidth = dummy.offsetWidth - dummy.clientWidth;
+document.body.removeChild(dummy);
+
+function setExpected(node, width, height)
+{
+    node.setAttribute('data-expected-width', width);
+    node.setAttribute('data-expected-height', height);
+}
+
+var containers = document.querySelectorAll('body > div');
+for (var i = 0; i < containers.length; i++) {
+    var width = containers[i].offsetWidth;
+    var height = containers[i].offsetHeight;
+
+    width -= scrollbarWidth;
+    height -= scrollbarWidth;
+    var first = containers[i].querySelector('div');
+    setExpected(first, width, height);
+
+    width -= scrollbarWidth;
+    height -= scrollbarWidth;
+    var second = first.querySelector('div');
+    setExpected(second, width, height);
+
+    width -= scrollbarWidth;
+    height -= scrollbarWidth;
+    var third = second.querySelector('*');
+    setExpected(third, width, height);
+}
+
+checkLayout('body > div')
+</script>
+
+</body>
\ No newline at end of file

Modified: trunk/LayoutTests/platform/chromium/TestExpectations (127914 => 127915)


--- trunk/LayoutTests/platform/chromium/TestExpectations	2012-09-07 20:50:41 UTC (rev 127914)
+++ trunk/LayoutTests/platform/chromium/TestExpectations	2012-09-07 20:54:56 UTC (rev 127915)
@@ -3598,5 +3598,7 @@
 
 BUGWK95813 : fast/innerHTML/innerHTML-iframe.html = TEXT PASS
 
+BUG_OJAN MAC WIN : fast/css/percent-top-value-with-relative-position.html = IMAGE+TEXT
+
 BUGWK96041 LINUX : platform/chromium-linux/compositing/gestures = PASS IMAGE
-BUGWK96041 LINUX : http/tests/cache/cancel-during-revalidation-succeeded.html = PASS TEXT
+BUGWK96041 LINUX : http/tests/cache/cancel-during-revalidation-succeeded.html = PASS TEXT
\ No newline at end of file

Modified: trunk/LayoutTests/platform/chromium-linux/fast/css/percent-top-value-with-relative-position-expected.png


(Binary files differ)

Modified: trunk/LayoutTests/platform/chromium-win/fast/css/percent-top-value-with-relative-position-expected.txt (127914 => 127915)


--- trunk/LayoutTests/platform/chromium-win/fast/css/percent-top-value-with-relative-position-expected.txt	2012-09-07 20:50:41 UTC (rev 127914)
+++ trunk/LayoutTests/platform/chromium-win/fast/css/percent-top-value-with-relative-position-expected.txt	2012-09-07 20:54:56 UTC (rev 127915)
@@ -4,7 +4,7 @@
   RenderBlock {HTML} at (0,0) size 800x600
     RenderBody {BODY} at (8,8) size 784x584
       RenderBlock {DIV} at (0,0) size 784x584
-layer at (8,308) size 784x20
+layer at (8,300) size 784x20
   RenderBlock (relative positioned) {DIV} at (0,0) size 784x20
     RenderText {#text} at (0,0) size 379x19
       text run at (0,0) width 379: "This test passes if the text is rendered in the middle of the page."

Modified: trunk/LayoutTests/platform/efl/TestExpectations (127914 => 127915)


--- trunk/LayoutTests/platform/efl/TestExpectations	2012-09-07 20:50:41 UTC (rev 127914)
+++ trunk/LayoutTests/platform/efl/TestExpectations	2012-09-07 20:54:56 UTC (rev 127915)
@@ -1089,3 +1089,5 @@
 
 // EFL does not yet support global selection
 BUGWK88238 SKIP : editing/pasteboard/paste-global-selection.html = TEXT
+
+BUG_OJAN : fast/css/percent-top-value-with-relative-position.html = TEXT IMAGE+TEXT

Modified: trunk/LayoutTests/platform/gtk/TestExpectations (127914 => 127915)


--- trunk/LayoutTests/platform/gtk/TestExpectations	2012-09-07 20:50:41 UTC (rev 127914)
+++ trunk/LayoutTests/platform/gtk/TestExpectations	2012-09-07 20:54:56 UTC (rev 127915)
@@ -1308,6 +1308,8 @@
 
 BUGWK88238 : editing/pasteboard/paste-global-selection.html = TEXT
 
+BUG_OJAN : fast/css/percent-top-value-with-relative-position.html = TEXT IMAGE+TEXT
+
 //////////////////////////////////////////////////////////////////////////////////////////
 // End of Tests failing
 //////////////////////////////////////////////////////////////////////////////////////////

Modified: trunk/LayoutTests/platform/mac/TestExpectations (127914 => 127915)


--- trunk/LayoutTests/platform/mac/TestExpectations	2012-09-07 20:50:41 UTC (rev 127914)
+++ trunk/LayoutTests/platform/mac/TestExpectations	2012-09-07 20:54:56 UTC (rev 127915)
@@ -371,3 +371,5 @@
 BUGWK94458 SKIP : fast/events/message-port-constructor-for-deleted-document.html = PASS
 
 BUGWK95501 SKIP : http/tests/security/inactive-document-with-empty-security-origin.html = TIMEOUT
+
+BUG_OJAN : fast/css/percent-top-value-with-relative-position.html = TEXT IMAGE+TEXT

Modified: trunk/LayoutTests/platform/qt/TestExpectations (127914 => 127915)


--- trunk/LayoutTests/platform/qt/TestExpectations	2012-09-07 20:50:41 UTC (rev 127914)
+++ trunk/LayoutTests/platform/qt/TestExpectations	2012-09-07 20:54:56 UTC (rev 127915)
@@ -140,3 +140,5 @@
 BUGWK94005 : css2.1/20110323/word-spacing-remove-space-003.htm = IMAGE
 BUGWK94005 : css2.1/20110323/word-spacing-remove-space-006.htm = IMAGE
 BUGWK94006 : fast/css/word-spacing-characters-complex-text.html = IMAGE
+
+BUG_OJAN : fast/css/percent-top-value-with-relative-position.html = TEXT IMAGE+TEXT

Modified: trunk/Source/WebCore/ChangeLog (127914 => 127915)


--- trunk/Source/WebCore/ChangeLog	2012-09-07 20:50:41 UTC (rev 127914)
+++ trunk/Source/WebCore/ChangeLog	2012-09-07 20:54:56 UTC (rev 127915)
@@ -1,3 +1,26 @@
+2012-09-07  Ojan Vafai  <o...@chromium.org>
+
+        Fix RenderBox::availableHeight to subtract scrollbars in the right places
+        https://bugs.webkit.org/show_bug.cgi?id=96031
+
+        Reviewed by Tony Chang.
+
+        This matches Firefox 15 and IE9 rendering for the two new tests.
+
+        Tests: fast/block/positioning/percent-top-left-on-relative-position.html
+               fast/css/nested-percent-height-on-replaced.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::computePercentageLogicalHeight):
+        Subtract scrollbars when recurring on percentage heights.
+        (WebCore::RenderBox::computeReplacedLogicalHeightUsing):
+        This is the wrong place to subtract scrollbars. availableLogicalHeight
+        should return a value that doesn't include scrollbars.
+        (WebCore::RenderBox::availableLogicalHeightUsing):
+        Subtract scrollbars from specified heights. Also, consolidate the code to
+        use computeContentLogicalHeightUsing. This makes percentage heights use
+        the right containingBlock in quirks mode and makes viewport percentage heights work.
+
 2012-09-07  Tony Chang  <t...@chromium.org>
 
         Make RenderBox::computeLogicalWidthInRegion const

Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (127914 => 127915)


--- trunk/Source/WebCore/rendering/RenderBox.cpp	2012-09-07 20:50:41 UTC (rev 127914)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp	2012-09-07 20:54:56 UTC (rev 127915)
@@ -2100,7 +2100,7 @@
     LayoutUnit heightIncludingScrollbar = computeContentLogicalHeightUsing(heightType, height);
     if (heightIncludingScrollbar == -1)
         return -1;
-    return std::max(LayoutUnit(0), computeContentBoxLogicalHeight(heightIncludingScrollbar) - scrollbarLogicalHeight());
+    return std::max<LayoutUnit>(0, computeContentBoxLogicalHeight(heightIncludingScrollbar) - scrollbarLogicalHeight());
 }
 
 LayoutUnit RenderBox::computeContentLogicalHeightUsing(SizeType heightType, const Length& height) const
@@ -2172,9 +2172,11 @@
         result = max<LayoutUnit>(0, contentBoxHeightWithScrollbar - cb->scrollbarLogicalHeight());
     } else if (cbstyle->logicalHeight().isPercent() && !isOutOfFlowPositionedWithSpecifiedHeight) {
         // We need to recur and compute the percentage height for our containing block.
-        result = cb->computePercentageLogicalHeight(cbstyle->logicalHeight());
-        if (result != -1)
-            result = cb->computeContentBoxLogicalHeight(result);
+        LayoutUnit heightWithScrollbar = cb->computePercentageLogicalHeight(cbstyle->logicalHeight());
+        if (heightWithScrollbar != -1) {
+            LayoutUnit contentBoxHeightWithScrollbar = cb->computeContentBoxLogicalHeight(heightWithScrollbar);
+            result = max<LayoutUnit>(0, contentBoxHeightWithScrollbar - cb->scrollbarLogicalHeight());
+        }
     } else if (cb->isRenderView() || (cb->isBody() && document()->inQuirksMode()) || 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.
@@ -2304,10 +2306,7 @@
                     cb = cb->containingBlock();
                 }
             }
-            availableHeight = computeContentBoxLogicalHeight(valueForLength(logicalHeight, availableHeight));
-            if (cb->isBox() && cb->style()->logicalHeight().isFixed())
-                availableHeight = max<LayoutUnit>(0, availableHeight - toRenderBox(cb)->scrollbarLogicalHeight());
-            return availableHeight;
+            return computeContentBoxLogicalHeight(valueForLength(logicalHeight, availableHeight));
         }
         case ViewportPercentageWidth:
         case ViewportPercentageHeight:
@@ -2325,9 +2324,6 @@
 
 LayoutUnit RenderBox::availableLogicalHeightUsing(const Length& h) const
 {
-    if (h.isFixed())
-        return computeContentBoxLogicalHeight(h.value());
-
     if (isRenderView())
         return isHorizontalWritingMode() ? toRenderView(this)->frameView()->visibleHeight() : toRenderView(this)->frameView()->visibleWidth();
 
@@ -2337,19 +2333,16 @@
     if (isTableCell() && (h.isAuto() || h.isPercent()))
         return overrideLogicalContentHeight();
 
-    if (h.isPercent()) {
-        LayoutUnit availableHeight;
-        // https://bugs.webkit.org/show_bug.cgi?id=64046
-        // For absolutely positioned elements whose containing block is based on a block-level element,
-        // the percentage is calculated with respect to the height of the padding box of that element
-        if (isOutOfFlowPositioned())
-            availableHeight = containingBlockLogicalHeightForPositioned(containingBlock());
-        else
-            availableHeight = containingBlock()->availableLogicalHeight();
+    if (h.isPercent() && isOutOfFlowPositioned()) {
+        LayoutUnit availableHeight = containingBlockLogicalHeightForPositioned(containingBlock());
         return computeContentBoxLogicalHeight(valueForLength(h, availableHeight));
     }
 
-    // FIXME: We can't just check top/bottom here.
+    LayoutUnit heightIncludingScrollbar = computeContentLogicalHeightUsing(MainOrPreferredSize, h);
+    if (heightIncludingScrollbar != -1)
+        return std::max<LayoutUnit>(0, computeContentBoxLogicalHeight(heightIncludingScrollbar) - scrollbarLogicalHeight());
+
+    // FIXME: Check logicalTop/logicalBottom here to correctly handle vertical writing-mode.
     // https://bugs.webkit.org/show_bug.cgi?id=46500
     if (isRenderBlock() && isOutOfFlowPositioned() && style()->height().isAuto() && !(style()->top().isAuto() || style()->bottom().isAuto())) {
         RenderBlock* block = const_cast<RenderBlock*>(toRenderBlock(this));
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to