Negative value on flex-basis should be rejected

[upsuper]

Relevant code:

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

Lines 200 to 207 in 8b8eb5e

	${helpers.predefined_type("flex-basis",
	"LengthOrPercentageOrAuto" if product == "gecko" else
	"LengthOrPercentageOrAutoOrContent",
	"computed::LengthOrPercentageOrAuto::Auto" if product == "gecko" else
	"computed::LengthOrPercentageOrAutoOrContent::Auto",
	spec="https://drafts.csswg.org/css-flexbox/#flex-basis-property",
	extra_prefixes="webkit",
	animatable=True if product == "gecko" else False)}

Should be an easy one although I cannot provide how to fix it without further investigation.

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

[upsuper]

cc @Manishearth

[canova]

This can be fixed by adding "parse_non_negative" and needs_context=False parameters to this helpers.predefined_type before the spec parameter. This will work fine in stylo build (can be built with ./mach build-geckolib) But will break servo build because LengthOrPercentageOrAutoOrContent doesn't have a parse_non_negative method. We need to add this function to enum like LengthOrPercentageOrAuto implementation except it will have additional content parsing.

[canova]

Oh, scratch that. It looks like LengthOrPercentageOrAutoOrContent's parse function is already rejecting negative values. Passing something like these parameters will solve the problem without changing parse implementations: "parse_non_negative" if product == "gecko" else "parse" needs_context=False if product == "gecko" else True,

[wafflespeanut]

Instead of going for a lot of if ... else blocks inside the definition, let's have two definitions - one for each servo and gecko.

[mrkalling]

Hi, I like to work on this @highfive: assign me

[highfive]

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

[wafflespeanut]

@mrkalling Did you start working on this?

[mrkalling]

I have looked at it but I don't understand what is the problem. When I test to set flex-basis to a negative number servo seems to ignore it which is what we want, right?

[upsuper]

It looks like Servo's LengthOrPercentageOrAutoOrContent is already only parsing non-negatives, while the Gecko port uses LengthOrPercentageOrAuto which does normal parsing. So it is a stylo-only issue. We probably need to use LengthOrPercentageOrAuto::parse_non_negative for that.

[upsuper]

Probably adding a new line

"parse_non_negative" if product == "gecko" else "parse",

would be enough.

[wafflespeanut]

@mrkalling ping?

[emilio]

This is Gecko only, and I'm helping out someone working on this.