Skip to content

declaration: read the whole shape-outside grammar #299

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

Open
samoht wants to merge 2 commits into from
Open

declaration: read the whole shape-outside grammar #299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9861c94
test: pin the shape-outside grammar
samoht Aug 3, 2026
7db2f20
declaration: read the whole shape-outside grammar
samoht Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@

### Parsing

- `shape-outside` reads its whole grammar. Only `none`, `circle()`, a non-empty
`inset()` and the CSS-wide keywords were accepted, so `margin-box`,
`circle(50%) content-box`, `url(shape.png)` and every other basic shape were
rejected and the declaration dropped
- An identifier takes any code point at or above U+0080, so a selector such as
`.text-↗` parses. `Css.of_string` takes `?enforce_spec` and `cascade` takes
`--enforce-spec` to restrict identifiers to the CSS Syntax 3 range list;
Expand Down
89 changes: 57 additions & 32 deletions lib/declaration.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,48 +579,73 @@ let read_text_decoration_lines t =
if duplicates lines then Cursor.err_invalid t "duplicate text-decoration-line";
lines

(* CSS Shapes 1 sec. 2.2: [<shape-box>] is [<visual-box> | margin-box], so the
three SVG boxes [<geometry-box>] adds are not valid here. *)
let check_shape_box t (box : clip_geometry_box) =
match box with
| Margin_box | Border_box | Padding_box | Content_box -> ()
| Fill_box | Stroke_box | View_box ->
Cursor.err_invalid t "shape-outside takes a <shape-box>"

(* [read_clip_path] reads [<basic-shape> || <geometry-box>], the same double-bar
pair shape-outside uses, along with [none], [url()] and the CSS-wide
keywords. Shapes 1 sec. 2 differs on two points, checked here: the box is a
[<shape-box>], and everything outside [<basic-shape>] is an alternative to
the pair rather than a member of it. *)
let check_shape_outside_shape t =
let is_basic_shape (shape : clip_path) =
match shape with
| Clip_path_inset _ | Clip_path_circle _ | Clip_path_ellipse _
| Clip_path_polygon _ | Clip_path_path _ | Clip_path_shape _
| Clip_path_xywh _ | Clip_path_rect _ ->
true
(* A shape the [clip_path] reader kept verbatim as spec-invalid: the raw
text survives here too rather than costing the whole declaration. *)
| Invalid _ -> true
| _ -> false
in
match read_clip_path t with
| Clip_path_box box -> check_shape_box t box
| Clip_path_with_box { shape; box; _ } when is_basic_shape shape ->
check_shape_box t box
| Clip_path_with_box _ ->
Cursor.err_invalid t "shape-outside pairs a <shape-box> with a shape"
| _ -> ()

(* CSS Shapes 1 sec. 2: [shape-outside] is [none | [<basic-shape> ||
<shape-box>] | <image>]. The value is the raw source text ([Shape_outside :
string property]): typing it would mean a sum of the [clip_path] shapes and
the whole [background_image] type for [<image>], so the reader validates the
grammar and hands the text back verbatim. *)
let read_shape_outside t =
let raw = Cursor.lookahead (Cursor.consume_to_decl_end ~trim:true) t in
let accept_single () =
Cursor.skip t;
Cursor.expect_eof t;
raw
in
let accept_var () =
let _ : string var =
Values.read_var
(fun inner -> Cursor.consume_remaining_as_string ~trim:true inner)
t
in
Cursor.expect_eof t;
raw
let read_shape_image t =
ignore (read_background_image t : background_image)
in
match Cursor.peek t with
| Some (Component.Preserved { kind = Token.Ident keyword; _ })
when Properties.is_css_wide_keyword keyword ->
accept_single ()
| Some (Component.Preserved { kind = Token.Ident "none"; _ }) ->
accept_single ()
(match Cursor.peek t with
| Some
(Component.Func
{ node = { name = "var"; terminated = true; arguments = []; _ }; _ })
->
Cursor.err_invalid t "empty var()"
| Some (Component.Func { node = { name = "var"; terminated = true; _ }; _ })
->
accept_var ()
| Some (Component.Func { node = { name = "circle"; terminated; _ }; _ })
when terminated ->
(* CSS Shapes 1 section 3.1: [circle()] is valid (both [<shape-radius>]
and [at <position>] are optional). *)
accept_single ()
| Some
(Component.Func { node = { name = "inset"; arguments; terminated }; _ })
when terminated && arguments <> [] ->
accept_single ()
| Some (Component.Func { node = { name = "inset"; _ }; _ }) ->
Cursor.err_invalid t "empty basic shape"
| _ -> Cursor.err_invalid t ("invalid shape-outside: " ^ raw)
let _ : string var =
Values.read_var
(fun inner -> Cursor.consume_remaining_as_string ~trim:true inner)
t
in
()
| _ ->
Cursor.one_of
[
check_shape_outside_shape;
read_shape_image;
(fun t -> Cursor.err_invalid t ("invalid shape-outside: " ^ raw));
]
t);
Cursor.expect_eof t;
raw

let read_grid_template_list t = read_grid_template t

Expand Down
64 changes: 64 additions & 0 deletions test/test_declaration.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2524,6 +2524,68 @@ let scroll_margin_negative_sheet () =
"a{scroll-margin-inline-start:-1px}";
]

(* A declaration the reader rejects is dropped from the sheet with nothing but a
warning, so every reader gap needs a whole-sheet pin too. *)
let check_sheet_roundtrip name css =
match Css.of_string ~strict:true css with
| Ok { stylesheet; _ } ->
Alcotest.(check string)
(name ^ " sheet roundtrip")
css
(String.trim (Css.to_string ~minify:true stylesheet))
| Error e -> Alcotest.failf "%s: %s" css (Error.to_string e)

(* CSS Shapes 1 sec. 2: [shape-outside] is [none | [<basic-shape> ||
<shape-box>] | <image>]. The reader only looked at the first component and
only knew [none], [circle()] and [inset()] there, so a reference box, a
box/shape pair, the other basic shapes and an image were all rejected and the
declaration dropped. *)
let shape_outside_grammar () =
List.iter
(fun css -> check_declaration ~roundtrip:true css)
[
"shape-outside:none";
"shape-outside:inherit";
"shape-outside:var(--shape)";
(* <basic-shape> *)
"shape-outside:circle(50%)";
"shape-outside:circle()";
"shape-outside:circle(50% at 20% 30%)";
"shape-outside:ellipse(closest-side farthest-side at 10px 20px)";
"shape-outside:inset(10px round 2px)";
"shape-outside:polygon(0 0,100% 0,100% 100%)";
"shape-outside:xywh(0 0 100% 100%)";
(* <shape-box> on its own, and either order in the [||] pair *)
"shape-outside:margin-box";
"shape-outside:content-box";
"shape-outside:circle() border-box";
"shape-outside:padding-box circle(50%)";
(* <image> *)
"shape-outside:url(shape.png)";
"shape-outside:linear-gradient(red,blue)";
];
(* Controls: an unknown keyword is no part of the grammar, [none] and a box
are alternatives rather than a [||] pair, the box appears once, and
[<shape-box>] excludes the three SVG boxes [<geometry-box>] adds. *)
List.iter
(neg_cursor read_declaration)
[
"shape-outside:not-a-shape";
"shape-outside:none margin-box";
"shape-outside:margin-box border-box";
"shape-outside:circle(50%) fill-box";
]

let shape_outside_sheet () =
List.iter
(check_sheet_roundtrip "shape-outside")
[
"a{shape-outside:none}";
"a{shape-outside:margin-box}";
"a{shape-outside:circle(50%) content-box}";
"a{shape-outside:url(shape.png)}";
]

let declaration_tests =
[
(* Core declaration type testing *)
Expand Down Expand Up @@ -2591,6 +2653,8 @@ let declaration_tests =
test_case "scroll-margin negative lengths" `Quick scroll_margin_negative;
test_case "scroll-margin negative lengths (sheet)" `Quick
scroll_margin_negative_sheet;
test_case "shape-outside grammar" `Quick shape_outside_grammar;
test_case "shape-outside grammar (sheet)" `Quick shape_outside_sheet;
(* Spec details and edge cases *)
test_case "CSS-wide keywords" `Quick css_wide_keywords;
test_case "spec cascade 3 shorthand properties" `Quick
Expand Down
8 changes: 5 additions & 3 deletions test/test_optimize.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1517,14 +1517,16 @@ let normalize_pairs =
collapse it to one canonical node and pp alone must stay a fixed point.
(Whether pp keeps these textually distinct depends on which forms the parser
canonicalizes, so that side is asserted only for the colour/calc pairs, where
the node distinction is certain.) *)
the node distinction is certain.) The basic-shape pairs use [clip-path]:
[shape-outside] holds its value as raw text, so nothing about it is
normalized. *)
let gradient_shape_pairs =
[
( "a{background:linear-gradient(to top,red,blue)}",
"a{background:linear-gradient(0deg,red,blue)}" );
("a{clip-path:circle(closest-side at center)}", "a{clip-path:circle()}");
( "a{shape-outside:ellipse(closest-side closest-side at center)}",
"a{shape-outside:ellipse()}" );
( "a{clip-path:ellipse(closest-side closest-side at center)}",
"a{clip-path:ellipse()}" );
]

let assert_pp_keeps_distinct pairs =
Expand Down
Loading