Commit 7c13f644 authored by Manuel Rego Casasnovas's avatar Manuel Rego Casasnovas Committed by Commit Bot

Subtract scrollbar in PerpendicularContainingBlockLogicalHeight

This is an issue that happens in Flexbox and Grid Layout
and any other layout method that uses OverrideLogicalHeight.
In LayoutBox::PerpendicularContainingBlockLogicalHeight()
we were returning the OverrideContentLogicalHeight() plus
the scrollbar height.
That caused issue with orthogonal flows between flex/grid items
and their children.

The issue is fixed just following the suggestion in the TODO
and using OverrideContentLogicalHeight()
(which doesn't include the scrollbar height).

BUG=836827
TEST=external/wpt/css/css-flexbox/percentage-size-subitems-001.html
TEST=external/wpt/css/css-grid/grid-items/percentage-size-subitems-001.html

Change-Id: I0c7586449067aab78430defc831b2774136e2f0b
Reviewed-on: https://chromium-review.googlesource.com/1028012
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarJavier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#553790}
parent 04d925bd
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Flexbox Test: Percentage size on child of a flex item with margin, border, padding and scrollbar</title>
<link rel="author" title="Manuel Rego Casasnovas" href="mailto:rego@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#flex-items">
<link rel="match" href="reference/percentage-size-subitems-001-ref.html">
<meta name="assert" content="Checks that flex items children resolve properly their percentage sizes, even when the flex item has margin, border, padding and scrollbar.">
<style>
.flex {
display: inline-flex;
border: solid 5px black;
width: 150px;
height: 100px;
margin: 10px;
vertical-align: top;
}
.item {
flex: 1;
overflow: scroll;
border: solid magenta;
border-width: 12px 9px 6px 3px;
margin: 1px 2px 3px 4px;
padding: 5px 15px 10px 20px;
background: cyan;
}
.subitem {
width: 100%;
height: 100%;
background: yellow;
}
.horizontalTB { writing-mode: horizontal-tb; }
.verticalLR { writing-mode: vertical-lr; }
.verticalRL { writing-mode: vertical-rl; }
</style>
<p>The test passes if in the different examples you see scrollbars but there's no overflow, so you cannot actually scroll.</p>
<div class="flex">
<div class="item horizontalTB">
<div class="subitem"></div>
</div>
</div>
<div class="flex">
<div class="item horizontalTB">
<div class="subitem verticalLR"></div>
</div>
</div>
<div class="flex">
<div class="item horizontalTB">
<div class="subitem verticalRL"></div>
</div>
</div>
<div class="flex">
<div class="item verticalLR">
<div class="subitem"></div>
</div>
</div>
<div class="flex">
<div class="item verticalLR">
<div class="subitem horizontalTB"></div>
</div>
</div>
<div class="flex">
<div class="item verticalLR">
<div class="subitem verticalRL"></div>
</div>
</div>
<div class="flex">
<div class="item verticalRL">
<div class="subitem"></div>
</div>
</div>
<div class="flex">
<div class="item verticalRL">
<div class="subitem horizontalTB"></div>
</div>
</div>
<div class="flex">
<div class="item verticalRL">
<div class="subitem verticalLR"></div>
</div>
</div>
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Refttest Reference: Percentage size on child of a flex item with margin, border, padding and scrollbar</title>
<link rel="author" title="Manuel Rego Casasnovas" href="mailto:rego@igalia.com">
<style>
.flex {
display: inline-block;
border: solid 5px black;
width: 150px;
height: 100px;
margin: 10px;
vertical-align: top;
}
.item {
overflow: scroll;
border: solid magenta;
border-width: 12px 9px 6px 3px;
margin: 1px 2px 3px 4px;
padding: 5px 15px 10px 20px;
background: cyan;
width: calc(100% - 6px);
height: calc(100% - 4px);
box-sizing: border-box;
}
.subitem {
width: 100%;
height: 100%;
background: yellow;
}
.horizontalTB { writing-mode: horizontal-tb; }
.verticalLR { writing-mode: vertical-lr; }
.verticalRL { writing-mode: vertical-rl; }
</style>
<p>The test passes if in the different examples you see scrollbars but there's no overflow, so you cannot actually scroll.</p>
<div class="flex">
<div class="item horizontalTB">
<div class="subitem"></div>
</div>
</div>
<div class="flex">
<div class="item horizontalTB">
<div class="subitem verticalLR"></div>
</div>
</div>
<div class="flex">
<div class="item horizontalTB">
<div class="subitem verticalRL"></div>
</div>
</div>
<div class="flex">
<div class="item verticalLR">
<div class="subitem"></div>
</div>
</div>
<div class="flex">
<div class="item verticalLR">
<div class="subitem horizontalTB"></div>
</div>
</div>
<div class="flex">
<div class="item verticalLR">
<div class="subitem verticalRL"></div>
</div>
</div>
<div class="flex">
<div class="item verticalRL">
<div class="subitem"></div>
</div>
</div>
<div class="flex">
<div class="item verticalRL">
<div class="subitem horizontalTB"></div>
</div>
</div>
<div class="flex">
<div class="item verticalRL">
<div class="subitem verticalLR"></div>
</div>
</div>
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Refttest Reference: Percentage size on child of a grid item with margin, border, padding and scrollbar</title>
<link rel="author" title="Manuel Rego Casasnovas" href="mailto:rego@igalia.com">
<style>
.grid {
display: inline-block;
border: solid 5px black;
width: 150px;
height: 100px;
margin: 10px;
vertical-align: top;
}
.item {
overflow: scroll;
border: solid magenta;
border-width: 12px 9px 6px 3px;
margin: 1px 2px 3px 4px;
padding: 5px 15px 10px 20px;
background: cyan;
width: calc(100% - 6px);
height: calc(100% - 4px);
box-sizing: border-box;
}
.subitem {
width: 100%;
height: 100%;
background: yellow;
font: 20px/1 Ahem;
}
.horizontalTB { writing-mode: horizontal-tb; }
.verticalLR { writing-mode: vertical-lr; }
.verticalRL { writing-mode: vertical-rl; }
</style>
<p>The test passes if in the different examples you see scrollbars but there's no overflow, so you cannot actually scroll.</p>
<div class="grid">
<div class="item horizontalTB">
<div class="subitem"></div>
</div>
</div>
<div class="grid">
<div class="item horizontalTB">
<div class="subitem verticalLR"></div>
</div>
</div>
<div class="grid">
<div class="item horizontalTB">
<div class="subitem verticalRL"></div>
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<div class="subitem"></div>
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<div class="subitem horizontalTB"></div>
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<div class="subitem verticalRL"></div>
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<div class="subitem"></div>
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<div class="subitem horizontalTB"></div>
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<div class="subitem verticalLR"></div>
</div>
</div>
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Test: Percentage size on child of a grid item with margin, border, padding and scrollbar</title>
<link rel="author" title="Manuel Rego Casasnovas" href="mailto:rego@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-grid-1/#grid-items">
<link rel="match" href="percentage-size-subitems-001-ref.html">
<meta name="assert" content="Checks that grid items children resolve properly their percentage sizes, even when the grid item has margin, border, padding and scrollbar.">
<style>
.grid {
display: inline-grid;
border: solid 5px black;
grid: 100px / 150px;
margin: 10px;
vertical-align: top;
}
.item {
overflow: scroll;
border: solid magenta;
border-width: 12px 9px 6px 3px;
margin: 1px 2px 3px 4px;
padding: 5px 15px 10px 20px;
background: cyan;
}
.subitem {
width: 100%;
height: 100%;
background: yellow;
}
.horizontalTB { writing-mode: horizontal-tb; }
.verticalLR { writing-mode: vertical-lr; }
.verticalRL { writing-mode: vertical-rl; }
</style>
<p>The test passes if in the different examples you see scrollbars but there's no overflow, so you cannot actually scroll.</p>
<div class="grid">
<div class="item horizontalTB">
<div class="subitem"></div>
</div>
</div>
<div class="grid">
<div class="item horizontalTB">
<div class="subitem verticalLR"></div>
</div>
</div>
<div class="grid">
<div class="item horizontalTB">
<div class="subitem verticalRL"></div>
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<div class="subitem"></div>
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<div class="subitem horizontalTB"></div>
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<div class="subitem verticalRL"></div>
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<div class="subitem"></div>
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<div class="subitem horizontalTB"></div>
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<div class="subitem verticalLR"></div>
</div>
</div>
......@@ -2127,11 +2127,8 @@ LayoutUnit LayoutBox::PerpendicularContainingBlockLogicalHeight() const {
return OverrideContainingBlockContentLogicalHeight();
LayoutBlock* cb = ContainingBlock();
if (cb->HasOverrideLogicalHeight()) {
// TODO(rego): Shouldn't we use OverrideContentLogicalHeight() directly, so
// scrollbar size gets subtracted?
return cb->OverrideContentAndScrollbarLogicalHeight();
}
if (cb->HasOverrideLogicalHeight())
return cb->OverrideContentLogicalHeight();
const ComputedStyle& containing_block_style = cb->StyleRef();
Length logical_height_length = containing_block_style.LogicalHeight();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment