Implement parsing webkit-prefixed gradient functions

[upsuper]

This is defined in the Compatibility Standard: https://compat.spec.whatwg.org/#css-image-type

The corresponding syntax is in https://www.w3.org/TR/2011/WD-css3-images-20110217/#gradients

[karan1276]

I would really like to work on this. If some one can give some extended mentoring on this it will be great. Some links to similar properties will be super awesome.

[upsuper]

This is not about property, but about another syntax for <image> values which is accepted by various properties.

The relevant code is https://github.com/servo/servo/blob/master/components/style/values/specified/image.rs#L102-L155

You would need to add a bunch of prefixed gradient functions there which are similar but have tricky minor difference to the unprefixed ones.

[karan1276]

Humm right, but for example in the specs they say -webkit-linear-gradient() must be treated as alias of linear-gradient so can't we just say sth like:

        let (gradient_kind, stops) = match_ignore_ascii_case! { try!(input.expect_function()),
            "linear-gradient" | "-webkit-linear-gradient"=> {      <--- just OR them with respective functions
                try!(input.parse_nested_block(|input| {
                        let kind = try!(GradientKind::parse_linear(context, input));
                        let stops = try!(input.parse_comma_separated(|i| ColorStop::parse(context, i)));

[upsuper]

No, because it says, e.g.

-webkit-linear-gradient() must be treated as an alias of linear-gradient as defined in [css3-images-20110217].

The current version of linear-gradient is based on what is defined in the latest css-images-3 spec, which has a different syntax than what was defined in css3-images-20110217.

[karan1276]

aah, okay

[karan1276]

Also, i just noticed linear-gradient and radial-gradient have the same code block, same for repeating-*. Would it be a good idea to merge them to one case clause. Or was it done intentionally?

[upsuper]

They are different... linear-gradients use parse_linear and radial-gradients use parse_radial. If you are able to merge them, it is probably also fine.

[karan1276]

ah! didn't see that. If they were same it made sense . I guess there is no real advantage merging them now

[nox]

Does serialisation need to preserve the prefixes?

[upsuper]

We probably need to, because there seems to be some functional difference?

It seems to me Gecko preserves the prefix in computed style (via nsStyleGradient::mLegacySyntax). But it only serializes things to -moz- prefix, even if the source is -webkit- prefixed.

If we are not going to implement -moz- thing, we probably should drop that part in Gecko side in advance, and only serialize things to -webkit- prefix there. And it may also be worth checking what other browsers do for serialization prefixed gradient functions. If they don't preserve prefixes, we probably can just do some parse-time conversion, and then drop prefixes as well.

[nox]

We probably need to, because there seems to be some functional difference?

The differences are only in parsing AFAICT: the token to isn't present in -webkit-linear-gradient, and the center of the gradient comes at the beginning of -webkit-radial-gradient, again without a to prefix.

As a side-note, didn't we forget to handle the <zero> part from https://drafts.csswg.org/css-images/#linear-gradient-syntax?

[upsuper]

The <zero> part is very new, probably something just resolved several days ago. IIRC, CSSWG made unitless zero usable for degree, but then they found there could be ambugity, so they decided to put that quirk explicitly in some places, and gradient is one of them.

[upsuper]

So Servo implements the unitless-zero-as-zero-degree, and Gecko never has that. But Servo should probably remove that support, and do what the current spec says instead.

[upsuper]

The differences are only in parsing AFAICT: the token to isn't present in -webkit-linear-gradient, and the center of the gradient comes at the beginning of -webkit-radial-gradient, again without a to prefix.

Probably check with Blink/WebKit and see if they do that? @miketaylr, could you help checking this? Also this may be something worth putting into the Compat spec as well.

[nox]

@upsuper Should the prefix also be preserved for serialisation of computed values?

[nox]

<div style="background-image: -webkit-linear-gradient(top, yellow, blue);">
<script>console.log(document.querySelector('div').style.backgroundImage)</script>

Unsurprisingly, both Safari and Chrome serialise that as the prefixed thing. No idea about Edge.

[upsuper]

Serialization of computation is done in Gecko side for Stylo, so probably doesn't really matter here, but we probably want to be consistent to specified value here?

I guess we would need to fix the Gecko side code to prefer using webkit-prefix first...

[nox]

@upsuper I already have a patch ready for parsing and serialisation of both specified and computed values, do you want me to make a PR of that, so that a try run can be done on Gecko's side, maybe?

[nox]

Err, I missed the operative part: that patch only handle linear gradients for now.

[upsuper]

I probably should figure out a plan for bug 1337655 first (which I totally forgot before). There may not be a significant difference on test result without having that bug resolved, so probably it isn't worth making a separate PR if you haven't finished all the gradient stuff.

[nox]

I'm on PTO tomorrow and Monday is a national holiday, so I'll file it anyway. :)

[nox]

@upsuper In #16511 I implement parsing of -webkit-radial-gradient. Now AFAIK all we need is for Gecko to drop -moz- and use -webkit-.

[upsuper]

According to bug 1241623 comment 8, we also need to implement the 2008 version of -webkit-gradient syntax in addition to -webkit-{linear,radial}-gradient, and there is no spec for that.

Also, -webkit-gradient seems to be the most expressive syntax among all, and Gecko only partially support its expressiveness (via some parse-time conversion). Servo may or may not want to follow the exactly same way as Gecko.