Implement place-{content,self,items} shorthand properties

[upsuper]

Spec:

• place-content: https://drafts.csswg.org/css-align/#propdef-place-content
• place-self: https://drafts.csswg.org/css-align/#place-self-property
• place-items: https://drafts.csswg.org/css-align/#place-items-property

They are basically just shorthands of several existing properties, so it should be easy to implement.

This should be implemented in https://github.com/servo/servo/blob/master/components/style/properties/shorthand/position.mako.rs

and the related sub-properties are defined in https://github.com/servo/servo/blob/master/components/style/properties/longhand/position.mako.rs

[highfive]

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. 😄

[SebastinSanty]

I think this would be similar to the other issue I am solving and hence would like to work on this too. @highfive: assign me

[highfive]

Hey @SebastinSanty! Thanks for your interest in working on this issue. It's now assigned to you!

[wafflespeanut]

@SebastinSanty Have you made any progress? Any questions?

[SebastinSanty]

@wafflespeanut Yep! I'm almost done. I'll put in a PR in some time.

[highfive]

cc @emilio

[wafflespeanut]

@SebastinSanty Any news?

[tamamu]

Is this issue now open? If so, I’ll give it a shot.
@highfive: assigin me

[wafflespeanut]

Yes it is. Go for it! (and, spelling) 😄

[tamamu]

Typo! Oops ... well, what I should do is to implement the shorthands properties in
https://github.com/servo/servo/blob/master/components/style/properties/shorthand/position.mako.rs
right? Is any test necessary?

[wafflespeanut]

You're right! And yeah, it'd be nice to have some tests in tests/unit/style/properties/serialization.rs (where other similar shorthand serialization tests live)

[tamamu]

Can I ask a question? I don't know how to import values::specified::align:::AlignJustifyContent. When I did mach test unit, I got an error as below (but mach build -d succeeded.)

   Compiling style v0.0.1 (file:///C:/Users/tamam/Documents/servo/components/style)
error[E0432]: unresolved import `values::specified::align::AlignJustifyContent`
     --> C:\Users\tamam\Documents\servo\target\debug\build\style-58fd9169ab377329\out/properties.rs:61186:9
      |
61186 |     use values::specified::align::AlignJustifyContent;
      |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `align` in `specified`

As requirements of place-content,

The second value is assigned to justify-content; if omitted, it is copied from the first value.

so I probably should use the type to convert an align-content value to justify-content value.

[upsuper]

Ah, it seems align mod is currently Gecko-only. I guess we may want to change it to not Gecko-only to help testing? But having a look at that mod, there are indeed lots of Gecko-specific things (from gecko_bindings::structs)... so I have no idea what should we do here.

Maybe we can just skip testing for this for now... @Manishearth what do you think?

[upsuper]

Or @wafflespeanut if you have any suggestion here.

[Manishearth]

yes, skipping testing is fine, provided the geckolib build works and it works in stylo

[Manishearth]

oh. don't remove the property from testing. not writing tests for it is fine (we have stylo reftests).

Instead of importing AlignJustifyContent, could you import properties::longhands::align_content::SpecifiedValue? That will do the right thing on both.

[tamamu]

OK, I have done place-content. So now I'm working on place-self.

And another question; one of place-self sub property, justify-self caught an error below by mach test-unit -p style. Though you said skipping testing is fine, can I ignore this error? Or have any solution?

Traceback (most recent call last):
  File "C:\Users\tamam\Documents\servo\components\style\properties\build.py", line 61, in render
    return template.render(**context).encode("utf8")
  File "C:\Users\tamam\Documents\servo\components\style\properties\Mako-0.9.1.zip\mako\template.py", line 443, in render
    return runtime._render(self, self.callable_, args, data)
  File "C:\Users\tamam\Documents\servo\components\style\properties\Mako-0.9.1.zip\mako\runtime.py", line 807, in _render
    **_kwargs_for_callable(callable_, data))
  File "C:\Users\tamam\Documents\servo\components\style\properties\Mako-0.9.1.zip\mako\runtime.py", line 839, in _render_context
    _exec_template(inherit, lclcontext, args=args, kwargs=kwargs)
  File "C:\Users\tamam\Documents\servo\components\style\properties\Mako-0.9.1.zip\mako\runtime.py", line 865, in _exec_template
    callable_(context, *args, **kwargs)
  File "C:/Users/tamam/Documents/servo/components/style/properties\properties.mako.rs", line 165, in render_body
    <%include file="/shorthand/position.mako.rs" />
  File "C:\Users\tamam\Documents\servo\components\style\properties\Mako-0.9.1.zip\mako\runtime.py", line 734, in _include_file
    callable_(ctx, **_kwargs_for_include(callable_, context._data, **kwargs))
  File "C:/Users/tamam/Documents/servo/components/style/properties/shorthand/position.mako.rs", line 187, in render_body
    <%helpers:shorthand name="place-self" sub_properties="align-self justify-self"
  File "C:/Users/tamam/Documents/servo/components/style/properties/helpers.mako.rs", line 502, in render_shorthand
    <%
  File "C:/Users/tamam/Documents/servo/components/style/properties\data.py", line 274, in declare_shorthand
    sub_properties = [self.longhands_by_name[s] for s in sub_properties]
KeyError: u'justify-self'

[Manishearth]

No, it has to build. Mark the shorthand with product="gecko", and test that it builds with ./mach build-geckolib

[tamamu]

I already marked with that, and also building has succeeded.
The problem is the testing.