Skip to content

Commit 31871cb

Browse files
weinigweinig
committed
Fix issues with DOMMatrix/IntersectionObserver absolute length unit requirements
https://bugs.webkit.org/show_bug.cgi?id=318632 Reviewed by Darin Adler. Adds a new off by default bit to `CSS::PropertyParserState` called `absoluteLengthUnitsOnly` to make it parse error to consume length units that are not absolute (e.g. font/viewport/container relative units). Uses this for the `DOMMatrix` (`parseTransformRaw`) and `IntersectionObserver` (`parseMargin`) parsers. Additionally, added support for using a `CSS::{primitive}Raw` type in the MetaConsumer, indicating that the parse should not allow `calc()`. Uses this to remove the post parse check in the `IntersectionObserver` (`parseMargin`) parser. Added a test for non-px, but still absolute units in IntersectionObserver and updated a failing DOMMatrix test result. Test: imported/w3c/web-platform-tests/intersection-observer/root-margin-scroll-margin-units.html * LayoutTests/imported/w3c/web-platform-tests/css/geometry/DOMMatrix-001-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/intersection-observer/root-margin-scroll-margin-units-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/intersection-observer/root-margin-scroll-margin-units.html: Added. * Source/WebCore/css/calc/CSSCalcTree+Parser.cpp: * Source/WebCore/css/parser/CSSPropertyParserConsumer+AngleDefinitions.h: * Source/WebCore/css/parser/CSSPropertyParserConsumer+AnglePercentageDefinitions.h: * Source/WebCore/css/parser/CSSPropertyParserConsumer+FlexDefinitions.h: * Source/WebCore/css/parser/CSSPropertyParserConsumer+FrequencyDefinitions.h: * Source/WebCore/css/parser/CSSPropertyParserConsumer+IntegerDefinitions.h: * Source/WebCore/css/parser/CSSPropertyParserConsumer+LengthDefinitions.h: * Source/WebCore/css/parser/CSSPropertyParserConsumer+LengthPercentageDefinitions.h: * Source/WebCore/css/parser/CSSPropertyParserConsumer+NumberDefinitions.h: * Source/WebCore/css/parser/CSSPropertyParserConsumer+PercentageDefinitions.h: * Source/WebCore/css/parser/CSSPropertyParserConsumer+ResolutionDefinitions.h: * Source/WebCore/css/parser/CSSPropertyParserConsumer+TimeDefinitions.h: * Source/WebCore/css/parser/CSSPropertyParserConsumer+Transform.cpp: * Source/WebCore/css/parser/CSSPropertyParserState.h: * Source/WebCore/page/IntersectionObserver.cpp: Canonical link: https://commits.webkit.org/316532@main
1 parent 402dc1b commit 31871cb

18 files changed

Lines changed: 147 additions & 37 deletions

‎LayoutTests/imported/w3c/web-platform-tests/css/geometry/DOMMatrix-001-expected.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ PASS new DOMMatrix("scale(2) translateX(5px) translateY(5px) rotate(5deg) rotate
2323
PASS new DOMMatrix("translateX (5px)")
2424
PASS new DOMMatrix("scale(2 2) translateX(5) translateY(5)")
2525
PASS new DOMMatrix("scale(2, 2), translateX(5) ,translateY(5)")
26-
FAIL new DOMMatrix("scale(sign(1em))") assert_throws_dom: function "function() { new self[constr](string); }" did not throw
26+
PASS new DOMMatrix("scale(sign(1em))")
2727
PASS new DOMMatrix("scale(sibling-index())")
2828
PASS new DOMMatrix("translateX(5em)")
2929
PASS new DOMMatrix("translateX(5ex)")
@@ -92,7 +92,7 @@ PASS new DOMMatrixReadOnly("scale(2) translateX(5px) translateY(5px) rotate(5deg
9292
PASS new DOMMatrixReadOnly("translateX (5px)")
9393
PASS new DOMMatrixReadOnly("scale(2 2) translateX(5) translateY(5)")
9494
PASS new DOMMatrixReadOnly("scale(2, 2), translateX(5) ,translateY(5)")
95-
FAIL new DOMMatrixReadOnly("scale(sign(1em))") assert_throws_dom: function "function() { new self[constr](string); }" did not throw
95+
PASS new DOMMatrixReadOnly("scale(sign(1em))")
9696
PASS new DOMMatrixReadOnly("scale(sibling-index())")
9797
PASS new DOMMatrixReadOnly("translateX(5em)")
9898
PASS new DOMMatrixReadOnly("translateX(5ex)")

‎LayoutTests/imported/w3c/web-platform-tests/intersection-observer/root-margin-scroll-margin-units-expected.txt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
2+
PASS observer.rootMargin using 'px'
3+
PASS observer.scrollMargin using 'px'
4+
PASS observer.rootMargin using 'cm'
5+
PASS observer.scrollMargin using 'cm'
6+
PASS observer.rootMargin using 'mm'
7+
PASS observer.scrollMargin using 'mm'
8+
PASS observer.rootMargin using 'q'
9+
PASS observer.scrollMargin using 'q'
10+
PASS observer.rootMargin using 'in'
11+
PASS observer.scrollMargin using 'in'
12+
PASS observer.rootMargin using 'pt'
13+
PASS observer.scrollMargin using 'pt'
14+
PASS observer.rootMargin using 'pc'
15+
PASS observer.scrollMargin using 'pc'
16+

‎LayoutTests/imported/w3c/web-platform-tests/intersection-observer/root-margin-scroll-margin-units.html‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta name="viewport" content="width=device-width,initial-scale=1">
5+
<link rel="author" href="mailto:sam@webkit.org" title="Sam Weinig">
6+
<link rel="help" href="https://w3c.github.io/IntersectionObserver/#dom-intersectionobserverinit-rootmargin">
7+
<link rel="help" href="https://w3c.github.io/IntersectionObserver/#dom-intersectionobserverinit-scrollmargin">
8+
9+
<script src="/resources/testharness.js"></script>
10+
<script src="/resources/testharnessreport.js"></script>
11+
</head>
12+
<body>
13+
14+
<script>
15+
const testcases = [
16+
{ unit: "px", expected: "10px" },
17+
{ unit: "cm", expected: "377px" },
18+
{ unit: "mm", expected: "37px" },
19+
{ unit: "q", expected: "9px" },
20+
{ unit: "in", expected: "960px" },
21+
{ unit: "pt", expected: "13px" },
22+
{ unit: "pc", expected: "160px" },
23+
];
24+
25+
for (testcase of testcases) {
26+
test(function() {
27+
const observer = new IntersectionObserver(function(e) {}, {
28+
rootMargin: `10${testcase.unit}`,
29+
});
30+
assert_equals(observer.rootMargin, `${testcase.expected} ${testcase.expected} ${testcase.expected} ${testcase.expected}`)
31+
}, `observer.rootMargin using '${testcase.unit}'`);
32+
33+
test(function() {
34+
const observer = new IntersectionObserver(function(e) {}, {
35+
scrollMargin: `10${testcase.unit}`,
36+
});
37+
assert_equals(observer.scrollMargin, `${testcase.expected} ${testcase.expected} ${testcase.expected} ${testcase.expected}`)
38+
}, `observer.scrollMargin using '${testcase.unit}'`);
39+
}
40+
</script>
41+
</body>
42+
</html>

‎Source/WebCore/css/calc/CSSCalcTree+Parser.cpp‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,13 @@ static std::optional<Random::SharingFixed> consumeOptionalRandomSharingFixed(CSS
667667

668668
// Use a non-property parsing state for the fixed number value to disconnect it from the current parse.
669669
// FIXME: Add a mechanism to pass along the depth count when doing this so that we can limit stack usage.
670-
auto numberParsingState = CSS::PropertyParserState { .context = state.propertyParserState.context, .pool = state.propertyParserState.pool };
670+
// FIXME: This should probably maintain the `cssRandomFunctionCount` state from the current state to allow for random() functions nested in the <number> or should document why this is not necessary.
671+
auto numberParsingState = CSS::PropertyParserState {
672+
.context = state.propertyParserState.context,
673+
.pool = state.propertyParserState.pool,
674+
.absoluteLengthUnitsOnly = state.propertyParserState.absoluteLengthUnitsOnly
675+
};
676+
671677
auto number = CSSPropertyParserHelpers::MetaConsumer<CSS::Number<CSS::ClosedUnitRange>>::consume(tokens, numberParsingState);
672678
if (!number)
673679
return { };
@@ -1683,8 +1689,11 @@ std::optional<TypedChild> parseCalcKeyword(const CSSParserToken& token, ParserSt
16831689
auto child = Symbol { token.id(), *unit };
16841690
auto type = Type::determineType(*unit);
16851691

1686-
if (conversionToCanonicalUnitRequiresConversionData(*unit))
1692+
if (conversionToCanonicalUnitRequiresConversionData(*unit)) {
1693+
if (state.propertyParserState.absoluteLengthUnitsOnly)
1694+
return std::nullopt;
16871695
state.requiresConversionData = true;
1696+
}
16881697

16891698
if (auto* simplificationOptions = state.simplificationOptions) {
16901699
if (auto replacement = simplify(child, *simplificationOptions))
@@ -1726,8 +1735,11 @@ std::optional<TypedChild> parseCalcDimension(const CSSParserToken& token, Parser
17261735
auto child = makeNumeric(token.numericValue(), token.unitType());
17271736
auto type = Type::determineType(token.unitType());
17281737

1729-
if (conversionToCanonicalUnitRequiresConversionData(token.unitType()))
1738+
if (conversionToCanonicalUnitRequiresConversionData(token.unitType())) {
1739+
if (state.propertyParserState.absoluteLengthUnitsOnly)
1740+
return std::nullopt;
17301741
state.requiresConversionData = true;
1742+
}
17311743

17321744
if (auto* simplificationOptions = state.simplificationOptions)
17331745
return TypedChild { copyAndSimplify(WTF::move(child), *simplificationOptions), type };

‎Source/WebCore/css/parser/CSSPropertyParserConsumer+AngleDefinitions.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,5 +65,10 @@ template<auto R, typename V> struct ConsumerDefinition<CSS::Angle<R, V>> {
6565
using NumberToken = NumberConsumerForUnitlessValues<CSS::Angle<R, V>, AngleValidator, CSS::AngleUnit::Deg>;
6666
};
6767

68+
template<auto R, typename V> struct ConsumerDefinition<CSS::AngleRaw<R, V>> {
69+
using DimensionToken = DimensionConsumer<CSS::Angle<R, V>, AngleValidator>;
70+
using NumberToken = NumberConsumerForUnitlessValues<CSS::Angle<R, V>, AngleValidator, CSS::AngleUnit::Deg>;
71+
};
72+
6873
} // namespace CSSPropertyParserHelpers
6974
} // namespace WebCore

‎Source/WebCore/css/parser/CSSPropertyParserConsumer+AnglePercentageDefinitions.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,5 +59,11 @@ template<auto R, typename V> struct ConsumerDefinition<CSS::AnglePercentage<R, V
5959
using NumberToken = NumberConsumerForUnitlessValues<CSS::AnglePercentage<R, V>, AnglePercentageValidator, CSS::AngleUnit::Deg>;
6060
};
6161

62+
template<auto R, typename V> struct ConsumerDefinition<CSS::AnglePercentageRaw<R, V>> {
63+
using DimensionToken = DimensionConsumer<CSS::AnglePercentage<R, V>, AnglePercentageValidator>;
64+
using PercentageToken = PercentageConsumer<CSS::AnglePercentage<R, V>, AnglePercentageValidator>;
65+
using NumberToken = NumberConsumerForUnitlessValues<CSS::AnglePercentage<R, V>, AnglePercentageValidator, CSS::AngleUnit::Deg>;
66+
};
67+
6268
} // namespace CSSPropertyParserHelpers
6369
} // namespace WebCore

‎Source/WebCore/css/parser/CSSPropertyParserConsumer+FlexDefinitions.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,9 @@ template<auto R, typename V> struct ConsumerDefinition<CSS::Flex<R, V>> {
4646
using DimensionToken = DimensionConsumer<CSS::Flex<R, V>, FlexValidator>;
4747
};
4848

49+
template<auto R, typename V> struct ConsumerDefinition<CSS::FlexRaw<R, V>> {
50+
using DimensionToken = DimensionConsumer<CSS::Flex<R, V>, FlexValidator>;
51+
};
52+
4953
} // namespace CSSPropertyParserHelpers
5054
} // namespace WebCore

‎Source/WebCore/css/parser/CSSPropertyParserConsumer+FrequencyDefinitions.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,9 @@ template<auto R, typename V> struct ConsumerDefinition<CSS::Frequency<R, V>> {
5050
using DimensionToken = DimensionConsumer<CSS::Frequency<R, V>, FrequencyValidator>;
5151
};
5252

53+
template<auto R, typename V> struct ConsumerDefinition<CSS::FrequencyRaw<R, V>> {
54+
using DimensionToken = DimensionConsumer<CSS::Frequency<R, V>, FrequencyValidator>;
55+
};
56+
5357
} // namespace CSSPropertyParserHelpers
5458
} // namespace WebCore

‎Source/WebCore/css/parser/CSSPropertyParserConsumer+IntegerDefinitions.h‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,13 @@ template<typename Primitive, typename Validator> struct NumberConsumerForInteger
6060
}
6161
};
6262

63-
template<CSS::Range R, typename IntType>
64-
struct ConsumerDefinition<CSS::Integer<R, IntType>> {
65-
using FunctionToken = FunctionConsumerForCalcValues<CSS::Integer<R, IntType>>;
66-
using NumberToken = NumberConsumerForIntegerValues<CSS::Integer<R, IntType>, IntegerValidator>;
63+
template<auto R, typename V> struct ConsumerDefinition<CSS::Integer<R, V>> {
64+
using FunctionToken = FunctionConsumerForCalcValues<CSS::Integer<R, V>>;
65+
using NumberToken = NumberConsumerForIntegerValues<CSS::Integer<R, V>, IntegerValidator>;
66+
};
67+
68+
template<auto R, typename V> struct ConsumerDefinition<CSS::IntegerRaw<R, V>> {
69+
using NumberToken = NumberConsumerForIntegerValues<CSS::Integer<R, V>, IntegerValidator>;
6770
};
6871

6972
} // namespace CSSPropertyParserHelpers

‎Source/WebCore/css/parser/CSSPropertyParserConsumer+LengthDefinitions.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ struct LengthValidator {
3737
{
3838
if (unitType == CSSUnitType::CSS_QUIRKY_EM && !isUASheetBehavior(options.overrideParserMode.value_or(state.context.mode)))
3939
return std::nullopt;
40-
return CSS::UnitTraits<CSS::LengthUnit>::validate(unitType);
40+
auto result = CSS::UnitTraits<CSS::LengthUnit>::validate(unitType);
41+
if (result && state.absoluteLengthUnitsOnly && CSS::conversionToCanonicalUnitRequiresConversionData(*result))
42+
return std::nullopt;
43+
return result;
4144
}
4245

4346
template<auto R, typename V> static bool isValid(CSS::LengthRaw<R, V> raw, CSSPropertyParserOptions)
@@ -66,5 +69,10 @@ template<auto R, typename V> struct ConsumerDefinition<CSS::Length<R, V>> {
6669
using NumberToken = NumberConsumerForUnitlessValues<CSS::Length<R, V>, LengthValidator, CSS::LengthUnit::Px>;
6770
};
6871

72+
template<auto R, typename V> struct ConsumerDefinition<CSS::LengthRaw<R, V>> {
73+
using DimensionToken = DimensionConsumer<CSS::Length<R, V>, LengthValidator>;
74+
using NumberToken = NumberConsumerForUnitlessValues<CSS::Length<R, V>, LengthValidator, CSS::LengthUnit::Px>;
75+
};
76+
6977
} // namespace CSSPropertyParserHelpers
7078
} // namespace WebCore

0 commit comments

Comments
 (0)