Add gecko glue for font-feature-settings property

[upsuper]

Servo has had parsing and serialization code for font-feature-settings property, but we need to add glue to make it usable in Gecko.

The existing parsing and serialization code is

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

Lines 634 to 742 in d309782

	<%helpers:longhand name="font-feature-settings" products="none" animatable="False" extra_prefixes="moz"
	spec="https://drafts.csswg.org/css-fonts/#propdef-font-feature-settings">
	use std::fmt;
	use style_traits::ToCss;
	use values::HasViewportPercentage;
	use values::computed::ComputedValueAsSpecified;
	pub use self::computed_value::T as SpecifiedValue;
	impl ComputedValueAsSpecified for SpecifiedValue {}
	no_viewport_percentage!(SpecifiedValue);
	pub mod computed_value {
	use cssparser::Parser;
	use parser::{Parse, ParserContext};
	use std::fmt;
	use style_traits::ToCss;
	#[derive(Debug, Clone, PartialEq)]
	#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
	pub enum T {
	Normal,
	Tag(Vec<FeatureTagValue>)
	}
	#[derive(Debug, Clone, PartialEq)]
	#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
	pub struct FeatureTagValue {
	pub tag: String,
	pub value: i32
	}
	impl ToCss for T {
	fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
	match *self {
	T::Normal => dest.write_str("normal"),
	T::Tag(ref ftvs) => {
	let mut iter = ftvs.iter();
	// handle head element
	try!(iter.next().unwrap().to_css(dest));
	// handle tail, precede each with a delimiter
	for ftv in iter {
	try!(dest.write_str(", "));
	try!(ftv.to_css(dest));
	}
	Ok(())
	}
	}
	}
	}
	impl ToCss for FeatureTagValue {
	fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
	match self.value {
	1 => write!(dest, "\"{}\"", self.tag),
	0 => write!(dest, "\"{}\" off", self.tag),
	x => write!(dest, "\"{}\" {}", self.tag, x)
	}
	}
	}
	impl Parse for FeatureTagValue {
	/// https://www.w3.org/TR/css-fonts-3/#propdef-font-feature-settings
	/// <string> [ on | off | <integer> ]
	fn parse(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
	let tag = try!(input.expect_string());
	// allowed strings of length 4 containing chars: <U+20, U+7E>
	if tag.len() != 4 ||
	tag.chars().any(|c| c < ' ' || c > '~')
	{
	return Err(())
	}
	if let Ok(value) = input.try(|input| input.expect_integer()) {
	// handle integer, throw if it is negative
	if value >= 0 {
	Ok(FeatureTagValue { tag: tag.into_owned(), value: value })
	} else {
	Err(())
	}
	} else if let Ok(_) = input.try(|input| input.expect_ident_matching("on")) {
	// on is an alias for '1'
	Ok(FeatureTagValue { tag: tag.into_owned(), value: 1 })
	} else if let Ok(_) = input.try(|input| input.expect_ident_matching("off")) {
	// off is an alias for '0'
	Ok(FeatureTagValue { tag: tag.into_owned(), value: 0 })
	} else {
	// empty value is an alias for '1'
	Ok(FeatureTagValue { tag:tag.into_owned(), value: 1 })
	}
	}
	}
	}
	#[inline]
	pub fn get_initial_value() -> computed_value::T {
	computed_value::T::Normal
	}
	/// normal | <feature-tag-value>#
	pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
	if input.try(|input| input.expect_ident_matching("normal")).is_ok() {
	Ok(computed_value::T::Normal)
	} else {
	input.parse_comma_separated(|i| computed_value::FeatureTagValue::parse(context, i))
	.map(computed_value::T::Tag)
	}
	}
	</%helpers:longhand>

The glue code should be added under

servo/components/style/properties/gecko.mako.rs

Lines 1165 to 1167 in d309782

	<%self:impl_trait style_struct_name="Font"
	skip_longhands="font-family font-size font-size-adjust font-weight font-synthesis -x-lang"
	skip_additionals="*">

It seems to me that the existing property code in font.mako.rs also needs some tweak. The feature tag can be represented as a u32, so we should not use String for it. Also in Gecko's style struct, gfxFontFeature::mTag is a u32 as well.

The target struct can be find in components/style/gecko_bindings/struct_debug.rs (GitHub doesn't display this file because it's too large). It should be nsFont::fontFeatureSettings. It is an nsTArray, but gfxFontFeature seems to be a POD type, so we can use nsTArray::set_len_pod for initializing its length.

[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. 😄

[ghost]

I'd love to sink my teeth into this. It will be my first foray into an open source project so I hope it wouldn't be too troublesome if I asked some questions every now and then. Thanks!

[jdm]

Asking questions is encouraged!

[wafflespeanut]

@WholeGrainGoats Have you made any progress? Need help with this?

[ghost]

@wafflespeanut Yes, sorry for the hiatus. I'm trying to find out what I should name the functions that I write. Like if there is a standard you have set up for things like this.

[wafflespeanut]

Oh, you'd be writing two functions set_font_feature_settings and copy_font_feature_settings which will change the values of gecko fields (one will set the values from servo struct into gecko struct, another will simply copy the values). The nearby set/copy methods in the file might give you an idea on how to approach this. Feel free to ping us if you're still unclear.

[ghost]

@wafflespeanut I've looked over the code but I see that the T enum from here has a value Normal. I looked at the MDN link here and see that the specification calls to insert default values. My question is what would those values be, or perhaps what should be done when that case is encountered? What should I do with fontFeatureSettings in that case?

[wafflespeanut]

Good question! Looking at the gecko code, it seems like an empty nsTArray (specifically, a null pointer) represents the value normal (I think). So, let's truncate the array when we see a normal value.

[ghost]

@wafflespeanut I've written the two functions you've asked for, converted the value to be a u32 for feature tag, and added the necessary changes to the parsing code to use u32. I'm having some difficulty testing this code, however. I inserted a print statement, changed one of the files to include font-feature-settings in the css, and it appears that the function is not being called. It could just be my confusion but am I supposed to personally test this code? If so, could you offer some assistance with this, or point me to some info on how this should be done?

[wafflespeanut]

Nah, you won't be able to test it unless you have a Stylo build (sadly). Feel free to make a PR, and we'll test this in gecko's try server :)