Skip to content

Enable basic SVG-related CSS properties for Servo#383

Open
mu-mostafa98 wants to merge 4 commits into
servo:mainfrom
mu-mostafa98:enable-svg-styling-for-servo
Open

Enable basic SVG-related CSS properties for Servo#383
mu-mostafa98 wants to merge 4 commits into
servo:mainfrom
mu-mostafa98:enable-svg-styling-for-servo

Conversation

@mu-mostafa98

@mu-mostafa98 mu-mostafa98 commented Jun 5, 2026

Copy link
Copy Markdown

Removes engine = "gecko" from 11 SVG-specific CSS properties to be enabled for servo:

  • Fill (3): fill, fill-opacity, fill-rule
  • Stroke (8): stroke, stroke-width, stroke-linecap, stroke-linejoin, stroke-dasharray, stroke-dashoffset, stroke-miterlimit, stroke-opacity

Also updates the ComputedValues size test (224 → 232) for the new fields.

Servo PR

@xiaochengh xiaochengh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for working on this!

We discussed offline and found this all-in-one PR hard to review. We will change the plan:

  • First, enable only a small set of properties for basic shapes in Stylo
  • And then, implement the basic shapes and these properties in Servo
  • Iterate the above process, and progressively enable more properties in Stylo and implement them in Servo

This way we can make sure that we don't introduce any unintended changes.

@xiaochengh xiaochengh changed the title Enable SVG-related CSS properties for Servo Enable basic SVG-related CSS properties for Servo Jun 9, 2026

@xiaochengh xiaochengh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good in its current shape now. Let's move focus to the Servo PR.

@mu-mostafa98 mu-mostafa98 force-pushed the enable-svg-styling-for-servo branch from 19a8850 to e664b45 Compare June 10, 2026 10:10
@mu-mostafa98 mu-mostafa98 marked this pull request as ready for review June 12, 2026 02:43
@mu-mostafa98 mu-mostafa98 force-pushed the enable-svg-styling-for-servo branch 2 times, most recently from e0eecd7 to b847200 Compare June 15, 2026 03:48
@nicoburns

Copy link
Copy Markdown
Collaborator

Why have the "geometric attributes" (x, y, cx, cy, rx, ry) been reverted? I think those will be needed for even a minimal implementation of SVG.

@mu-mostafa98

Copy link
Copy Markdown
Author

Why have the "geometric attributes" (x, y, cx, cy, rx, ry) been reverted? I think those will be needed for even a minimal implementation of SVG.

I reverted them just to keep this PR strictly focused on fill and stroke so we can get them tested with WPT in Servo. Don't worry, geometric attributes are coming in the very next PR! That will go hand-in-hand with the Servo PR implementing the DOM elements for shapes (rect, circle, ellipse) that actually use them.

@mu-mostafa98 mu-mostafa98 force-pushed the enable-svg-styling-for-servo branch from b847200 to a05d66d Compare June 16, 2026 01:31
@mu-mostafa98 mu-mostafa98 force-pushed the enable-svg-styling-for-servo branch from a05d66d to b058334 Compare June 17, 2026 08:13
@mu-mostafa98 mu-mostafa98 requested a review from xiaochengh June 17, 2026 08:24
@mu-mostafa98 mu-mostafa98 force-pushed the enable-svg-styling-for-servo branch from b058334 to fdea424 Compare June 22, 2026 03:27
Mohamed Mostafa added 3 commits June 26, 2026 09:52
- Remove engine = gecko gating and servo_pref gating from SVG
properties and shorthands.
- Update size_of_test.

Signed-off-by: Mohamed Mostafa mu-mostafa98@gmail.com
…and keep SVG only

Signed-off-by: Mohamed Mostafa mu-mostafa98@gmail.com
…geometry

Signed-off-by: Mohamed Mostafa mu-mostafa98@gmail.com
@mu-mostafa98 mu-mostafa98 force-pushed the enable-svg-styling-for-servo branch 2 times, most recently from 76bf704 to 55cfd2f Compare June 26, 2026 03:23
Signed-off-by: Mohamed Mostafa mu-mostafa98@gmail.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants