Skip to content

GitLab

T
tangled
Commit 7dea9b2c authored by Manuel Rego Casasnovas's avatar Manuel Rego Casasnovas Committed by Commit Bot
Browse files
Options

[css-grid] Subtract scrollbar in ComputeReplacedLogicalHeightUsing() 

In LayoutBox::ComputeReplacedLogicalHeightUsing() we were using
the OverrideContentLogicalHeight() plus scrollbar height,
that was wrong and we should subtract the scrollbar too.
This caused issues to resolve the percentage heights on grid item
replaced children, if the grid item has a scrollbar.

To fix the issue we just need to follow the suggestion in the TODO
and use OverrideContentLogicalHeight() directly.

This was the last place using OverrideContentAndScrollbarLogicalHeight()
so we can get rid of it. The patch removes that method together with
OverrideContentAndScrollbarLogicalWidth() that was no longer used.

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

Change-Id: I28cf6e65c21e6314808c4430515f06c07d4a739e
Reviewed-on: https://chromium-review.googlesource.com/1035003Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#554834}
parent 9ea5bb72
Hide whitespace changes
Inline Side-by-side
Showing with 192 additions and 27 deletions
third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/percentage-size-replaced-subitems-001-ref.html 0 → 100644
View file @ 7dea9b2c
<!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;
}
img {
width: 100%;
height: 100%;
display: block;
}
.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">
<img src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item horizontalTB">
<img class="verticalLR" src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item horizontalTB">
<img class="verticalRL" src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<img src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<img class="horizontalTB" src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<img class="verticalRL" src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<img src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<img class="horizontalTB" src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<img class="verticalRL" src="support/100x100-green.png" />
</div>
</div>
third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/percentage-size-replaced-subitems-001.html 0 → 100644
View file @ 7dea9b2c
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Test: Percentage size on replaced 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-replaced-subitems-001-ref.html">
<meta name="assert" content="Checks that grid items replaced 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;
}
img {
width: 100%;
height: 100%;
display: block;
}
.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">
<img src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item horizontalTB">
<img class="verticalLR" src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item horizontalTB">
<img class="verticalRL" src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<img src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<img class="horizontalTB" src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalLR">
<img class="verticalRL" src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<img src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<img class="horizontalTB" src="support/100x100-green.png" />
</div>
</div>
<div class="grid">
<div class="item verticalRL">
<img class="verticalRL" src="support/100x100-green.png" />
</div>
</div>
third_party/blink/renderer/core/layout/layout_box.cc
View file @ 7dea9b2c
...@@ -1431,20 +1431,6 @@ LayoutUnit LayoutBox::OverrideContentLogicalHeight() const { ...@@ -1431,20 +1431,6 @@ LayoutUnit LayoutBox::OverrideContentLogicalHeight() const {
.ClampNegativeToZero(); .ClampNegativeToZero();
} }
// TODO(rego): Probably at some point we could remove this if all calls to
// override sizes consider the scrollbar size properly.
LayoutUnit LayoutBox::OverrideContentAndScrollbarLogicalWidth() const {
return (OverrideLogicalWidth() - BorderAndPaddingLogicalWidth())
.ClampNegativeToZero();
}
// TODO(rego): Probably at some point we could remove this if all calls to
// override sizes consider the scrollbar size properly.
LayoutUnit LayoutBox::OverrideContentAndScrollbarLogicalHeight() const {
return (OverrideLogicalHeight() - BorderAndPaddingLogicalHeight())
.ClampNegativeToZero();
}
// TODO (lajava) Shouldn't we implement these functions based on physical // TODO (lajava) Shouldn't we implement these functions based on physical
// direction ?. // direction ?.
LayoutUnit LayoutBox::OverrideContainingBlockContentLogicalWidth() const { LayoutUnit LayoutBox::OverrideContainingBlockContentLogicalWidth() const {
...@@ -3640,6 +3626,8 @@ LayoutUnit LayoutBox::ComputeReplacedLogicalHeightUsing( ...@@ -3640,6 +3626,8 @@ LayoutUnit LayoutBox::ComputeReplacedLogicalHeightUsing(
IsOutOfFlowPositioned() ? Container() : ContainingBlock(); IsOutOfFlowPositioned() ? Container() : ContainingBlock();
while (cb->IsAnonymous()) while (cb->IsAnonymous())
cb = cb->ContainingBlock(); cb = cb->ContainingBlock();
bool has_perpendicular_containing_block =
cb->IsHorizontalWritingMode() != IsHorizontalWritingMode();
LayoutUnit stretched_height(-1); LayoutUnit stretched_height(-1);
if (cb->IsLayoutBlock()) { if (cb->IsLayoutBlock()) {
LayoutBlock* block = ToLayoutBlock(cb); LayoutBlock* block = ToLayoutBlock(cb);
...@@ -3648,10 +3636,9 @@ LayoutUnit LayoutBox::ComputeReplacedLogicalHeightUsing( ...@@ -3648,10 +3636,9 @@ LayoutUnit LayoutBox::ComputeReplacedLogicalHeightUsing(
stretched_height = stretched_height =
ToLayoutFlexibleBox(block->Parent()) ToLayoutFlexibleBox(block->Parent())
->ChildLogicalHeightForPercentageResolution(*block); ->ChildLogicalHeightForPercentageResolution(*block);
} else if (block->IsGridItem() && block->HasOverrideLogicalHeight()) { } else if (block->IsGridItem() && block->HasOverrideLogicalHeight() &&
// TODO(rego): Shouldn't we use OverrideContentLogicalHeight() !has_perpendicular_containing_block) {
// directly, so scrollbar size gets subtracted? stretched_height = block->OverrideContentLogicalHeight();
stretched_height = block->OverrideContentAndScrollbarLogicalHeight();
} }
} }
...@@ -3676,11 +3663,10 @@ LayoutUnit LayoutBox::ComputeReplacedLogicalHeightUsing( ...@@ -3676,11 +3663,10 @@ LayoutUnit LayoutBox::ComputeReplacedLogicalHeightUsing(
} else if (stretched_height != -1) { } else if (stretched_height != -1) {
available_height = stretched_height; available_height = stretched_height;
} else { } else {
available_height = available_height = has_perpendicular_containing_block
IsHorizontalWritingMode() == cb->IsHorizontalWritingMode() ? ContainingBlockLogicalWidthForContent()
? ContainingBlockLogicalHeightForContent( : ContainingBlockLogicalHeightForContent(
kIncludeMarginBorderPadding) kIncludeMarginBorderPadding);
: ContainingBlockLogicalWidthForContent();
// It is necessary to use the border-box to match WinIE's broken // It is necessary to use the border-box to match WinIE's broken
// box model. This is essential for sizing inside // box model. This is essential for sizing inside
// table cells using percentage heights. // table cells using percentage heights.
......
third_party/blink/renderer/core/layout/layout_box.h
View file @ 7dea9b2c
...@@ -701,10 +701,6 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject { ...@@ -701,10 +701,6 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject {
LayoutUnit OverrideContentLogicalWidth() const; LayoutUnit OverrideContentLogicalWidth() const;
LayoutUnit OverrideContentLogicalHeight() const; LayoutUnit OverrideContentLogicalHeight() const;
// TODO(rego): Probably at some point the next 2 methods can be removed if all
// the calls to override sizes consider the scrollbar size properly.
LayoutUnit OverrideContentAndScrollbarLogicalWidth() const;
LayoutUnit OverrideContentAndScrollbarLogicalHeight() const;
LayoutUnit OverrideContainingBlockContentLogicalWidth() const; LayoutUnit OverrideContainingBlockContentLogicalWidth() const;
LayoutUnit OverrideContainingBlockContentLogicalHeight() const; LayoutUnit OverrideContainingBlockContentLogicalHeight() const;
......
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