Bug 96844 - Sign in front of keyframe selector causes the subsequent ruleset to be ignored
: Sign in front of keyframe selector causes the subsequent ruleset to be ignored
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: CSS
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Glenn Adams
:
Depends on:
Blocks: 93414
  Show dependency treegraph
 
Reported: 2012-09-14 17:28 PDT by Simon Fraser (smfr)
Modified: 2012-09-30 23:13 PDT (History)
11 users (show)

See Also:


Attachments
Testcase (416 bytes, text/html)
2012-09-14 17:29 PDT, Simon Fraser (smfr)
no flags Details
Better test case. (1.08 KB, text/html)
2012-09-28 19:57 PDT, Glenn Adams
no flags Details
Patch (4.43 KB, patch)
2012-09-29 19:55 PDT, Glenn Adams
no flags Details | Formatted Diff | Diff
Patch (4.40 KB, patch)
2012-09-30 00:23 PDT, Glenn Adams
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2012-09-14 17:28:56 PDT
A keyframes rule like:

@-webkit-keyframes fade {
    -10% { background-color: blue; }
    100% { background-color: green; }
}

will cause stylesheet parsing to abort.

I don't understand why, since the 'key' production is using PERCENTAGE, which is a <number>, which should accept signs.

I think this is a problem in the lexer code.
Comment 1 Simon Fraser (smfr) 2012-09-14 17:29:13 PDT
Created attachment 164254 [details]
Testcase
Comment 2 Simon Fraser (smfr) 2012-09-14 17:45:09 PDT
Looks like dashes are only handled in NthChildMode. Maybe we need a mode for keyframe selectors.
Comment 3 Takashi Sakamoto 2012-09-18 00:40:38 PDT
According to the spec: http://dev.w3.org/csswg/css3-animations/#keyframes,
"If a keyframe selector specifies negative percentage values or values higher than 100%, then the keyframe will be ignored."

So I think, the attached TestCase does not work.

Best regards,
Takashi Sakamoto

(In reply to comment #0)
> A keyframes rule like:
> 
> @-webkit-keyframes fade {
>     -10% { background-color: blue; }
>     100% { background-color: green; }
> }
> 
> will cause stylesheet parsing to abort.
> 
> I don't understand why, since the 'key' production is using PERCENTAGE, which is a <number>, which should accept signs.
> 
> I think this is a problem in the lexer code.
Comment 4 Simon Fraser (smfr) 2012-09-18 08:39:31 PDT
(In reply to comment #3)
> According to the spec: http://dev.w3.org/csswg/css3-animations/#keyframes,
> "If a keyframe selector specifies negative percentage values or values higher than 100%, then the keyframe will be ignored."

That's not the point. A minus sign in front of the keyframe selector causes the entire rest of the stylesheet to be ignored. That's obviously wrong.
Comment 5 Glenn Adams 2012-09-28 19:57:48 PDT
Created attachment 166351 [details]
Better test case.

This test case shows that it is the next (ruleset) statement that is being skipped, and not the entire remaining stylesheet.
Comment 6 Glenn Adams 2012-09-29 19:55:17 PDT
Created attachment 166375 [details]
Patch
Comment 7 Simon Fraser (smfr) 2012-09-29 22:02:43 PDT
Comment on attachment 166375 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=166375&action=review

> Source/WebCore/ChangeLog:8
> +        Upon encountering negative percentage, ignore only subsequent block.

I don't think we should ignore the block at the parsing level. This is totally valid CSS; we just need to handle keyframes outside of 0-100% in the code.
Comment 8 Glenn Adams 2012-09-30 00:23:12 PDT
Created attachment 166381 [details]
Patch
Comment 9 Glenn Adams 2012-09-30 03:29:48 PDT
Comment on attachment 166381 [details]
Patch

address comment #7
Comment 10 WebKit Review Bot 2012-09-30 23:13:41 PDT
Comment on attachment 166381 [details]
Patch

Clearing flags on attachment: 166381

Committed r130007: <http://trac.webkit.org/changeset/130007>
Comment 11 WebKit Review Bot 2012-09-30 23:13:45 PDT
All reviewed patches have been landed.  Closing bug.