Stylo: Implement font-variant-caps

[wafflespeanut]

Please go through the guide before working on this issue.

font-variant-caps is a keyword property which takes one of the following values: normal | small-caps | all-small-caps | petite-caps | all-petite-caps | unicase | titling-caps. The default value is normal. The linked MDN has more information about this.

The constant prefix you should be using is NS_FONT_VARIANT_CAPS

Code: servo/components/style/properties/longhand/font.mako.rs

• Use the example from the MDN page.
• Build stylo and screenshot how the example looks before your changes.
• Make your changes, build stylo and screenshot how it looks after.
• Open a PR with your changes and attach the screenshots

[highfive]

Please make a comment here if you intend to work on this issue. Thank you!

[vgvenkat]

Hello people! I would like to take this on.
I am guessing adding gecko_constant_prefix="NS_FONT_VARIANT_CAPS" at [line repo=stylo-flat](https://github.com/Manishearth/stylo-flat/blob/master/servo/components/style/properties/longhand/font.mako.rs#L128 should do the trick?) .
Is there anything else that needs done?

Also should i submit PR to the stylo-flat repo as I am starting out?

[Manishearth]

Actually, this one is trickier. It needs to be manually implemented.

So the change will be more like https://github.com/servo/servo/pull/13570/files

It is preferable if you submit the pull request to Servo. You can submit to stylo-flat if you think that's easier.

[Manishearth]

You'll also need to expand the list of possible values, right now it only takes two of the ones listed in the issue above. gecko_constant_prefix is only necessary if you're not manually implementing the gecko glue.

[wafflespeanut]

No, this is a little bit trickier than the others, but it can still be implemented using the single_keyword method.

@vgvenkat You're right about the gecko_constant_prefix. Along with that, we'll be needing the gecko_ffi_name (which, in this case would be "mFont.variantCaps").

Also, the constants used in gecko are a bit weird. So, we have to remove some hyphens and adjust some of the values. Using something like, "normal smallcaps allsmall petitecaps allpetite unicase titling", we'll be good to go!

Actually, I was wrong about this. As Manish says, this requires manual glue code for proper parsing and setting of values 😞

It can now be implemented using single_keyword

[Manishearth]

Any updates?

[vgvenkat]

Yes, currently working on a PR with changes similar to https://github.com/servo/servo/pull/13570/files. I will need some more help to check after submission.

[vgvenkat]

I will need to contact some one in the IRC, doing ./mach build --release or ./mach build --dev gives out Build failed, waiting for other jobs to finish....
If some one else is willing to work on this, I can delegate as well, else I will need some help with building the repo. Completed Mac OSx homebrew setup.

[Manishearth]

It's ./mach build-geckolib for this. Also, you're building the Servo repo, you should be building stylo-flat (see the guide). That's just ./mach build

[vgvenkat]

I have added all values from the MDN for the existing font-variant in font.marko.rs. I am guessing no other changes in this file.

As pointed out, I guess the manual glue needs to be added in the gecko.marko.rs file. Also the pointed set_font_stretch in https://github.com/servo/servo/pull/13570/files is no longer in the gecko.mako.rs .

I tried creating a similar function in gecko.marko.rs modifying the set_font_stretch function to set_font_variant and its properties. I ran mach build faster then mach run --disable-e10s https://en.wikipedia.org. The browser opens with the site, if I inspect any element it crashes.

Any directions on how to make the gecko glue manually would be great!

[wafflespeanut]

@vgvenkat Let's wait until #13902 lands, and this will be much easier to implement :)

[wafflespeanut]

@vgvenkat Sorry, I forgot to ping you. That PR has landed. Are you planning to work on this?

[wafflespeanut]

This is open for anyone to jump in. All we have to do is build a constants map just like what we do for image-rendering and auto-generate the code using single_keyword helper.

[Impally]

Reading the guide and taking a glance at this later today so I would like to claim it if that is alright. Thanks.

[wafflespeanut]

Yes please...

[Impally]

Am I wrong in assuming I just need to insert the following code into servo/components/style/properties/longhand/font.mako.rs

${helpers.single_keyword("font-variant-caps",
                         "normal small-caps all-small-caps petite-caps unicase tilting-caps",
                         gecko_constant_prefix="NS_FONT_VARIANT_CAPS",
                         animatable=False)}

After adding that, my build fails because method Font_variant_caps is not a member of trait 'CSSStyleDeclarationMethods'

[Manishearth]

There needs to be a products="gecko" there and you need to run ./mach build-geckolib

Please see the guide. You have to test this using a clone of the Stylo repository. It is not enough to just add that code and submit it.

[wafflespeanut]

Also, note that the generated constants will have incorrect names. This should be done just like image-rendering i.e., a constants map should be passed as argument. For example, simply passing small-caps will generate NS_FONT_VARIANT_CAPS_SMALL_CAPS, which is wrong, because, in gecko, this doesn't have an underscore. So, we should have a dictionary that contains "small-caps": "SMALLCAPS" in order for it to be generated correctly.