Fix the issue with parsing @keyframes. #8480

Merged
merged 3 commits into from Aug 5, 2014

Conversation

Projects
None yet
2 participants
Contributor

RaymondLim commented Jul 21, 2014

Also add a test case and update some test files to capture the issue with @Keyframes parsing.

This fixes #7494 and #8463.

@RaymondLim RaymondLim Fix the issue with parsing @keyframes.
Also add a test case and update some test files to capture the issue with @keyframes parsing.
fd648c0

redmunds self-assigned this Jul 31, 2014

@redmunds redmunds and 1 other commented on an outdated diff Jul 31, 2014

src/language/CSSUtils.js
@@ -923,11 +923,20 @@ define(function (require, exports, module) {
}
} else {
+ var unmatchedBraces = 0;
@redmunds

redmunds Jul 31, 2014

Contributor

Nice fix. This might be needed somewhere else, so this cold be a function called something like _skipToClosingBracket(char). It only needs to handle braces for now, but it would be easy to add support for other brackets, if necessary.

@RaymondLim

RaymondLim Aug 4, 2014

Contributor

Nice suggestion! But I don't have the char param in my function since we don't need to support other brackets yet.

Contributor

redmunds commented Jul 31, 2014

This improves the results of recipe in #7494, but it's still weird that CSSInlineEditor opens for <100%> tag.

Also, when I press "New Rule" button, I get this rule:

100% {

}

Seems like that should be an error instead.

Contributor

redmunds commented Jul 31, 2014

Done with review.

Contributor

RaymondLim commented Aug 4, 2014

I agree that <100%> tag is weird for CSSInlineEditor. But that is also true for any non standard xml tags. If you think we should NOT open CSSInlineEditor for unknown tag, then you should log a new issue.

Contributor

RaymondLim commented Aug 4, 2014

Changes are pushed. Ready for re-review.

Contributor

redmunds commented Aug 5, 2014

In response to this comment, you are allowed to create new tags in HTML and then create CSS rules for them. But HTML Tag Syntax only allows alphanumeric ASCII characters, which are "uppercase ASCII letters, lowercase ASCII letters, or ASCII digits". It would be easy to test for these in the CSSInlineEditor.

Note that we may also need to allow for : for namespacing.

Contributor

RaymondLim commented Aug 5, 2014

@redmunds OK, I replaced with from and to and use to tag in my test case. But it does not change the fact that our CSS inline editor opens up on an unknown tag and allows user to add a new rule with the unknown tag.

Contributor

redmunds commented Aug 5, 2014

Merging.

@redmunds redmunds added a commit that referenced this pull request Aug 5, 2014

@redmunds redmunds Merge pull request #8480 from adobe/rlim/at-keyframes-issue
Fix the issue with parsing @keyframes.
6e5015a

@redmunds redmunds merged commit 6e5015a into master Aug 5, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

redmunds deleted the rlim/at-keyframes-issue branch Aug 5, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment