Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add block-step-size to CSS parser. #10176

Merged
merged 1 commit into from
Feb 21, 2023
Merged

Add block-step-size to CSS parser. #10176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions ...utTests/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ PASS background-repeat
PASS background-size
PASS baseline-shift
PASS block-size
PASS block-step-size
PASS border-block-end-color
PASS border-block-end-style
PASS border-block-end-width
Expand Down
8 changes: 8 additions & 0 deletions ...orted/w3c/web-platform-tests/css/css-rhythm/parsing/block-step-size-computed-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

PASS Property block-step-size value '0px'
PASS Property block-step-size value 'none'
PASS Property block-step-size value '100px'
PASS Property block-step-size value '2em'
PASS Property block-step-size value 'calc(10px + 0.5em)'
PASS Property block-step-size value 'calc(10px - 0.5em)'

29 changes: 29 additions & 0 deletions ...ests/imported/w3c/web-platform-tests/css/css-rhythm/parsing/block-step-size-computed.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<!DOCTYPE html>
<html>
<head>
<title>CSS Rhythm: block-step-size computed values</title>
<link rel="author" title="Sammy Gill" href="sammy.gill@apple.com">
<link rel="help" href="https://drafts.csswg.org/css-rhythm/#block-step-size">
<meta name="assert" content="Checking computed values for block-step-size">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/css/support/computed-testcommon.js"></script>
<script src="/css/support/parsing-testcommon.js"></script>
<style>
#target {
font-size: 40px;
}
</style>
</head>
<body>
<div id="target"></div>
<script>
test_computed_value("block-step-size", "0px");
test_computed_value("block-step-size", "none");
test_computed_value("block-step-size", "100px");
test_computed_value("block-step-size", "2em", "80px");
test_computed_value("block-step-size", "calc(10px + 0.5em)", "30px");
test_computed_value("block-step-size", "calc(10px - 0.5em)", "0px");
</script>
</body>
</html>
7 changes: 7 additions & 0 deletions ...ported/w3c/web-platform-tests/css/css-rhythm/parsing/block-step-size-invalid-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

PASS e.style['block-step-size'] = "auto" should not set the property value
PASS e.style['block-step-size'] = "-1px" should not set the property value
PASS e.style['block-step-size'] = "min-content" should not set the property value
PASS e.style['block-step-size'] = "10%" should not set the property value
PASS e.style['block-step-size'] = "20" should not set the property value

28 changes: 28 additions & 0 deletions ...Tests/imported/w3c/web-platform-tests/css/css-rhythm/parsing/block-step-size-invalid.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!DOCTYPE html>
<html>
<head>
<title>CSS Rhythm: block-step-size invalid values</title>
<link rel="author" title="Sammy Gill" href="sammy.gill@apple.com">
<link rel="help" href="https://drafts.csswg.org/css-rhythm/#block-step-size">
<meta name="assert" content="Invalid values for block-step-size should not be parsed">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/css/support/computed-testcommon.js"></script>
<script src="/css/support/parsing-testcommon.js"></script>
<style>
#target {
font-size: 40px;
}
</style>
</head>
<body>
<div id="target"></div>
<script>
test_invalid_value("block-step-size", "auto");
test_invalid_value("block-step-size", "-1px");
test_invalid_value("block-step-size", "min-content");
test_invalid_value("block-step-size", "10%");
test_invalid_value("block-step-size", "20");
</script>
</body>
</html>
8 changes: 8 additions & 0 deletions ...imported/w3c/web-platform-tests/css/css-rhythm/parsing/block-step-size-valid-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

PASS e.style['block-step-size'] = "1px" should set the property value
PASS e.style['block-step-size'] = "2em" should set the property value
PASS e.style['block-step-size'] = "0" should set the property value
PASS e.style['block-step-size'] = "none" should set the property value
PASS e.style['block-step-size'] = "calc(2em + 3ex)" should set the property value
PASS e.style['block-step-size'] = "1.2em" should set the property value

29 changes: 29 additions & 0 deletions ...utTests/imported/w3c/web-platform-tests/css/css-rhythm/parsing/block-step-size-valid.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<!DOCTYPE html>
<html>
<head>
<title>CSS Rhythm: block-step-size valid values</title>
<link rel="author" title="Sammy Gill" href="sammy.gill@apple.com">
<link rel="help" href="https://drafts.csswg.org/css-rhythm/#block-step-size">
<meta name="assert" content="Parsing valid values for block-step-size properties">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/css/support/computed-testcommon.js"></script>
<script src="/css/support/parsing-testcommon.js"></script>
<style>
#target {
font-size: 40px;
}
</style>
</head>
<body>
<div id="target"></div>
<script>
test_valid_value("block-step-size", "1px");
test_valid_value("block-step-size", "2em");
test_valid_value("block-step-size", "0", "0px");
test_valid_value("block-step-size", "none");
test_valid_value("block-step-size", "calc(2em + 3ex)");
test_valid_value("block-step-size", "1.2em");
</script>
</body>
</html>
1 change: 1 addition & 0 deletions ...orm/gtk/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ PASS background-repeat
PASS background-size
PASS baseline-shift
PASS block-size
PASS block-step-size
PASS border-block-end-color
PASS border-block-end-style
PASS border-block-end-width
Expand Down
1 change: 1 addition & 0 deletions ...ios-wk2/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ PASS background-repeat
PASS background-size
PASS baseline-shift
PASS block-size
PASS block-step-size
PASS border-block-end-color
PASS border-block-end-style
PASS border-block-end-width
Expand Down
1 change: 1 addition & 0 deletions ...rm/ipad/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ PASS background-repeat
PASS background-size
PASS baseline-shift
PASS block-size
PASS block-step-size
PASS border-block-end-color
PASS border-block-end-style
PASS border-block-end-width
Expand Down
1 change: 1 addition & 0 deletions ...mac-wk1/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ PASS background-repeat
PASS background-size
PASS baseline-shift
PASS block-size
PASS block-step-size
PASS border-block-end-color
PASS border-block-end-style
PASS border-block-end-width
Expand Down
1 change: 1 addition & 0 deletions ...orm/wpe/imported/w3c/web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ PASS background-repeat
PASS background-size
PASS baseline-shift
PASS block-size
PASS block-step-size
PASS border-block-end-color
PASS border-block-end-style
PASS border-block-end-width
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/animation/CSSPropertyAnimation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3662,6 +3662,7 @@ CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap()
case CSSPropertyAnimationPlayState:
case CSSPropertyAnimationTimingFunction:
case CSSPropertyAppearance:
case CSSPropertyBlockStepSize:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, the spec doesn't define whether the property is animatable. Can you file an issue on w3c/csswg-drafts?

Feel free to ignore my comment about animation tests for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://drafts.csswg.org/css-rhythm/#block-step-size

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

w3c/csswg-drafts#8459

case CSSPropertyBorderBlock: // logical shorthand
case CSSPropertyBorderBlockColor: // logical shorthand
case CSSPropertyBorderBlockStyle: // logical shorthand
Expand Down
11 changes: 11 additions & 0 deletions Source/WebCore/css/CSSProperties.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,17 @@
"url": "https://drafts.csswg.org/css-grid-3/#tracks-alignment"
}
},
"block-step-size": {
"codegen-properties": {
"settings-flag": "cssRhythmicSizingEnabled",
"converter": "BlockStepSize",
"parser-grammar": "none | <length [0,inf]>"
},
"specification": {
"category": "css-rhythm",
"url": "https://drafts.csswg.org/css-rhythm/#block-step-size"
}
},
"caret-color": {
"inherited": true,
"values": [
Expand Down
6 changes: 6 additions & 0 deletions Source/WebCore/css/ComputedStyleExtractor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2846,6 +2846,12 @@ RefPtr<CSSValue> ComputedStyleExtractor::valueForPropertyInStyle(const RenderSty
list.append(createSingleAxisPositionValueForLayer(propertyID, *currLayer, style));
return CSSValueList::createCommaSeparated(WTFMove(list));
}
case CSSPropertyBlockStepSize: {
auto blockStepSize = style.blockStepSize();
if (!blockStepSize)
return CSSPrimitiveValue::create(CSSValueNone);
return zoomAdjustedPixelValueForLength(*blockStepSize, style);
}
case CSSPropertyBorderCollapse:
if (style.borderCollapse() == BorderCollapse::Collapse)
return CSSPrimitiveValue::create(CSSValueCollapse);
Expand Down
4 changes: 4 additions & 0 deletions Source/WebCore/rendering/style/RenderStyle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2052,6 +2052,10 @@ class RenderStyle {
void setOverflowAnchor(OverflowAnchor a) { SET_NESTED_VAR(m_nonInheritedData, rareData, overflowAnchor, static_cast<unsigned>(a)); }
static OverflowAnchor initialOverflowAnchor() { return OverflowAnchor::Auto; }

static std::optional<Length> initialBlockStepSize() { return { }; }
std::optional<Length> blockStepSize() const { return m_nonInheritedData->rareData->blockStepSize; }
void setBlockStepSize(std::optional<Length> length) { SET_NESTED_VAR(m_nonInheritedData, rareData, blockStepSize, length); }

private:
struct NonInheritedFlags {
bool operator==(const NonInheritedFlags&) const;
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ StyleRareNonInheritedData::StyleRareNonInheritedData()
// scrollSnapAlign
// scrollSnapStop
, zoom(RenderStyle::initialZoom())
, blockStepSize(RenderStyle::initialBlockStepSize())
, overscrollBehaviorX(static_cast<unsigned>(RenderStyle::initialOverscrollBehaviorX()))
, overscrollBehaviorY(static_cast<unsigned>(RenderStyle::initialOverscrollBehaviorY()))
, pageSizeType(PAGE_SIZE_AUTO)
Expand Down Expand Up @@ -165,6 +166,7 @@ inline StyleRareNonInheritedData::StyleRareNonInheritedData(const StyleRareNonIn
, scrollSnapAlign(o.scrollSnapAlign)
, scrollSnapStop(o.scrollSnapStop)
, zoom(o.zoom)
, blockStepSize(o.blockStepSize)
, overscrollBehaviorX(o.overscrollBehaviorX)
, overscrollBehaviorY(o.overscrollBehaviorY)
, pageSizeType(o.pageSizeType)
Expand Down Expand Up @@ -252,6 +254,7 @@ bool StyleRareNonInheritedData::operator==(const StyleRareNonInheritedData& o) c
&& scrollSnapAlign == o.scrollSnapAlign
&& scrollSnapStop == o.scrollSnapStop
&& zoom == o.zoom
&& blockStepSize == o.blockStepSize
&& overscrollBehaviorX == o.overscrollBehaviorX
&& overscrollBehaviorY == o.overscrollBehaviorY
&& pageSizeType == o.pageSizeType
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/rendering/style/StyleRareNonInheritedData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ class StyleRareNonInheritedData : public RefCounted<StyleRareNonInheritedData> {

float zoom;

std::optional<Length> blockStepSize;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to compare in the equality operator as well and add it to the copy constructor.

https://searchfox.org/wubkat/source/Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp#161
https://searchfox.org/wubkat/rev/6312ca8a662145d355274780bbf68b6ce735d8e8/Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp#283

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think I got everything but let me know if I missed something


unsigned overscrollBehaviorX : 2; // OverscrollBehavior
unsigned overscrollBehaviorY : 2; // OverscrollBehavior

Expand Down
9 changes: 9 additions & 0 deletions Source/WebCore/style/StyleBuilderConverter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ class BuilderConverter {
static TextSpacingTrim convertTextSpacingTrim(BuilderState&, const CSSValue&);
static TextAutospace convertTextAutospace(BuilderState&, const CSSValue&);

static std::optional<Length> convertBlockStepSize(BuilderState&, const CSSValue&);

private:
friend class BuilderCustom;

Expand Down Expand Up @@ -1831,5 +1833,12 @@ inline TextAutospace BuilderConverter::convertTextAutospace(BuilderState&, const
return { };
}

inline std::optional<Length> BuilderConverter::convertBlockStepSize(BuilderState& builderState, const CSSValue& value)
{
if (downcast<CSSPrimitiveValue>(value).valueID() == CSSValueNone)
return { };
return convertLength(builderState, value);
}

} // namespace Style
} // namespace WebCore