Implement selector contextualization for keyframe rules

[mgreter]

Fixes #947

[am11]

👍 🎉

[xzyfer]

I don't think this is correct. @keyframes do not have selectors, they have a value/label that is a string.

We should just resolve the interpolate value.

[am11]

@xzyfer, nice catch. But the keyframes-selector can be from and to as well? Would it make sense to keep a whitelist to qualify for the keyframes-selector (in case CSS brings more terms to the mix in future)? ["{numeric_value}%", "from", "to"]
Ref: http://www.w3schools.com/cssref/css3_pr_animation-keyframes.asp

[mechanicalduck]

Specs: sass/sass-spec#279

[mgreter]

@xzyfer seems odd, since IMO the only place where this value is set is in Expand(Ruleset*):

Statement* Expand::operator()(Ruleset* r) {
...
  Keyframe_Rule* k = new (ctx.mem) Keyframe_Rule ...
  k->selector(r->selector()->perform(contextualize->with(0, env, backtrace)))

So the initial value seems to be a ruleset selector? But maybe you're right. It just looked like an oversight to me, and I thought that this would actually always be a selector ...

[xzyfer]

@mgreter it's because the parse is rather dumb about keyframes. It's on my todo list to improve.

@am11 I'd rather not for now because there's a lot of combinators to account for and it may change in the future.

[mgreter]

@xzyfer What do you think about this PR in the meantime? Just wonder if it's better than nothing, regarding that it doesn't seem to break any existing spec tests? Or should I close it then?

[xzyfer]

@mgreter I say leave it for now. I'll take a pass doing naive interpolation expanding. If it works as expected I'll use that otherwise I'll ship this.

[xzyfer]

I'll take a look at this over the weekend. I think it's rather straight forward.

[coveralls]

Coverage increased (+0.03%) to 80.42% when pulling 621f1e7 on mgreter:bugfix/issue_947 into 8e7a294 on sass:master.

[mgreter]

@xzyfer do you think you can come up with a better solution in a reasonable timeframe for the 3.2.0 release? IMO I'd rather include this PR as is than not including a fix for the reported issue. Would really like to include this fix in the next beta! Or do you see any obvious regressions in the changes?

[coveralls]

Coverage increased (+0.0%) to 80.07% when pulling 47dffc5 on mgreter:bugfix/issue_947 into efeace5 on sass:master.

[mgreter]

Merged it beside the concerns given by @xzyfer. IMO this seems to be an improvement and we can add a more precise implementation later. Just want to have this included in the next release!