- Revision
- 130549
- Author
- o...@chromium.org
- Date
- 2012-10-05 14:12:18 -0700 (Fri, 05 Oct 2012)
Log Message
Deprecated flexboxes subtract scrollbar width/height twice
https://bugs.webkit.org/show_bug.cgi?id=98552
Reviewed by Tony Chang.
Source/WebCore:
This is a regression from http://trac.webkit.org/changeset/119507.
The problem is that contentHeight subtracts the scrollbar and
RenderDeprecatedFlexbox subtracts the scrollbar.
-Make it so that we only access override sizes if one has been set.
I think this makes the calling code more clear.
-If we don't have one set, grab the height/width - borderAndPadding.
-Add a FIXME to change this all back to being borderbox sizes.
There's something trick with making table padding/border work right for that
though (noted in the original patch).
Test: fast/flexbox/flexing-overflow-scroll-item.html
* rendering/RenderBox.cpp:
(WebCore::RenderBox::overrideLogicalContentWidth):
(WebCore::RenderBox::overrideLogicalContentHeight):
(WebCore::RenderBox::availableLogicalHeightUsing):
* rendering/RenderBox.h:
(RenderBox):
* rendering/RenderDeprecatedFlexibleBox.cpp:
(WebCore::contentWidthForChild):
(WebCore):
(WebCore::contentHeightForChild):
(WebCore::RenderDeprecatedFlexibleBox::layoutHorizontalBox):
(WebCore::RenderDeprecatedFlexibleBox::layoutVerticalBox):
(WebCore::RenderDeprecatedFlexibleBox::allowedChildFlex):
LayoutTests:
* fast/flexbox/flexing-overflow-scroll-item-expected.txt: Added.
* fast/flexbox/flexing-overflow-scroll-item.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (130548 => 130549)
--- trunk/LayoutTests/ChangeLog 2012-10-05 20:47:16 UTC (rev 130548)
+++ trunk/LayoutTests/ChangeLog 2012-10-05 21:12:18 UTC (rev 130549)
@@ -1,3 +1,13 @@
+2012-10-05 Ojan Vafai <o...@chromium.org>
+
+ Deprecated flexboxes subtract scrollbar width/height twice
+ https://bugs.webkit.org/show_bug.cgi?id=98552
+
+ Reviewed by Tony Chang.
+
+ * fast/flexbox/flexing-overflow-scroll-item-expected.txt: Added.
+ * fast/flexbox/flexing-overflow-scroll-item.html: Added.
+
2012-10-05 Ryosuke Niwa <rn...@webkit.org>
GTK+ and Qt rebaselines after r130532.
Added: trunk/LayoutTests/fast/flexbox/flexing-overflow-scroll-item-expected.txt (0 => 130549)
--- trunk/LayoutTests/fast/flexbox/flexing-overflow-scroll-item-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/flexbox/flexing-overflow-scroll-item-expected.txt 2012-10-05 21:12:18 UTC (rev 130549)
@@ -0,0 +1,2 @@
+PASS
+PASS
Added: trunk/LayoutTests/fast/flexbox/flexing-overflow-scroll-item.html (0 => 130549)
--- trunk/LayoutTests/fast/flexbox/flexing-overflow-scroll-item.html (rev 0)
+++ trunk/LayoutTests/fast/flexbox/flexing-overflow-scroll-item.html 2012-10-05 21:12:18 UTC (rev 130549)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<div id="flexbox" style="display: -webkit-box; height: 100px; width: 200px;">
+ <div style="overflow: scroll; -webkit-box-flex: 1;" data-expected-width=200 data-expected-height=100></div>
+</div>
+
+<div id="flexbox" style="display: -webkit-box; height: 100px; width: 200px; -webkit-box-orient: vertical;">
+ <div style="overflow: scroll; -webkit-box-flex: 1;" data-expected-width=200 data-expected-height=100></div>
+</div>
+
+<script src=""
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+checkLayout('#flexbox');
+</script>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (130548 => 130549)
--- trunk/Source/WebCore/ChangeLog 2012-10-05 20:47:16 UTC (rev 130548)
+++ trunk/Source/WebCore/ChangeLog 2012-10-05 21:12:18 UTC (rev 130549)
@@ -1,3 +1,37 @@
+2012-10-05 Ojan Vafai <o...@chromium.org>
+
+ Deprecated flexboxes subtract scrollbar width/height twice
+ https://bugs.webkit.org/show_bug.cgi?id=98552
+
+ Reviewed by Tony Chang.
+
+ This is a regression from http://trac.webkit.org/changeset/119507.
+ The problem is that contentHeight subtracts the scrollbar and
+ RenderDeprecatedFlexbox subtracts the scrollbar.
+
+ -Make it so that we only access override sizes if one has been set.
+ I think this makes the calling code more clear.
+ -If we don't have one set, grab the height/width - borderAndPadding.
+ -Add a FIXME to change this all back to being borderbox sizes.
+ There's something trick with making table padding/border work right for that
+ though (noted in the original patch).
+
+ Test: fast/flexbox/flexing-overflow-scroll-item.html
+
+ * rendering/RenderBox.cpp:
+ (WebCore::RenderBox::overrideLogicalContentWidth):
+ (WebCore::RenderBox::overrideLogicalContentHeight):
+ (WebCore::RenderBox::availableLogicalHeightUsing):
+ * rendering/RenderBox.h:
+ (RenderBox):
+ * rendering/RenderDeprecatedFlexibleBox.cpp:
+ (WebCore::contentWidthForChild):
+ (WebCore):
+ (WebCore::contentHeightForChild):
+ (WebCore::RenderDeprecatedFlexibleBox::layoutHorizontalBox):
+ (WebCore::RenderDeprecatedFlexibleBox::layoutVerticalBox):
+ (WebCore::RenderDeprecatedFlexibleBox::allowedChildFlex):
+
2012-10-05 Eric Seidel <e...@webkit.org>
Make tables which don't use col/row span much faster to layout
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (130548 => 130549)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2012-10-05 20:47:16 UTC (rev 130548)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2012-10-05 21:12:18 UTC (rev 130549)
@@ -708,14 +708,14 @@
LayoutUnit RenderBox::overrideLogicalContentWidth() const
{
- // FIXME: This should probably be returning contentLogicalWidth instead of contentWidth.
- return hasOverrideWidth() ? gOverrideWidthMap->get(this) : contentWidth();
+ ASSERT(hasOverrideWidth());
+ return gOverrideWidthMap->get(this);
}
LayoutUnit RenderBox::overrideLogicalContentHeight() const
{
- // FIXME: This should probably be returning contentLogicalHeight instead of contentHeight.
- return hasOverrideHeight() ? gOverrideHeightMap->get(this) : contentHeight();
+ ASSERT(hasOverrideHeight());
+ return gOverrideHeightMap->get(this);
}
LayoutUnit RenderBox::adjustBorderBoxLogicalWidthForBoxSizing(LayoutUnit width) const
@@ -2341,8 +2341,11 @@
// We need to stop here, since we don't want to increase the height of the table
// artificially. We're going to rely on this cell getting expanded to some new
// height, and then when we lay out again we'll use the calculation below.
- if (isTableCell() && (h.isAuto() || h.isPercent()))
- return overrideLogicalContentHeight();
+ if (isTableCell() && (h.isAuto() || h.isPercent())) {
+ if (hasOverrideHeight())
+ return overrideLogicalContentHeight();
+ return logicalHeight() - borderAndPaddingLogicalHeight();
+ }
if (h.isPercent() && isOutOfFlowPositioned()) {
// FIXME: This is wrong if the containingBlock has a perpendicular writing mode.
Modified: trunk/Source/WebCore/rendering/RenderBox.h (130548 => 130549)
--- trunk/Source/WebCore/rendering/RenderBox.h 2012-10-05 20:47:16 UTC (rev 130548)
+++ trunk/Source/WebCore/rendering/RenderBox.h 2012-10-05 21:12:18 UTC (rev 130549)
@@ -289,6 +289,10 @@
virtual LayoutUnit minPreferredLogicalWidth() const;
virtual LayoutUnit maxPreferredLogicalWidth() const;
+ // FIXME: We should rename these back to overrideLogicalHeight/Width and have them store
+ // the border-box height/width like the regular height/width accessors on RenderBox.
+ // Right now, these are different than contentHeight/contentWidth because they still
+ // include the scrollbar height/width.
LayoutUnit overrideLogicalContentWidth() const;
LayoutUnit overrideLogicalContentHeight() const;
bool hasOverrideHeight() const;
Modified: trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp (130548 => 130549)
--- trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp 2012-10-05 20:47:16 UTC (rev 130548)
+++ trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp 2012-10-05 21:12:18 UTC (rev 130549)
@@ -151,6 +151,20 @@
return child->isOutOfFlowPositioned() || child->style()->visibility() == COLLAPSE;
}
+static LayoutUnit contentWidthForChild(RenderBox* child)
+{
+ if (child->hasOverrideWidth())
+ return child->overrideLogicalContentWidth();
+ return child->logicalWidth() - child->borderAndPaddingLogicalWidth();
+}
+
+static LayoutUnit contentHeightForChild(RenderBox* child)
+{
+ if (child->hasOverrideHeight())
+ return child->overrideLogicalContentHeight();
+ return child->logicalHeight() - child->borderAndPaddingLogicalHeight();
+}
+
void RenderDeprecatedFlexibleBox::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
{
RenderStyle* oldStyle = style();
@@ -538,7 +552,7 @@
if (allowedChildFlex(child, expanding, i)) {
LayoutUnit spaceAdd = LayoutUnit(spaceAvailableThisPass * (child->style()->boxFlex() / totalFlex));
if (spaceAdd) {
- child->setOverrideLogicalContentWidth(child->overrideLogicalContentWidth() + spaceAdd);
+ child->setOverrideLogicalContentWidth(contentWidthForChild(child) + spaceAdd);
flexingChildren = true;
relayoutChildren = true;
}
@@ -555,7 +569,7 @@
LayoutUnit spaceAdd = groupRemainingSpace > 0 ? 1 : -1;
for (RenderBox* child = iterator.first(); child && groupRemainingSpace; child = iterator.next()) {
if (allowedChildFlex(child, expanding, i)) {
- child->setOverrideLogicalContentWidth(child->overrideLogicalContentWidth() + spaceAdd);
+ child->setOverrideLogicalContentWidth(contentWidthForChild(child) + spaceAdd);
flexingChildren = true;
relayoutChildren = true;
remainingSpace -= spaceAdd;
@@ -789,7 +803,7 @@
if (allowedChildFlex(child, expanding, i)) {
LayoutUnit spaceAdd = static_cast<LayoutUnit>(spaceAvailableThisPass * (child->style()->boxFlex() / totalFlex));
if (spaceAdd) {
- child->setOverrideLogicalContentHeight(child->overrideLogicalContentHeight() + spaceAdd);
+ child->setOverrideLogicalContentHeight(contentHeightForChild(child) + spaceAdd);
flexingChildren = true;
relayoutChildren = true;
}
@@ -806,7 +820,7 @@
LayoutUnit spaceAdd = groupRemainingSpace > 0 ? 1 : -1;
for (RenderBox* child = iterator.first(); child && groupRemainingSpace; child = iterator.next()) {
if (allowedChildFlex(child, expanding, i)) {
- child->setOverrideLogicalContentHeight(child->overrideLogicalContentHeight() + spaceAdd);
+ child->setOverrideLogicalContentHeight(contentHeightForChild(child) + spaceAdd);
flexingChildren = true;
relayoutChildren = true;
remainingSpace -= spaceAdd;
@@ -1019,7 +1033,7 @@
if (isHorizontal()) {
// FIXME: For now just handle fixed values.
LayoutUnit maxWidth = MAX_LAYOUT_UNIT;
- LayoutUnit width = child->overrideLogicalContentWidth();
+ LayoutUnit width = contentWidthForChild(child);
if (!child->style()->maxWidth().isUndefined() && child->style()->maxWidth().isFixed())
maxWidth = child->style()->maxWidth().value();
else if (child->style()->maxWidth().type() == Intrinsic)
@@ -1032,7 +1046,7 @@
} else {
// FIXME: For now just handle fixed values.
LayoutUnit maxHeight = MAX_LAYOUT_UNIT;
- LayoutUnit height = child->overrideLogicalContentHeight();
+ LayoutUnit height = contentHeightForChild(child);
if (!child->style()->maxHeight().isUndefined() && child->style()->maxHeight().isFixed())
maxHeight = child->style()->maxHeight().value();
if (maxHeight == MAX_LAYOUT_UNIT)
@@ -1044,7 +1058,7 @@
// FIXME: For now just handle fixed values.
if (isHorizontal()) {
LayoutUnit minWidth = child->minPreferredLogicalWidth();
- LayoutUnit width = child->overrideLogicalContentWidth();
+ LayoutUnit width = contentWidthForChild(child);
if (child->style()->minWidth().isFixed())
minWidth = child->style()->minWidth().value();
else if (child->style()->minWidth().type() == Intrinsic)
@@ -1060,7 +1074,7 @@
Length minHeight = child->style()->minHeight();
if (minHeight.isFixed() || minHeight.isAuto()) {
LayoutUnit minHeight = child->style()->minHeight().value();
- LayoutUnit height = child->overrideLogicalContentHeight();
+ LayoutUnit height = contentHeightForChild(child);
LayoutUnit allowedShrinkage = min<LayoutUnit>(0, minHeight - height);
return allowedShrinkage;
}
