Skip to content
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.

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

Enable all supported style properties in "servo" configuration of Stylo #17

Open
nicoburns opened this issue Mar 8, 2024 · 4 comments

Comments

@nicoburns
Copy link
Contributor

Motivation

Enable OSS (non-gecko, non-servo) users of stylo to use all style properties that stylo supports.

Background

The set of style properties that Stylo supports are currently conditionally compiled using a python script to generate them. There are two sets of styles: Gecko and Servo. The Servo style set only enables styles properties (and values of style properties) that Servo supports. For example, the Servo version of the justify-content property does not support the start and end values (although it does support flex-start and flex-end which are similar but ultimately different).

It is presumably set up like this partly for efficiency (don't compile or run code that isn't being used) and partly because the CSS specification specifies that style property values that are not understood should not be parsed (to ensure future compatibility if/when support is added).

Proposal

  • Enable all style properties in the "servo" configuration of stylo (at least where this is relatively easy to do)
  • Align Servo's generated types with Gecko's where possible (e.g. AFAICT there's no reason why Servo couldn't use the same JustifiedContent enum that Gecko is using)
  • Implement ignoring of unsupported style properties at a higher level (for instance by updating unsupported values back to the default value). This doesn't stick to the letter of the spec: it will parse unsupported properties. But it should be functionally equivalent.
  • If particular functionality is particularly heavyweight and merits conditional compilation then it could be given a functionality-specific cargo feature.

Notes

  • Such a migration does not need to be done all at once and could be done property-by-property.
  • It may be possible to remove conditional compilation entirely from large parts of Stylo using this approach
@mrobinson
Copy link
Member

This seems like an interesting idea and pretty useful. This is the sort of change that should be developed upstream in Gecko and then gradually make its way to downstream stylo.

@Loirooriol
Copy link
Contributor

This doesn't stick to the letter of the spec: it will parse unsupported properties. But it should be functionally equivalent.

This is exposed via @supports, CSS.parse, getComputedStyle, etc. It would break websites, Gecko won't accept this at all, and we can't have it in Servo either if we expect it to be a serious browser some day. It would need to be a different feature.

@nicoburns
Copy link
Contributor Author

A few points:

  1. I am not suggesting that anything changes regarding the Gecko build of Stylo. This issue is really about "reducing the diff" and exposing as much of the functionality that is already implemented in Stylo for use by Gecko in the Servo/OSS build of Stylo.

  2. It has occurred to me that Stylo already has a mechanism for runtime enabling/disabling style properties. Specifically the flags in the style_config crate. It seems to me that this could be a good fit for enabling things like CSS Grid properties in (the Servo build of) Stylo while still disabling them in Servo itself until such time as they are implemented.

    This could also be a really good way of landing changes in Stylo while the corresponding Servo changes are still in progress (meaning that e.g. a PR to implement CSS Grid or the gap property could run against Stylo main rather than having to maintain a long-running PR against Stylo.

  3. While it's true that Servo will eventually need accurate support for @supports etc in order to be considered a serious browser, there are also a whole bunch of style properties that Servo will need to actually support in order to be considered a serious browser. We could perhaps be a little loose with enabling parsing of some of those ahead of time where we plan to implement them relatively soon.

    This would reduce the maintenance overhead of implementing these features and would allow other projects depending on Stylo (e.g. stylo-dioxus) to take advantage of support for those styles in the meantime. It is also already the case that we have poor or partial support for a bunch of enabled properties (e.g. column-gap is enabled for multi-column support but not implemented in Flexbox. flex-direction is enabled but not actually implemented).

My disposition is basically that "accurately advertising poor css support" isn't a state worth putting effort into maintaining. We should optimise for "actually implementing good css support" ASAP. And not having to maintain our own copies of style parsing when there is a production grade implementation available would help with that.

@mrobinson
Copy link
Member

@nicoburns Are you interested in these properties in the context of Servo? Perhaps the solution here is to create a third configuration option that exposes all CSS features.

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

No branches or pull requests

3 participants