Skip to content

Commit

Permalink
[@container] Change order of container shorthand
Browse files Browse the repository at this point in the history
Adjust ConsumeContainerName so that we don't fail parsing completely
whenever we encounter something unexpected.

Since the initial value of container-type is an open issue [1],
I'm leaving that as-is for now.

Fixed: 1305128

[1] w3c/csswg-drafts#7202

Change-Id: I156a8da9a79d2108935958f3418b16aeb83d490e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3575938
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/main@{#990367}
NOKEYCHECK=True
GitOrigin-RevId: 9a8ea60b02f145d506b43ce64bdb080f5247e849
  • Loading branch information
andruud authored and copybara-github committed Apr 8, 2022
1 parent b560111 commit b0b7cc8
Show file tree
Hide file tree
Showing 15 changed files with 114 additions and 103 deletions.
10 changes: 5 additions & 5 deletions blink/renderer/core/css/container_query_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ TEST_F(ContainerQueryTest, OldStyleForTransitions) {
SetBodyInnerHTML(R"HTML(
<style>
#container {
container: inline-size;
container-type: inline-size;
width: 20px;
}
#target {
Expand Down Expand Up @@ -611,7 +611,7 @@ TEST_F(ContainerQueryTest, TransitionAppearingInFinalPass) {
SetBodyInnerHTML(R"HTML(
<style>
#container {
container: inline-size;
container-type: inline-size;
width: 20px;
}
#target {
Expand Down Expand Up @@ -684,7 +684,7 @@ TEST_F(ContainerQueryTest, TransitionTemporarilyAppearing) {
SetBodyInnerHTML(R"HTML(
<style>
#container {
container: inline-size;
container-type: inline-size;
width: 20px;
}
#target {
Expand Down Expand Up @@ -758,7 +758,7 @@ TEST_F(ContainerQueryTest, RedefiningAnimations) {
to { height: 100px; }
}
#container {
container: inline-size;
container-type: inline-size;
width: 10px;
}
@container (width: 120px) {
Expand Down Expand Up @@ -834,7 +834,7 @@ TEST_F(ContainerQueryTest, UnsetAnimation) {
to { height: 100px; }
}
#container {
container: inline-size;
container-type: inline-size;
width: 10px;
}
#target {
Expand Down
2 changes: 1 addition & 1 deletion blink/renderer/core/css/css_properties.json5
Original file line number Diff line number Diff line change
Expand Up @@ -6373,7 +6373,7 @@
{
name: "container",
longhands: [
"container-type", "container-name"
"container-name", "container-type"
],
property_methods: ["ParseShorthand", "CSSValueFromComputedStyleInternal"],
runtime_flag: "CSSContainerQueries",
Expand Down
20 changes: 10 additions & 10 deletions blink/renderer/core/css/properties/computed_style_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2950,27 +2950,27 @@ CSSValueList* ComputedStyleUtils::ValuesForContainerShorthand(
bool allow_visited_style) {
CHECK_EQ(containerShorthand().length(), 2u);
CHECK_EQ(containerShorthand().properties()[0],
&GetCSSPropertyContainerType());
CHECK_EQ(containerShorthand().properties()[1],
&GetCSSPropertyContainerName());
CHECK_EQ(containerShorthand().properties()[1],
&GetCSSPropertyContainerType());

CSSValueList* list = CSSValueList::CreateSlashSeparated();

const CSSValue* type =
GetCSSPropertyContainerType().CSSValueFromComputedStyle(
style, layout_object, allow_visited_style);
const CSSValue* name =
GetCSSPropertyContainerName().CSSValueFromComputedStyle(
style, layout_object, allow_visited_style);
const CSSValue* type =
GetCSSPropertyContainerType().CSSValueFromComputedStyle(
style, layout_object, allow_visited_style);

DCHECK(type);
DCHECK(name);
DCHECK(type);

list->Append(*type);
list->Append(*name);

if (!(IsA<CSSIdentifierValue>(name) &&
To<CSSIdentifierValue>(*name).GetValueID() == CSSValueID::kNone)) {
list->Append(*name);
if (!(IsA<CSSIdentifierValue>(type) &&
To<CSSIdentifierValue>(*type).GetValueID() == CSSValueID::kNone)) {
list->Append(*type);
}

return list;
Expand Down
4 changes: 2 additions & 2 deletions blink/renderer/core/css/properties/css_parsing_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5265,10 +5265,10 @@ CSSValue* ConsumeContainerName(CSSParserTokenRange& range,

CSSValueList* list = CSSValueList::CreateSpaceSeparated();

while (!range.AtEnd()) {
while (true) {
CSSValue* value = ConsumeSingleContainerName(range, context);
if (!value)
return nullptr;
break;
list->Append(*value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -900,26 +900,27 @@ bool Container::ParseShorthand(
const CSSParserContext& context,
const CSSParserLocalContext&,
HeapVector<CSSPropertyValue, 256>& properties) const {
const CSSValue* type = css_parsing_utils::ConsumeContainerType(range);
if (!type)
const CSSValue* name =
css_parsing_utils::ConsumeContainerName(range, context);
if (!name)
return false;

const CSSValue* name = CSSIdentifierValue::Create(CSSValueID::kNone);
const CSSValue* type = CSSIdentifierValue::Create(CSSValueID::kNone);
if (css_parsing_utils::ConsumeSlashIncludingWhitespace(range)) {
if (!(name = css_parsing_utils::ConsumeContainerName(range, context)))
if (!(type = css_parsing_utils::ConsumeContainerType(range)))
return false;
}

if (!range.AtEnd())
return false;

css_parsing_utils::AddProperty(
CSSPropertyID::kContainerType, CSSPropertyID::kContainer, *type,
CSSPropertyID::kContainerName, CSSPropertyID::kContainer, *name,
important, css_parsing_utils::IsImplicitProperty::kNotImplicit,
properties);

css_parsing_utils::AddProperty(
CSSPropertyID::kContainerName, CSSPropertyID::kContainer, *name,
CSSPropertyID::kContainerType, CSSPropertyID::kContainer, *type,
important, css_parsing_utils::IsImplicitProperty::kNotImplicit,
properties);

Expand Down
18 changes: 9 additions & 9 deletions blink/renderer/core/css/style_property_serializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -693,25 +693,25 @@ bool StylePropertySerializer::AppendFontLonghandValueIfNotNormal(
String StylePropertySerializer::ContainerValue() const {
CHECK_EQ(containerShorthand().length(), 2u);
CHECK_EQ(containerShorthand().properties()[0],
&GetCSSPropertyContainerType());
CHECK_EQ(containerShorthand().properties()[1],
&GetCSSPropertyContainerName());
CHECK_EQ(containerShorthand().properties()[1],
&GetCSSPropertyContainerType());

CSSValueList* list = CSSValueList::CreateSlashSeparated();

const CSSValue* type =
property_set_.GetPropertyCSSValue(GetCSSPropertyContainerType());
const CSSValue* name =
property_set_.GetPropertyCSSValue(GetCSSPropertyContainerName());
const CSSValue* type =
property_set_.GetPropertyCSSValue(GetCSSPropertyContainerType());

DCHECK(type);
DCHECK(name);
DCHECK(type);

list->Append(*type);
list->Append(*name);

if (!(IsA<CSSIdentifierValue>(name) &&
To<CSSIdentifierValue>(*name).GetValueID() == CSSValueID::kNone)) {
list->Append(*name);
if (!(IsA<CSSIdentifierValue>(type) &&
To<CSSIdentifierValue>(*type).GetValueID() == CSSValueID::kNone)) {
list->Append(*type);
}

return list->CssText();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@ This is a testharness.js-based test.
PASS Property container value 'initial'
PASS Property container value 'inherit'
PASS Property container value 'unset'
PASS Property container value 'inline-size'
PASS Property container value 'size'
PASS Property container value 'none / inline-size'
PASS Property container value 'none / size'
PASS Property container value 'inline-size / inline-size'
PASS Property container value 'size / block-size'
FAIL Property container value 'size style / name' assert_true: 'size style / name' is a supported value for container. expected true got false
FAIL Property container value 'inline-size style/ name' assert_true: 'inline-size style/ name' is a supported value for container. expected true got false
PASS Property container value 'inline-size / foo'
PASS Property container value 'inline-size /foo'
PASS Property container value 'inline-size/ foo'
PASS Property container value 'inline-size/foo'
PASS Property container value 'size / FoO'
PASS Property container value 'size / foo bar'
PASS Property container value 'block-size / size'
FAIL Property container value 'name / size style' assert_true: 'name / size style' is a supported value for container. expected true got false
FAIL Property container value 'name /inline-size style' assert_true: 'name /inline-size style' is a supported value for container. expected true got false
PASS Property container value 'foo / inline-size'
PASS Property container value 'foo /inline-size'
PASS Property container value 'foo/ inline-size'
PASS Property container value 'foo/inline-size'
PASS Property container value 'FoO / size'
PASS Property container value 'foo bar / size'
Harness: the test ran to completion.

Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@
test_computed_value('container', 'initial', 'none');
test_computed_value('container', 'inherit', 'none');
test_computed_value('container', 'unset', 'none');
test_computed_value('container', 'inline-size');
test_computed_value('container', 'size');
test_computed_value('container', 'none / inline-size');
test_computed_value('container', 'none / size');
test_computed_value('container', 'inline-size / inline-size');
test_computed_value('container', 'size / block-size');
test_computed_value('container', 'size style / name', 'style size / name');
test_computed_value('container', 'inline-size style/ name', 'style inline-size / name');
test_computed_value('container', 'inline-size / foo');
test_computed_value('container', 'inline-size /foo', 'inline-size / foo');
test_computed_value('container', 'inline-size/ foo', 'inline-size / foo');
test_computed_value('container', 'inline-size/foo', 'inline-size / foo');
test_computed_value('container', 'size / FoO', 'size / FoO');
test_computed_value('container', 'size / foo bar', 'size / foo bar');
test_computed_value('container', 'block-size / size');
test_computed_value('container', 'name / size style', 'name / style size');
test_computed_value('container', 'name /inline-size style', 'name / style inline-size');
test_computed_value('container', 'foo / inline-size');
test_computed_value('container', 'foo /inline-size', 'foo / inline-size');
test_computed_value('container', 'foo/ inline-size', 'foo / inline-size');
test_computed_value('container', 'foo/inline-size', 'foo / inline-size');
test_computed_value('container', 'FoO / size');
test_computed_value('container', 'foo bar / size', 'foo bar / size');
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,40 @@ PASS e.style['container'] = "revert" should set the property value
PASS e.style['container'] = "none" should set the property value
PASS e.style['container'] = "none / none" should set the property value
PASS e.style['container'] = "inline-size" should set the property value
PASS e.style['container'] = "inline-size / none" should set the property value
PASS e.style['container'] = "none / inline-size" should set the property value
PASS e.style['container'] = "size" should set the property value
PASS e.style['container'] = "size / block-size" should set the property value
PASS e.style['container'] = "block-size / size" should set the property value
PASS e.style['container'] = "inline-size / inline-size" should set the property value
PASS e.style['container'] = "size / size" should set the property value
FAIL e.style['container'] = "size style / none" should set the property value assert_not_equals: property should be set got disallowed value ""
PASS e.style['container'] = "size / foo" should set the property value
PASS e.style['container'] = "size / foo bar" should set the property value
FAIL e.style['container'] = "none / size style" should set the property value assert_not_equals: property should be set got disallowed value ""
PASS e.style['container'] = "foo" should set the property value
PASS e.style['container'] = "foo / none" should set the property value
PASS e.style['container'] = "foo bar / size" should set the property value
PASS e.style['container'] = "foo bar / none" should set the property value
PASS e.style['container'] = "FOO / size" should set the property value
PASS e.style['container'] = "FOO/size" should set the property value
PASS e.style['container'] = " FOO /size" should set the property value
PASS e.style['container'] = "none none" should not set the property value
PASS e.style['container'] = "none inline-size" should not set the property value
PASS e.style['container'] = "inline-size none" should not set the property value
PASS e.style['container'] = "inline-size inline-size" should not set the property value
PASS e.style['container'] = "inline-size block-size unknown" should not set the property value
PASS e.style['container'] = "inline-size block-size" should not set the property value
PASS e.style['container'] = "size block-size" should not set the property value
PASS e.style['container'] = "none / inline-size none" should not set the property value
PASS e.style['container'] = "none / inline-size inline-size" should not set the property value
PASS e.style['container'] = "none / inline-size block-size unknown" should not set the property value
PASS e.style['container'] = "none / inline-size block-size" should not set the property value
PASS e.style['container'] = "none / size block-size" should not set the property value
PASS e.style['container'] = "none, none" should not set the property value
PASS e.style['container'] = "foo" should not set the property value
PASS e.style['container'] = "foo, bar" should not set the property value
PASS e.style['container'] = "none / foo" should not set the property value
PASS e.style['container'] = "none / foo, bar" should not set the property value
PASS e.style['container'] = "#fff" should not set the property value
PASS e.style['container'] = "1px" should not set the property value
PASS e.style['container'] = "default" should not set the property value
PASS e.style['container'] = "inline-size / 10px" should not set the property value
PASS e.style['container'] = "inline-size / #fefefe" should not set the property value
PASS e.style['container'] = "inline-size / calc(3px)" should not set the property value
PASS e.style['container'] = "10px / inline-size" should not set the property value
PASS e.style['container'] = "#fefefe / inline-size" should not set the property value
PASS e.style['container'] = "calc(3px) / inline-size" should not set the property value
PASS e.style['container'] = "size 1 / name" should not set the property value
PASS e.style['container'] = "block-size" should not set the property value
PASS e.style['container'] = "block-size / name" should not set the property value
PASS e.style['container'] = "block-size / NAME" should not set the property value
PASS e.style['container'] = "block-size/NAME" should not set the property value
PASS e.style['container'] = "none / block-size" should not set the property value
PASS e.style['container'] = "name / block-size" should not set the property value
PASS e.style['container'] = " NAME / block-size" should not set the property value
PASS e.style['container'] = "NAME/block-size" should not set the property value
PASS e.style['container'] = "block-size / block-size" should not set the property value
Harness: the test ran to completion.

Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,40 @@
test_valid_value('container', 'none');
test_valid_value('container', 'none / none', 'none');
test_valid_value('container', 'inline-size');
test_valid_value('container', 'inline-size / none', 'inline-size');
test_valid_value('container', 'none / inline-size', 'none / inline-size');
test_valid_value('container', 'size');
test_valid_value('container', 'size / block-size');
test_valid_value('container', 'block-size / size');
test_valid_value('container', 'inline-size / inline-size');
test_valid_value('container', 'size / size');
test_valid_value('container', 'size style / none', 'style size');
test_valid_value('container', 'size / foo');
test_valid_value('container', 'size / foo bar');
test_valid_value('container', 'none / size style', 'none / style size');
test_valid_value('container', 'foo');
test_valid_value('container', 'foo / none', 'foo');
test_valid_value('container', 'foo bar / size');
test_valid_value('container', 'foo bar / none', 'foo bar');
test_valid_value('container', 'FOO / size');
test_valid_value('container', 'FOO/size', 'FOO / size');
test_valid_value('container', ' FOO /size', 'FOO / size');

test_invalid_value('container', 'none none');
test_invalid_value('container', 'none inline-size');
test_invalid_value('container', 'inline-size none');
test_invalid_value('container', 'inline-size inline-size');
test_invalid_value('container', 'inline-size block-size unknown');
test_invalid_value('container', 'inline-size block-size');
test_invalid_value('container', 'size block-size');
test_invalid_value('container', 'none / inline-size none');
test_invalid_value('container', 'none / inline-size inline-size');
test_invalid_value('container', 'none / inline-size block-size unknown');
test_invalid_value('container', 'none / inline-size block-size');
test_invalid_value('container', 'none / size block-size');
test_invalid_value('container', 'none, none');
test_invalid_value('container', 'foo');
test_invalid_value('container', 'foo, bar');
test_invalid_value('container', 'none / foo');
test_invalid_value('container', 'none / foo, bar');
test_invalid_value('container', '#fff');
test_invalid_value('container', '1px');
test_invalid_value('container', 'default');
test_invalid_value('container', 'inline-size / 10px');
test_invalid_value('container', 'inline-size / #fefefe');
test_invalid_value('container', 'inline-size / calc(3px)');
test_invalid_value('container', '10px / inline-size');
test_invalid_value('container', '#fefefe / inline-size');
test_invalid_value('container', 'calc(3px) / inline-size');
test_invalid_value('container', 'size 1 / name');
test_invalid_value('container', 'block-size');
test_invalid_value('container', 'block-size / name');
test_invalid_value('container', 'block-size / NAME', 'block-size / NAME');
test_invalid_value('container', 'block-size/NAME','block-size / NAME');
test_invalid_value('container', 'none / block-size');
test_invalid_value('container', 'name / block-size');
test_invalid_value('container', ' NAME / block-size', 'NAME / block-size');
test_invalid_value('container', 'NAME/block-size','NAME / block-size');
test_invalid_value('container', 'block-size / block-size');
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@
.inline { container-type: inline-size; }
.size { container-type: size; }

.a-inline { container: inline-size / a; }
.a-size { container: size / a; }
.a-inline { container: a / inline-size; }
.a-size { container: a / size; }

.b-size { container: inline-size / b; }
.b-size { container: size / b; }
.b-size { container: inline- b / size; }
.b-size { container: b / size; }

.ab-size { container: size / a b; }
.ab-size { container: a b / size; }

.a { container-name: a; contain: strict; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<link rel="help" href="https://drafts.csswg.org/css-contain-3/#size-container">
<link rel="help" href="https://crbug.com/1289850">
<p>Pass if there is no crash.</p>
<canvas id="canv" style="display:block;position:absolute;container:inline-size"></canvas>
<canvas id="canv" style="display:block;position:absolute;container-type:inline-size"></canvas>
<script>
canv.offsetTop;
canv.appendChild(document.createElement("span"));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1289718">
<div style="container:inline-size;">
<div style="container-type:inline-size;">
<span style="columns:2;"></span>
</div>
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1289718">
<div style="container:inline-size;">
<div style="container-type:inline-size;">
<video style="columns:2;"></video>
</div>
Loading

0 comments on commit b0b7cc8

Please sign in to comment.