Skip to content

Commit

Permalink
[css-typed-om] Whitelist supported properties.
Browse files Browse the repository at this point in the history
This patch whitelists all our supported properties. Any property that
is not supported will return a base CSSStyleValue. This ensures that
we don't accidentally produce unexpected results for properties that
we haven't tested it.

We also add a few new properties to support, bringing the total number
of property tests to 50. We also remove support for 'z-index' because
it uses <integers> which we don't support yet.

transition-duration is failing tests because we currently convert
ms to seconds when we compute, which is incorrect.

Some paint/layout tests depend on properties that are not whitelisted,
so we've changed them:
- border-radius -> margin-left
- align-items -> empty-cell

Bug: 816402
Change-Id: I84d2fd8a15df92624122f0c1ecf4f7c42f928928
Reviewed-on: https://chromium-review.googlesource.com/942651
Reviewed-by: nainar <nainar@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540397}
  • Loading branch information
darrnshn authored and Commit Bot committed Mar 2, 2018
1 parent 97c6f1f commit 1ccb08d
Show file tree
Hide file tree
Showing 23 changed files with 264 additions and 126 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
<style>
.result {
background: green;
border-radius: 2px;
margin: 10px;
margin-left: 2px;

width: 100px;
height: 100px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

/* Properties under test. */
--foo: bar;
border-radius: 2px;
margin-left: 2px;
}

@supports (display: layout(test)) {
Expand All @@ -30,16 +30,16 @@
<script id="code" type="text/worklet">
registerLayout('test', class {
static get inputProperties() {
return [ '--bar', '--foo', 'align-items', 'border-top-left-radius'];
return [ '--bar', '--foo', 'empty-cells', 'margin-left'];
}

*intrinsicSizes() {}
*layout(children, edges, constraints, styleMap) {
const expected = [
{property: '--bar', value: '[CSSUnparsedValue=]'},
{property: '--foo', value: '[CSSUnparsedValue= bar]'},
{property: 'align-items', value: '[CSSKeywordValue=normal]'},
{property: 'border-top-left-radius', value: '[CSSUnitValue=2px]'},
{property: 'empty-cells', value: '[CSSKeywordValue=show]'},
{property: 'margin-left', value: '[CSSUnitValue=2px]'},
];

const actual = Array.from(styleMap.keys()).sort().map((property) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
<style>
.result {
background: green;
border-radius: 2px;
margin: 10px;
margin-left: 2px;

width: 100px;
height: 100px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

/* Properties under test. */
--foo: bar;
border-radius: 2px;
margin-left: 2px;
}

@supports (display: layout(test)) {
Expand Down Expand Up @@ -60,8 +60,8 @@
const tests = [
{property: '--bar', expected: '[CSSUnparsedValue=]'},
{property: '--foo', expected: '[CSSUnparsedValue= bar]'},
{property: 'align-items', expected: '[CSSKeywordValue=normal]'},
{property: 'border-top-left-radius', expected: '[CSSUnitValue=2px]'},
{property: 'empty-cells', expected: '[CSSKeywordValue=show]'},
{property: 'margin-left', expected: '[CSSUnitValue=2px]'},
];

const workletSource = tests.map(tmpl).join('\n');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<!DOCTYPE html>
<html>
<body>
<canvas id ="canvas" width="100" height="100" style="border-radius: 2px"></canvas>
<canvas id ="canvas" width="100" height="100" style="margin-left: 2px"></canvas>
<script>
var canvas = document.getElementById('canvas');
var context = canvas.getContext("2d");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
.container {
width: 100px;
height: 100px;
border-radius: 2px;
margin-left: 2px;
--foo: bar;
}

Expand All @@ -24,8 +24,8 @@
return [
'--bar',
'--foo',
'align-items',
'border-top-left-radius',
'empty-cells',
'margin-left',
];
}
paint(ctx, geom, styleMap) {
Expand All @@ -45,9 +45,9 @@
ctx.strokeStyle = 'red';
if (serializedStrings[1] != "--foo: [CSSUnparsedValue= bar]")
ctx.strokeStyle = 'blue';
if (serializedStrings[2] != "align-items: [CSSKeywordValue=normal]")
if (serializedStrings[2] != "empty-cells: [CSSKeywordValue=show]")
ctx.strokeStyle = 'yellow';
if (serializedStrings[3] != "border-top-left-radius: [CSSUnitValue=2px]")
if (serializedStrings[3] != "margin-left: [CSSUnitValue=2px]")
ctx.strokeStyle = 'cyan';
ctx.lineWidth = 4;
ctx.strokeRect(0, 0, geom.width, geom.height);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
<html>
<style>
div {
border-radius: 3px;
margin-left: 3px;
}
div::before {
width: 100px;
height: 100px;
border-radius: 2px;
margin-left: 2px;
content: 'foo';
color: rgba(0, 0, 0, 0);
}
canvas{
border-radius: 2px;
margin-left: 2px;
display: block;
position: relative;
top: -1em;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<link rel="match" href="style-before-pseudo-ref.html">
<style>
div {
border-radius: 3px;
margin-left: 3px;
}

div::before {
Expand All @@ -13,7 +13,7 @@
color: rgba(0, 0, 0, 0);

background-image: paint(geometry);
border-radius: 2px;
margin-left: 2px;
--foo: bar;
}
</style>
Expand All @@ -28,7 +28,7 @@
return [
'--bar',
'--foo',
'border-top-left-radius',
'margin-left',
];
}
paint(ctx, geom, styleMap) {
Expand All @@ -48,7 +48,7 @@
ctx.strokeStyle = 'red';
if (serializedStrings[1] != "--foo: [CSSUnparsedValue= bar]")
ctx.strokeStyle = 'blue';
if (serializedStrings[2] != "border-top-left-radius: [CSSUnitValue=2px]")
if (serializedStrings[2] != "margin-left: [CSSUnitValue=2px]")
ctx.strokeStyle = 'yellow';
ctx.lineWidth = 4;
ctx.strokeRect(0, 0, geom.width, geom.height);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
}, 'Normalizing a <dimension> returns a CSSUnitValue with the correct unit');

test(t => {
test_numeric_normalization(t, 'z-index', '0', CSS.number(0));
test_numeric_normalization(t, 'opacity', '0', CSS.number(0));
}, 'Normalizing a <number> with a unitless zero returns 0');

test(t => {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<!doctype html>
<meta charset="utf-8">
<title>'z-index' property</title>
<title>'backface-visibility' property</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
Expand All @@ -13,11 +13,9 @@
<script>
'use strict';

runPropertyTests('z-index', [
{ syntax: 'auto'},
// FIXME: This also supports <integer> but the testharness
// doesn't support that yet.
// { syntax: '<integer>' }
runPropertyTests('backface-visibility', [
{ syntax: 'visible' },
{ syntax: 'hidden' },
]);

</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!doctype html>
<meta charset="utf-8">
<title>'border-collapse' property</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<script src="resources/testsuite.js"></script>
<body>
<div id="log"></div>
<script>
'use strict';

runPropertyTests('border-collapse', [
{ syntax: 'separate' },
{ syntax: 'collapse' },
]);

</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!doctype html>
<meta charset="utf-8">
<title>'direction' property</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<script src="resources/testsuite.js"></script>
<body>
<div id="log"></div>
<script>
'use strict';

runPropertyTests('direction', [
{ syntax: 'ltr' },
{ syntax: 'rtl' },
]);

</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!doctype html>
<meta charset="utf-8">
<title>'empty-cells' property</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<script src="resources/testsuite.js"></script>
<body>
<div id="log"></div>
<script>
'use strict';

runPropertyTests('empty-cells', [
{ syntax: 'show' },
{ syntax: 'hide' },
]);

</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!doctype html>
<meta charset="utf-8">
<title>'list-style-image' property</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<script src="resources/testsuite.js"></script>
<body>
<div id="log"></div>
<script>
'use strict';

runPropertyTests('list-style-image', [
{ syntax: 'none' },
{ syntax: '<image>' },
]);

</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!doctype html>
<meta charset="utf-8">
<title>'overflow-anchor' property</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<script src="resources/testsuite.js"></script>
<body>
<div id="log"></div>
<script>
'use strict';

runPropertyTests('overflow-anchor', [
{ syntax: 'auto' },
{ syntax: 'none' },
]);

</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!doctype html>
<meta charset="utf-8">
<title>'resize' property</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<script src="resources/testsuite.js"></script>
<body>
<div id="log"></div>
<script>
'use strict';

runPropertyTests('resize', [
{ syntax: 'none' },
{ syntax: 'both' },
{ syntax: 'horizontal' },
{ syntax: 'vertical' },
]);

</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
This is a testharness.js-based test.
PASS Can set 'transition-duration' to CSS-wide keywords
PASS Can set 'transition-duration' to var() references
FAIL Can set 'transition-duration' to a time assert_approx_equals: expected -3.14 +/- 0.000001 but got -0.00314
PASS Setting 'transition-duration' to a length throws TypeError
PASS Setting 'transition-duration' to a percent throws TypeError
PASS Setting 'transition-duration' to a number throws TypeError
PASS Setting 'transition-duration' to a position throws TypeError
PASS Setting 'transition-duration' to a transform throws TypeError
Harness: the test ran to completion.

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<meta charset="utf-8">
<title>'transition-duration' property</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-get">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#dom-stylepropertymap-set">
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#property-stle-value-normalization">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<script src="resources/testsuite.js"></script>
<body>
<div id="log"></div>
<script>
'use strict';

// FIXME: transition-duration is list-valued. Run list-valued tests here too.
runPropertyTests('transition-duration', [
{ syntax: '<time>' },
]);

</script>
Loading

0 comments on commit 1ccb08d

Please sign in to comment.