Skip to content

GitLab

T
tangled
Commit 0b5eb052 authored by Javier Fernandez's avatar Javier Fernandez Committed by Commit Bot
Browse files
Options

[css-grid] Distribution offset doesn't account for non-spanning items 

We recenttly changed the track sizing algorithm so that it considers the
offsets added by the Content Alignment properties. The change in r566412
introduce intermediate steps in the track sizing algorithm so that
inline-axis Content Distribution accounts for the row tracks sizes
computed in the next step.

However, we were adding the Content Distribution offsets for any column
track, but we should do it only for the tracks with spanning items. This
error led to the mentioned bugs and this change fixes them.

Bug: 871230, 871242
Change-Id: I8ac789ccacc2e5a51bfafd7820ed08e96a31e58b
Reviewed-on: https://chromium-review.googlesource.com/1176803Reviewed-by: default avatarSergio Villar <svillar@igalia.com>
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#583802}
parent 951f0130
Hide whitespace changes
Inline Side-by-side
Showing with 83 additions and 19 deletions
third_party/WebKit/LayoutTests/external/wpt/css/css-grid/alignment/grid-content-alignment-second-pass-002-expected.txt deleted 100644 → 0
View file @ 951f0130
This is a testharness.js-based test.
PASS .grid 1
PASS .grid 2
PASS .grid 3
PASS .grid 4
PASS .grid 5
PASS .grid 6
PASS .grid 7
FAIL .grid 8 assert_equals:
<div class="grid contentSpaceAround" style="grid-template-rows: 25%;" data-expected-width="100" data-expected-height="80">
<div class="firstRowFirstColumn" data-offset-x="15" data-offset-y="5" data-expected-width="70" data-expected-height="20">XXX X</div>
<div class="secondRowFirstColumn" data-offset-x="15" data-offset-y="35" data-expected-width="70" data-expected-height="40">X<br>X</div>
</div>
height expected 80 but got 60
Harness: the test ran to completion.
third_party/WebKit/LayoutTests/external/wpt/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003.html 0 → 100644
View file @ 0b5eb052
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Content Distribution and the track sizing algorithm</title>
<link rel="author" title="Javier Fernandez Garcia-Boente" href="mailto:jfernandez@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-grid/#grid-align">
<link rel="help" href="https://drafts.csswg.org/css-align/#content-distribution">
<link rel="help" href="https://drafts.csswg.org/css-align/#valdef-align-content-space-around">
<link rel="stylesheet" href="../../support/alignment.css">
<meta name="flags" content="ahem">
<meta name="assert" content="Content Distribution offset doesn't account for tracks with non-spanning items.">
<style>
.grid {
display: inline-grid;
background: grey;
grid-template-columns: 100px;
font: 20px/1 Ahem;
width: 200px;
}
.item1 {
background: green;
grid-column: 1;
grid-row: 1;
}
.item2 {
background: blue;
grid-column: 1;
grid-row: 2;
}
</style>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<body onload="checkLayout('.grid')">
<div class="grid justifyContentSpaceAround" data-expected-width="200" data-expected-height="60">
<div class="item1" data-expected-width="100" data-expected-height="40">XXXX XXX</div>
<div class="item2" data-expected-width="100" data-expected-height="20">XXX</div>
</div>
</body>
third_party/WebKit/LayoutTests/external/wpt/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004.html 0 → 100644
View file @ 0b5eb052
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Content Distribution and the track sizing algorithm</title>
<link rel="author" title="Javier Fernandez Garcia-Boente" href="mailto:jfernandez@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-grid/#grid-align">
<link rel="help" href="https://drafts.csswg.org/css-align/#content-distribution">
<link rel="help" href="https://drafts.csswg.org/css-align/#valdef-align-content-space-between">
<link rel="stylesheet" href="../../support/alignment.css">
<meta name="flags" content="ahem">
<meta name="assert" content="Content Distribution offset doesn't account for relative sized tracks with non-spanning items.">
<style>
.grid {
display: inline-grid;
background: grey;
grid-template-columns: 50%;
font: 20px/1 Ahem;
}
.item1 {
background: green;
grid-column: 1;
grid-row: 1;
}
.item2 {
background: blue;
grid-column: 2;
grid-row: 1;
}
</style>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<body onload="checkLayout('.grid')">
<div class="grid justifyContentSpaceBetween" data-expected-width="220" data-expected-height="40">
<div class="item1" data-expected-width="110" data-expected-height="40">XXXX XXX</div>
<div class="item2" data-expected-width="60" data-expected-height="40">XXX</div>
</div>
</body>
third_party/blink/renderer/core/layout/grid_track_sizing_algorithm.cc
View file @ 0b5eb052
...@@ -227,10 +227,12 @@ LayoutUnit GridTrackSizingAlgorithm::GridAreaBreadthForChild( ...@@ -227,10 +227,12 @@ LayoutUnit GridTrackSizingAlgorithm::GridAreaBreadthForChild(
const Vector<GridTrack>& all_tracks = Tracks(direction); const Vector<GridTrack>& all_tracks = Tracks(direction);
const GridSpan& span = grid_.GridItemSpan(child, direction); const GridSpan& span = grid_.GridItemSpan(child, direction);
LayoutUnit grid_area_breadth; LayoutUnit grid_area_breadth;
for (const auto& track_position : span) { for (const auto& track_position : span)
grid_area_breadth += all_tracks[track_position].BaseSize(); grid_area_breadth += all_tracks[track_position].BaseSize();
if (add_content_alignment_offset)
grid_area_breadth += layout_grid_->GridItemOffset(direction); if (add_content_alignment_offset) {
grid_area_breadth +=
(span.IntegerSpan() - 1) * layout_grid_->GridItemOffset(direction);
} }
grid_area_breadth += grid_area_breadth +=
......
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