Implement parsing and serialization of font-variant-{alternates,east-asian,ligatures,numeric}

[upsuper]

Their definitions can be found in CSS Font spec:

• font-variant-alternates
• font-variant-east-asian
• font-variant-ligatures
• font-variant-numeric

They should be added into https://github.com/servo/servo/blob/master/components/style/properties/longhand/font.mako.rs

And all of them should be added as subproperties of font and font-variant shorthands in https://github.com/servo/servo/blob/master/components/style/properties/shorthand/font.mako.rs

It might not be too hard, but the work here isn't trivial either.

[SebastinSanty]

@highfive: assign me

[highfive]

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

[KiChjang]

@SebastinSanty In general, it's discouraged to reserve issues that a person is not working on in the immediate future. Please finish the other issue first before claiming this one.

[SebastinSanty]

@KiChjang Sorry for that. Thought of taking the easier one first.

[abhiQmar]

@highfive: assign me

[highfive]

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

[abhiQmar]

Sorry, haven't been able to find time to complete this, anyone is free to take this up.

[highfive]

cc @emilio

[streichgeorg]

@highfive: assign me

[highfive]

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

[streichgeorg]

I managed to implement the longhands but when I try to add them to the shorthands the tests
properties::scaffolding::properties_list_json and properties::serialization::shorthand_serialization::font::font_should_serialize_all_available_properties fail. What do I have to do to fix this?

[jdm]

What are the failures?

[streichgeorg]

	thread 'properties::serialization::shorthand_serialization::font::font_should_serialize_all_available_properties' panicked at 'assertion failed: `(left == right)` (left: `"font-style: italic; font-variant: normal; font-weight: bolder; font-stretch: expanded; font-size: 4px; line-height: 3; font-family: serif; font-size-adjust: none; font-kerning: auto; font-variant-caps: normal; font-variant-position: normal; font-language-override: normal;"`, right: `"font: italic normal bolder expanded 4px/3 serif;"`)', /home/computer/Documents/servo/tests/unit/style/properties/serialization.rs:659
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: style_tests::properties::serialization::shorthand_serialization::font::font_should_serialize_all_available_properties
             at ./properties/serialization.rs:659
   1: <F as test::FnBox<T>>::call_box
             at /checkout/src/libtest/lib.rs:1368
             at /checkout/src/libtest/lib.rs:140
   2: std::panicking::try::do_call
             at /checkout/src/libtest/lib.rs:1314
             at /checkout/src/libstd/panic.rs:296
             at /checkout/src/libstd/panicking.rs:454
   3: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
   4: std::panicking::try::do_call
             at /checkout/src/libstd/panicking.rs:433
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libtest/lib.rs:1313
             at /checkout/src/libstd/panic.rs:296
             at /checkout/src/libstd/panicking.rs:454
   5: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
   6: <F as alloc::boxed::FnBox<A>>::call_box
             at /checkout/src/libstd/panicking.rs:433
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libstd/thread/mod.rs:360
             at /checkout/src/liballoc/boxed.rs:640
   7: std::sys::imp::thread::Thread::new::thread_start
             at /checkout/src/liballoc/boxed.rs:650
             at /checkout/src/libstd/sys_common/thread.rs:21
             at /checkout/src/libstd/sys/unix/thread.rs:84
   8: start_thread
   9: clone

---- properties::scaffolding::properties_list_json stdout ----
	thread 'properties::scaffolding::properties_list_json' panicked at 'assertion failed: status.success()', /home/computer/Documents/servo/tests/unit/style/properties/scaffolding.rs:27
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: style_tests::properties::scaffolding::properties_list_json
             at ./properties/scaffolding.rs:27
   1: <F as test::FnBox<T>>::call_box
             at /checkout/src/libtest/lib.rs:1368
             at /checkout/src/libtest/lib.rs:140
   2: std::panicking::try::do_call
             at /checkout/src/libtest/lib.rs:1314
             at /checkout/src/libstd/panic.rs:296
             at /checkout/src/libstd/panicking.rs:454
   3: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
   4: std::panicking::try::do_call
             at /checkout/src/libstd/panicking.rs:433
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libtest/lib.rs:1313
             at /checkout/src/libstd/panic.rs:296
             at /checkout/src/libstd/panicking.rs:454
   5: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
   6: <F as alloc::boxed::FnBox<A>>::call_box
             at /checkout/src/libstd/panicking.rs:433
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libstd/thread/mod.rs:360
             at /checkout/src/liballoc/boxed.rs:640
   7: std::sys::imp::thread::Thread::new::thread_start
             at /checkout/src/liballoc/boxed.rs:650
             at /checkout/src/libstd/sys_common/thread.rs:21
             at /checkout/src/libstd/sys/unix/thread.rs:84
   8: start_thread
   9: clone```

I hope this helps.

[jdm]

The first test is verifying that if all of the known font longhands are present, the serialized version is the font shorthand. https://dxr.mozilla.org/servo/rev/065f50014f321466c979120967da720e88ae2b29/tests/unit/style/properties/serialization.rs#640-652 should be updated to include the new properties that the shorthand now checks for.

The second test looks like it might be failing to execute python correctly. It will be hard to provide any advice about that without seeing the changes you've made. You could check whether you see that failure on an unmodified version of master when running ./mach test-unit -p style.

[upsuper]

Sorry that it seems @hiikezoe has had some patches for this in bug 1354876. We will probably just take that and close this issue.

Thanks for your work.

[hiikezoe]

In bug 1354876, I did not parse font-variant-alternates properly, so there are still remaining work. And I am hoping @streichgeorg fixes the remaining issue.
Thanks!

[nox]

Are you still working on this, @streichgeorg?

[streichgeorg]

Oh I'm sorry, I kinda forgot this and also didn't have any time in the past week. But I should be able to finish this in the following days, I just need to do some minor stuff.

[hiikezoe]

That's good to know. Thank you!

[upsuper]

@hiikezoe I saw you wrote a FIXME in font.mako.rs referring this issue, but it is not clear to me what is left there. Could you clarify? It seems to me this issue can be considered fixed. Is there anything I'm missing?

[hiikezoe]

Some values (e.g. styleset) involve feature-value-name(s). I did not implement that part (Honestly I did miss that part completely). Whereas @streichgeorg tries to parse them [1] (Great!). This is the left issue.

[1] 72eddc0#diff-668b3a1bd0c2ed6b1cf89503c93ee9e8R1167

[upsuper]

That sounds like part of @font-feature-values support which @chenpighead is currently working on in bug 1355721. If that's the only remaining part, I think we can close this issue.