search

servo / stylo

70 stars 15 forks source link

`text-decoration` shorthand should support style and color #25

Closed Loirooriol closed 4 months ago

Loirooriol commented 6 months ago

https://github.com/servo/servo/commit/689b413bdebc0d09b105c0b254171f1b1520f449 and https://github.com/servo/servo/commit/928bd0d1d280abf058057ce28b28b0c71033d538 added support for text-decoration-style and text-decoration-color on Servo. However, the shorthand wasn't modified: https://github.com/servo/stylo/blob/3d41c54f18ace2bc3708fe3b5c032c5341a331ed/style/properties/shorthands/text.mako.rs#L10-L11

This seems something that we should fix after catching up with the style updates.

MunishMummadi commented 6 months ago

Hey @Loirooriol I tried to understand the code and I think sub_properties should be updated with.

sub_properties="text-decoration-line ${' text-decoration-style text-decoration-color' if engine == 'servo' else ''} ${' text-decoration-style text-decoration-color text-decoration-thickness' if engine == 'gecko' else ''}"

Let me know if I am wrong.

mrobinson commented 6 months ago

@MunishMummadi The proposal here was to wait until Stylo has been updated to incorporate all of the changes from upstream.

Loirooriol commented 6 months ago

Yeah. Well I guess it's not a big deal to fix it beforehand, but that code is not enough, the parser and serializer need to change too.

MunishMummadi commented 6 months ago

Got it!!!! thanks for info @Loirooriol @mrobinson