Commit 35d044cf authored by Manuel Rego Casasnovas's avatar Manuel Rego Casasnovas Committed by Commit Bot

[css-grid] Add support for calc() in gutter properties

There was a crash in debug if you use calc()
mixing fixed and percentage values due to the wrong DCHECK
in GapLength constructor. The patch fixes this assert.

In addition LayoutGrid::GridGap() was also wrong and didn't consider
calc() either. The fix is again easy just using the proper check.

Regarding testing, the parsing tests have been updated to include
this combination of fixed and percentage values in calc().
At the same time, the patch actually uses "grid-" prefixed properties
in the tests that were supposed to test those.
Last, two new tests are added to verify the proper behavior of calc()
with mixed values on a grid layout container.

BUG=816300
TEST=external/wpt/css/css-align/gaps/column-gap-parsing-001.html
TEST=external/wpt/css/css-align/gaps/gap-parsing-001.html
TEST=external/wpt/css/css-align/gaps/grid-column-gap-parsing-001.html
TEST=external/wpt/css/css-align/gaps/grid-gap-parsing-001.html
TEST=external/wpt/css/css-align/gaps/grid-row-gap-parsing-001.html
TEST=external/wpt/css/css-align/gaps/row-gap-parsing-001.html
TEST=external/wpt/css/css-grid/alignment/grid-gutters-011.html
TEST=external/wpt/css/css-grid/alignment/grid-gutters-012.html

Change-Id: I4c9fe6b2525a253c6bb00cbda727c2bf1ae6e90b
Reviewed-on: https://chromium-review.googlesource.com/962148
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Reviewed-by: default avatarSergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#543079}
parent 83ef62e3
......@@ -11,6 +11,7 @@
#columnGapVw { column-gap: 2vw; }
#columnGapPercent { column-gap: 15%; }
#columnGapCalc { column-gap: calc(10px + 4px); }
#columnGapCalcFixedPercent { column-gap: calc(5px + 10%); }
.columnGapInitial { column-gap: initial; }
.columnGapInherit { column-gap: inherit; }
......@@ -31,6 +32,7 @@
<div id="columnGapVw"></div>
<div id="columnGapPercent"></div>
<div id="columnGapCalc"></div>
<div id="columnGapCalcFixedPercent"></div>
<div id="columnGapInitial" class="columnGapInitial"></div>
<div class="columnGapPx">
<div id="columnGapInitialPx" class="columnGapInitial"></div>
......@@ -81,6 +83,11 @@
var target = document.getElementById("columnGapCalc");
assert_equals(getComputedStyle(target).columnGap, "14px");
}, "column-gap accepts calc()");
test(
function(){
var target = document.getElementById("columnGapCalcFixedPercent");
assert_equals(getComputedStyle(target).columnGap, "calc(5px + 10%)");
}, "column-gap accepts calc() mixing fixed and percentage values");
test(
function(){
var target = document.getElementById("columnGapInitial");
......
......@@ -16,6 +16,7 @@
#gapPercent { gap: 15%; }
#gapPercentTwo { gap: 15% 10%; }
#gapCalc { gap: calc(10px + 4px); }
#gapCalcFixedPercent { gap: calc(5px + 10%); }
#gapCalcTwo { gap: calc(10px + 4px) calc(20px - 8px); }
.gapInitial { gap: initial; }
.gapInherit { gap: inherit; }
......@@ -44,6 +45,7 @@
<div id="gapPercent"></div>
<div id="gapPercentTwo"></div>
<div id="gapCalc"></div>
<div id="gapCalcFixedPercent"></div>
<div id="gapCalcTwo"></div>
<div id="gapInitial" class="gapInitial"></div>
<div class="gapPx">
......@@ -135,6 +137,12 @@
assert_equals(getComputedStyle(target).rowGap, "14px");
assert_equals(getComputedStyle(target).columnGap, "14px");
}, "gap accepts calc()");
test(
function(){
var target = document.getElementById("gapCalcFixedPercent");
assert_equals(getComputedStyle(target).rowGap, "calc(5px + 10%)");
assert_equals(getComputedStyle(target).columnGap, "calc(5px + 10%)");
}, "gap accepts calc() mixing fixed and percentage values");
test(
function(){
var target = document.getElementById("gapCalcTwo");
......
......@@ -11,6 +11,7 @@
#columnGapVw { column-gap: 2vw; }
#columnGapPercent { column-gap: 15%; }
#columnGapCalc { column-gap: calc(10px + 4px); }
#columnGapCalcFixedPercent { column-gap: calc(5px + 10%); }
.columnGapInitial { column-gap: initial; }
.columnGapInherit { column-gap: inherit; }
......@@ -31,6 +32,7 @@
<div id="columnGapVw"></div>
<div id="columnGapPercent"></div>
<div id="columnGapCalc"></div>
<div id="columnGapCalcFixedPercent"></div>
<div id="columnGapInitial" class="columnGapInitial"></div>
<div class="columnGapPx">
<div id="columnGapInitialPx" class="columnGapInitial"></div>
......@@ -81,6 +83,11 @@
var target = document.getElementById("columnGapCalc");
assert_equals(getComputedStyle(target).columnGap, "14px");
}, "column-gap accepts calc()");
test(
function(){
var target = document.getElementById("columnGapCalcFixedPercent");
assert_equals(getComputedStyle(target).columnGap, "calc(5px + 10%)");
}, "column-gap accepts calc() mixing fixed and percentage values");
test(
function(){
var target = document.getElementById("columnGapInitial");
......
......@@ -16,6 +16,7 @@
#gapPercent { grid-gap: 15%; }
#gapPercentTwo { grid-gap: 15% 10%; }
#gapCalc { grid-gap: calc(10px + 4px); }
#gapCalcFixedPercent { grid-gap: calc(5px + 10%); }
#gapCalcTwo { grid-gap: calc(10px + 4px) calc(20px - 8px); }
.gapInitial { grid-gap: initial; }
.gapInherit { grid-gap: inherit; }
......@@ -44,6 +45,7 @@
<div id="gapPercent"></div>
<div id="gapPercentTwo"></div>
<div id="gapCalc"></div>
<div id="gapCalcFixedPercent"></div>
<div id="gapCalcTwo"></div>
<div id="gapInitial" class="gapInitial"></div>
<div class="gapPx">
......@@ -135,6 +137,12 @@
assert_equals(getComputedStyle(target).rowGap, "14px");
assert_equals(getComputedStyle(target).columnGap, "14px");
}, "gap accepts calc()");
test(
function(){
var target = document.getElementById("gapCalcFixedPercent");
assert_equals(getComputedStyle(target).rowGap, "calc(5px + 10%)");
assert_equals(getComputedStyle(target).columnGap, "calc(5px + 10%)");
}, "gap accepts calc() mixing fixed and percentage values");
test(
function(){
var target = document.getElementById("gapCalcTwo");
......
......@@ -11,6 +11,7 @@
#rowGapVw { row-gap: 2vw; }
#rowGapPercent { row-gap: 15%; }
#rowGapCalc { row-gap: calc(10px + 4px); }
#rowGapCalcFixedPercent { row-gap: calc(5px + 10%); }
.rowGapInitial { row-gap: initial; }
.rowGapInherit { row-gap: inherit; }
......@@ -31,6 +32,7 @@
<div id="rowGapVw"></div>
<div id="rowGapPercent"></div>
<div id="rowGapCalc"></div>
<div id="rowGapCalcFixedPercent"></div>
<div id="rowGapInitial" class="rowGapInitial"></div>
<div class="rowGapPx">
<div id="rowGapInitialPx" class="rowGapInitial"></div>
......@@ -81,6 +83,11 @@
var target = document.getElementById("rowGapCalc");
assert_equals(getComputedStyle(target).rowGap, "14px");
}, "row-gap accepts calc()");
test(
function(){
var target = document.getElementById("rowGapCalcFixedPercent");
assert_equals(getComputedStyle(target).rowGap, "calc(5px + 10%)");
}, "row-gap accepts calc() mixing fixed and percentage values");
test(
function(){
var target = document.getElementById("rowGapInitial");
......
......@@ -11,6 +11,7 @@
#rowGapVw { row-gap: 2vw; }
#rowGapPercent { row-gap: 15%; }
#rowGapCalc { row-gap: calc(10px + 4px); }
#rowGapCalcFixedPercent { row-gap: calc(5px + 10%); }
.rowGapInitial { row-gap: initial; }
.rowGapInherit { row-gap: inherit; }
......@@ -31,6 +32,7 @@
<div id="rowGapVw"></div>
<div id="rowGapPercent"></div>
<div id="rowGapCalc"></div>
<div id="rowGapCalcFixedPercent"></div>
<div id="rowGapInitial" class="rowGapInitial"></div>
<div class="rowGapPx">
<div id="rowGapInitialPx" class="rowGapInitial"></div>
......@@ -81,6 +83,11 @@
var target = document.getElementById("rowGapCalc");
assert_equals(getComputedStyle(target).rowGap, "14px");
}, "row-gap accepts calc()");
test(
function(){
var target = document.getElementById("rowGapCalcFixedPercent");
assert_equals(getComputedStyle(target).rowGap, "calc(5px + 10%)");
}, "row-gap accepts calc() mixing fixed and percentage values");
test(
function(){
var target = document.getElementById("rowGapInitial");
......
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Support for calc mixing fixed and percentage values for gap</title>
<link rel="help" href="https://www.w3.org/TR/css-grid-1/#gutters">
<link rel="help" href="https://www.w3.org/TR/css-align-3/#gap-shorthand">
<link rel="match" href="../reference/grid-different-gutters-ref.html">
<link rel="author" title="Manuel Rego Casasnovas" href="mailto:rego@igalia.com">
<style>
#grid {
display: grid;
width: 200px;
height: 220px;
gap: calc(15% + 7px) calc(10px + 5%);
grid-template-columns: 90px 90px;
grid-template-rows: 90px 90px;
background-color: green;
}
#grid > div {
background-color: silver;
}
</style>
<p>The test passes if it has the same visual effect as reference.</p>
<div id="grid">
<div></div>
<div></div>
<div></div>
<div></div>
</div>
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Support for calc mixing fixed and percentage values for grid-gap as alias for gap</title>
<link rel="help" href="https://www.w3.org/TR/css-grid-1/#gutters">
<link rel="help" href="https://www.w3.org/TR/css-align-3/#gap-shorthand">
<link rel="help" href0"https://www.w3.org/TR/css-align-3/#gap-legacy">
<link rel="match" href="../reference/grid-different-gutters-ref.html">
<link rel="author" title="Manuel Rego Casasnovas" href="mailto:rego@igalia.com">
<style>
#grid {
display: grid;
width: 200px;
height: 220px;
grid-gap: calc(15% + 7px) calc(10px + 5%);
grid-template-columns: 90px 90px;
grid-template-rows: 90px 90px;
background-color: green;
}
#grid > div {
background-color: silver;
}
</style>
<p>The test passes if it has the same visual effect as reference.</p>
<div id="grid">
<div></div>
<div></div>
<div></div>
<div></div>
</div>
......@@ -423,7 +423,7 @@ LayoutUnit LayoutGrid::GridGap(GridTrackSizingDirection direction) const {
if (gap.IsNormal())
return LayoutUnit();
if (gap.GetLength().IsPercent())
if (gap.GetLength().IsPercentOrCalc())
available_size = is_row_axis
? AvailableLogicalWidth()
: AvailableLogicalHeightForPercentageComputation();
......
......@@ -16,7 +16,7 @@ class GapLength {
public:
GapLength() : is_normal_(true) {}
GapLength(const Length& length) : is_normal_(false), length_(length) {
DCHECK(length.IsFixed() || length.IsPercent());
DCHECK(length.IsSpecified());
}
bool IsNormal() const { return is_normal_; }
......
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