Skip to content

Commit 0ce68bd

Browse files
marco-cmarco-c
committed
Bug 1823475 - Drop DefaultPosition and omit "at " if it is not specified. r=devtools-reviewers,emilio
`at ` is optional value and we should omit it if author doesn't specify it, for all basic-shape functions, and ray(). Note that there is a related interpolation issue [1] if one of the end values doesn't specify `at `. We didn't address this issue in this patch. [1] w3c/csswg-drafts#9068 Also, this updates the devtool code to give it the default value, "50% 50%", for circle() and ellipse() if `at ` is not specified. Differential Revision: https://phabricator.services.mozilla.com/D181918 UltraBlame original commit: 150d4fac35369ce8b1f5ae336a5a5474993d640d
1 parent bb4a75e commit 0ce68bd

18 files changed

+49
-1155
lines changed

‎devtools/server/actors/highlighters/shapes.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2171,7 +2171,9 @@ class ShapesHighlighter extends AutoRefreshHighlighter {
21712171
const values = definition.split("at");
21722172
let radius = values[0] ? values[0].trim() : "closest-side";
21732173
const { width, height } = this.currentDimensions;
2174-
const center = splitCoords(values[1]).map(
2174+
2175+
const position = values[1] || "50% 50%";
2176+
const center = splitCoords(position).map(
21752177
this.convertCoordsToPercent.bind(this)
21762178
);
21772179

@@ -2258,7 +2260,9 @@ class ShapesHighlighter extends AutoRefreshHighlighter {
22582260
}
22592261

22602262
const values = definition.split("at");
2261-
const center = splitCoords(values[1]).map(
2263+
2264+
const position = values[1] || "50% 50%";
2265+
const center = splitCoords(position).map(
22622266
this.convertCoordsToPercent.bind(this)
22632267
);
22642268

‎layout/base/ShapeUtils.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,12 @@ nsPoint ShapeUtils::ComputeCircleOrEllipseCenter(
5151
const auto& position = aBasicShape.IsCircle()
5252
? aBasicShape.AsCircle().position
5353
: aBasicShape.AsEllipse().position;
54-
MOZ_ASSERT(position.IsPosition(), "A default position should be given");
54+
55+
if (position.IsAuto()) {
56+
return ComputePosition(StylePosition::FromPercentage(0.5), aRefBox);
57+
}
58+
59+
MOZ_ASSERT(position.IsPosition());
5560
return ComputePosition(position.AsPosition(), aRefBox);
5661
}
5762

‎layout/style/test/test_transitions_per_property.html

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -747,9 +747,9 @@
747747
},
748748
// matching functions
749749
{ start: "circle(100px)", end: "circle(500px)",
750-
expected: ["circle", ["200px at 50% 50%"]] },
750+
expected: ["circle", ["200px"]] },
751751
{ start: "ellipse(100px 100px)", end: "ellipse(500px 500px)",
752-
expected: ["ellipse", ["200px 200px at 50% 50%"]] },
752+
expected: ["ellipse", ["200px 200px"]] },
753753
{ start: "circle(100px at 100px 100px) border-box",
754754
end: "circle(500px at 500px 500px) border-box",
755755
expected: ["circle", ["200px at 200px 200px"], "border-box"]
@@ -768,9 +768,9 @@
768768
},
769769
// matching functions percentage
770770
{ start: "circle(100%)", end: "circle(500%)",
771-
expected: ["circle", ["200% at 50% 50%"]] },
771+
expected: ["circle", ["200%"]] },
772772
{ start: "ellipse(100% 100%)", end: "ellipse(500% 500%)",
773-
expected: ["ellipse", ["200% 200% at 50% 50%"]] },
773+
expected: ["ellipse", ["200% 200%"]] },
774774
{ start: "circle(100% at 100% 100%) border-box",
775775
end: "circle(500% at 500% 500%) border-box",
776776
expected: ["circle", ["200% at 200% 200%"], "border-box"]
@@ -788,42 +788,42 @@
788788
expected: ["inset", ["200% round 200%"], "border-box"] },
789789
// matching functions with calc() values
790790
{ start: "circle(calc(80px + 20px))", end: "circle(calc(200px + 300px))",
791-
expected: ["circle", ["200px at 50% 50%"]] },
791+
expected: ["circle", ["200px"]] },
792792
{ start: "circle(calc(80% + 20%))", end: "circle(calc(200% + 300%))",
793-
expected: ["circle", ["200% at 50% 50%"]] },
793+
expected: ["circle", ["200%"]] },
794794
{ start: "circle(calc(10px + 20%))", end: "circle(calc(50px + 40%))",
795-
expected: ["circle", ["calc(25% + 20px) at 50% 50%"]] },
795+
expected: ["circle", ["calc(25% + 20px)"]] },
796796
// matching functions with interpolation between percentage/pixel values
797797
{ start: "circle(20px)", end: "circle(100%)",
798-
expected: ["circle", ["calc(25% + 15px) at 50% 50%"]] },
798+
expected: ["circle", ["calc(25% + 15px)"]] },
799799
{ start: "ellipse(100% 100px at 8px 20%) border-box",
800800
end: "ellipse(40px 4% at 80% 60px) border-box",
801801
expected: ["ellipse", ["calc(75% + 10px) calc(1% + 75px) at " +
802802
"calc(20% + 6px) calc(15% + 15px)"],
803803
"border-box"] },
804804
// no interpolation for keywords
805805
{ start: "circle()", end: "circle(50px)",
806-
expected: ["circle", ["50px at 50% 50%"]] },
806+
expected: ["circle", ["50px"]] },
807807
{ start: "circle(closest-side)", end: "circle(500px)",
808-
expected: ["circle", ["500px at 50% 50%"]] },
808+
expected: ["circle", ["500px"]] },
809809
{ start: "circle(farthest-side)", end: "circle(500px)",
810-
expected: ["circle", ["500px at 50% 50%"]] },
810+
expected: ["circle", ["500px"]] },
811811
{ start: "circle(500px)", end: "circle(farthest-side)",
812-
expected: ["circle", ["farthest-side at 50% 50%"]]},
812+
expected: ["circle", ["farthest-side"]]},
813813
{ start: "circle(500px)", end: "circle(closest-side)",
814-
expected: ["circle", ["at 50% 50%"]]},
814+
expected: ["circle", [""]]},
815815
{ start: "ellipse()", end: "ellipse(50px 50px)",
816-
expected: ["ellipse", ["50px 50px at 50% 50%"]] },
816+
expected: ["ellipse", ["50px 50px"]] },
817817
{ start: "ellipse(closest-side closest-side)", end: "ellipse(500px 500px)",
818-
expected: ["ellipse", ["500px 500px at 50% 50%"]] },
818+
expected: ["ellipse", ["500px 500px"]] },
819819
{ start: "ellipse(farthest-side closest-side)", end: "ellipse(500px 500px)",
820-
expected: ["ellipse", ["500px 500px at 50% 50%"]] },
820+
expected: ["ellipse", ["500px 500px"]] },
821821
{ start: "ellipse(farthest-side farthest-side)", end: "ellipse(500px 500px)",
822-
expected: ["ellipse", ["500px 500px at 50% 50%"]] },
822+
expected: ["ellipse", ["500px 500px"]] },
823823
{ start: "ellipse(500px 500px)", end: "ellipse(farthest-side farthest-side)",
824-
expected: ["ellipse", ["farthest-side farthest-side at 50% 50%"]] },
824+
expected: ["ellipse", ["farthest-side farthest-side"]] },
825825
{ start: "ellipse(500px 500px)", end: "ellipse(closest-side closest-side)",
826-
expected: ["ellipse", ["at 50% 50%"]] },
826+
expected: ["ellipse", [""]] },
827827
// mismatching boxes
828828
{ start: "circle(100px at 100px 100px) border-box",
829829
end: "circle(500px at 500px 500px) content-box",
@@ -847,14 +847,14 @@
847847
expected: ["ellipse", ["500px 500px at 500px 500px"], "border-box"]
848848
},
849849
{ start: "inset(0px round 20px)", end: "ellipse(500px 500px)",
850-
expected: ["ellipse", ["500px 500px at 50% 50%"]]
850+
expected: ["ellipse", ["500px 500px"]]
851851
},
852852
// shape to reference box
853853
{ start: "circle(20px)", end: "content-box", expected: ["content-box"] },
854-
{ start: "content-box", end: "circle(20px)", expected: ["circle", ["20px at 50% 50%"]] },
854+
{ start: "content-box", end: "circle(20px)", expected: ["circle", ["20px"]] },
855855
// url to shape
856856
{ start: "circle(20px)", end: "url(http://localhost/a.png)", expected: ["url", ["\"http://localhost/a.png\""]] },
857-
{ start: "url(http://localhost/a.png)", end: "circle(20px)", expected: ["circle", ["20px at 50% 50%"]] },
857+
{ start: "url(http://localhost/a.png)", end: "circle(20px)", expected: ["circle", ["20px"]] },
858858
// url to none
859859
{ start: "none", end: "url(http://localhost/a.png)", expected: ["url", ["\"http://localhost/a.png\""]] },
860860
{ start: "http://localhost/a.png", end: "none", expected: ["none"] },
@@ -863,7 +863,7 @@
863863
const basicShapesWithFragmentUrlTests = [
864864
// Fragment url to shape
865865
{ start: "circle(20px)", end: "url('#a')", expected: ["url", ["\"#a\""]] },
866-
{ start: "url('#a')", end: "circle(20px)", expected: ["circle", ["20px at 50% 50%"]] },
866+
{ start: "url('#a')", end: "circle(20px)", expected: ["circle", ["20px"]] },
867867
// Fragment url to none
868868
{ start: "none", end: "url('#a')", expected: ["url", ["\"#a\""]] },
869869
{ start: "url('#a')", end: "none", expected: ["none"] },

‎servo/components/style/values/specified/basic_shape.rs

Lines changed: 10 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -129,25 +129,6 @@ pub enum ShapeType {
129129
Outline,
130130
}
131131

132-
133-
134-
135-
136-
137-
138-
139-
pub enum DefaultPosition {
140-
141-
Center,
142-
143-
144-
145-
146-
147-
148-
Context,
149-
}
150-
151132
bitflags! {
152133
/// The flags to represent which basic shapes we would like to support.
153134
///
@@ -219,15 +200,7 @@ where
219200
loop {
220201
if shape.is_none() {
221202
shape = input
222-
.try_parse(|i| {
223-
BasicShape::parse(
224-
context,
225-
i,
226-
flags,
227-
ShapeType::Filled,
228-
DefaultPosition::Center,
229-
)
230-
})
203+
.try_parse(|i| BasicShape::parse(context, i, flags, ShapeType::Filled))
231204
.ok();
232205
}
233206

@@ -311,7 +284,6 @@ impl BasicShape {
311284
input: &mut Parser<'i, 't>,
312285
flags: AllowedBasicShapes,
313286
shape_type: ShapeType,
314-
default_position: DefaultPosition,
315287
) -> Result<Self, ParseError<'i>> {
316288
let location = input.current_source_location();
317289
let function = input.expect_function()?.clone();
@@ -339,11 +311,11 @@ impl BasicShape {
339311
.map(BasicShape::Rect)
340312
},
341313
"circle" if flags.contains(AllowedBasicShapes::CIRCLE) => {
342-
Circle::parse_function_arguments(context, i, default_position)
314+
Circle::parse_function_arguments(context, i)
343315
.map(BasicShape::Circle)
344316
},
345317
"ellipse" if flags.contains(AllowedBasicShapes::ELLIPSE) => {
346-
Ellipse::parse_function_arguments(context, i, default_position)
318+
Ellipse::parse_function_arguments(context, i)
347319
.map(BasicShape::Ellipse)
348320
},
349321
"polygon" if flags.contains(AllowedBasicShapes::POLYGON) => {
@@ -399,19 +371,12 @@ impl InsetRect {
399371
fn parse_at_position<'i, 't>(
400372
context: &ParserContext,
401373
input: &mut Parser<'i, 't>,
402-
default_position: DefaultPosition,
403374
) -> Result<PositionOrAuto, ParseError<'i>> {
404375
if input.try_parse(|i| i.expect_ident_matching("at")).is_ok() {
405-
Position::parse(context, input).map(PositionOrAuto::Position)
406-
} else {
407-
408-
409-
410-
match default_position {
411-
DefaultPosition::Center => Ok(PositionOrAuto::Position(Position::center())),
412-
DefaultPosition::Context => Ok(PositionOrAuto::Auto),
413-
}
376+
return Position::parse(context, input).map(PositionOrAuto::Position);
414377
}
378+
379+
Ok(PositionOrAuto::Auto)
415380
}
416381

417382
impl Parse for Circle {
@@ -420,22 +385,19 @@ impl Parse for Circle {
420385
input: &mut Parser<'i, 't>,
421386
) -> Result<Self, ParseError<'i>> {
422387
input.expect_function_matching("circle")?;
423-
input.parse_nested_block(|i| {
424-
Self::parse_function_arguments(context, i, DefaultPosition::Center)
425-
})
388+
input.parse_nested_block(|i| Self::parse_function_arguments(context, i))
426389
}
427390
}
428391

429392
impl Circle {
430393
fn parse_function_arguments<'i, 't>(
431394
context: &ParserContext,
432395
input: &mut Parser<'i, 't>,
433-
default_position: DefaultPosition,
434396
) -> Result<Self, ParseError<'i>> {
435397
let radius = input
436398
.try_parse(|i| ShapeRadius::parse(context, i))
437399
.unwrap_or_default();
438-
let position = parse_at_position(context, input, default_position)?;
400+
let position = parse_at_position(context, input)?;
439401

440402
Ok(generic::Circle { radius, position })
441403
}
@@ -447,17 +409,14 @@ impl Parse for Ellipse {
447409
input: &mut Parser<'i, 't>,
448410
) -> Result<Self, ParseError<'i>> {
449411
input.expect_function_matching("ellipse")?;
450-
input.parse_nested_block(|i| {
451-
Self::parse_function_arguments(context, i, DefaultPosition::Center)
452-
})
412+
input.parse_nested_block(|i| Self::parse_function_arguments(context, i))
453413
}
454414
}
455415

456416
impl Ellipse {
457417
fn parse_function_arguments<'i, 't>(
458418
context: &ParserContext,
459419
input: &mut Parser<'i, 't>,
460-
default_position: DefaultPosition,
461420
) -> Result<Self, ParseError<'i>> {
462421
let (semiaxis_x, semiaxis_y) = input
463422
.try_parse(|i| -> Result<_, ParseError> {
@@ -467,7 +426,7 @@ impl Ellipse {
467426
))
468427
})
469428
.unwrap_or_default();
470-
let position = parse_at_position(context, input, default_position)?;
429+
let position = parse_at_position(context, input)?;
471430

472431
Ok(generic::Ellipse {
473432
semiaxis_x,

‎servo/components/style/values/specified/motion.rs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,7 @@ impl Parse for OffsetPathFunction {
150150
context: &ParserContext,
151151
input: &mut Parser<'i, 't>,
152152
) -> Result<Self, ParseError<'i>> {
153-
use crate::values::specified::basic_shape::{
154-
AllowedBasicShapes, DefaultPosition, ShapeType,
155-
};
153+
use crate::values::specified::basic_shape::{AllowedBasicShapes, ShapeType};
156154

157155

158156

@@ -175,14 +173,8 @@ impl Parse for OffsetPathFunction {
175173
AllowedBasicShapes::PATH
176174
};
177175

178-
BasicShape::parse(
179-
context,
180-
input,
181-
allowed_shapes,
182-
ShapeType::Outline,
183-
DefaultPosition::Context,
184-
)
185-
.map(OffsetPathFunction::Shape)
176+
BasicShape::parse(context, input, allowed_shapes, ShapeType::Outline)
177+
.map(OffsetPathFunction::Shape)
186178
}
187179
}
188180

‎testing/web-platform/meta/css/css-masking/clip-path/interpolation.html.ini

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎testing/web-platform/meta/css/css-masking/parsing/clip-path-valid.html.ini

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,11 @@
11
[clip-path-valid.html]
22
expected:
33
if (os == "android") and fission: [OK, TIMEOUT]
4-
[e.style['clip-path'\] = "ellipse(1px closest-side)" should set the property value]
5-
expected: FAIL
6-
7-
[e.style['clip-path'\] = "circle()" should set the property value]
8-
expected: FAIL
9-
10-
[e.style['clip-path'\] = "circle(1px)" should set the property value]
11-
expected: FAIL
12-
13-
[e.style['clip-path'\] = "circle(closest-side)" should set the property value]
4+
[e.style['clip-path'\] = "ellipse(farthest-side 4% at bottom left)" should set the property value]
145
expected: FAIL
156

167
[e.style['clip-path'\] = "circle(farthest-side at center top)" should set the property value]
178
expected: FAIL
189

1910
[e.style['clip-path'\] = "circle(4% at top right)" should set the property value]
2011
expected: FAIL
21-
22-
[e.style['clip-path'\] = "ellipse()" should set the property value]
23-
expected: FAIL
24-
25-
[e.style['clip-path'\] = "ellipse(farthest-side 4% at bottom left)" should set the property value]
26-
expected: FAIL

‎testing/web-platform/meta/css/css-shapes/basic-shape-circle-ellipse-serialization.html.ini

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,24 +17,6 @@
1717
[Serialization of basic shapes 5]
1818
expected: FAIL
1919

20-
[Serialization of basic shapes 6]
21-
expected: FAIL
22-
23-
[Serialization of basic shapes 7]
24-
expected: FAIL
25-
26-
[Serialization of basic shapes 8]
27-
expected: FAIL
28-
29-
[Serialization of basic shapes 9]
30-
expected: FAIL
31-
32-
[Serialization of basic shapes 10]
33-
expected: FAIL
34-
35-
[Serialization of basic shapes 11]
36-
expected: FAIL
37-
3820
[Serialization of basic shapes 14]
3921
expected: FAIL
4022

‎testing/web-platform/meta/css/css-shapes/basic-shape-interpolation.html.ini

Lines changed: 0 additions & 6 deletions
This file was deleted.

0 commit comments

Comments
 (0)