Reviewers: MikeSamuel,

Description:
<style>p { animation-name: foo; }</style>
throws a SomethingWidgy error in es53.

That's very unfriendly, because the guest-code developer ends up
getting a "Server Error" message with no diagnostic details,
and it's not at all obvious what to do next to fix it.

Since css rules are loaded dynamically at run-time, we can't really
know at compile-time what all the rule atoms are, so encountering an
unrecognized atom doesn't really merit a SomethingWidgy error.

Instead, this CL just fails the match, which makes CSS validator
emit a warning about a bad value, drop the value, and proceed.
And the guest-code developer can find the warning easily.

Please review this at https://codereview.appspot.com/12837046/

Affected files:
  M     src/com/google/caja/plugin/CssValidator.java
  M     tests/com/google/caja/plugin/CssValidatorTest.java


Index: src/com/google/caja/plugin/CssValidator.java
===================================================================
--- src/com/google/caja/plugin/CssValidator.java        (revision 5559)
+++ src/com/google/caja/plugin/CssValidator.java        (working copy)
@@ -1339,9 +1339,13 @@
       }
       candidate.match(term, CssPropertyPartType.IDENT, propertyName);
       ++candidate.exprIdx;
+    } else if ("global-name".equals(symbolName)) {
+      // <global-name> is used by @keyframes and animation-name. Since es53
+      // doesn't support @keyframes, we just fail the match.
+      return false;
     } else {
-      throw new SomethingWidgyHappenedError(
-          "unhandled symbol " + sig.symbolName + "\n" + dump(atom));
+      // Unrecognized atom: fail the match.
+      return false;
     }

     if (null != constraints
Index: tests/com/google/caja/plugin/CssValidatorTest.java
===================================================================
--- tests/com/google/caja/plugin/CssValidatorTest.java  (revision 5559)
+++ tests/com/google/caja/plugin/CssValidatorTest.java  (working copy)
@@ -39,6 +39,18 @@
  */
 public final class CssValidatorTest extends CajaTestCase {

+  public final void testGlobalName() throws Exception {
+    runTest("a { animation-name: foo; }",
+        "StyleSheet\n"
+        + "  RuleSet\n"
+        + "    Selector\n"
+        + "      SimpleSelector\n"
+        + "        IdentLiteral : a\n"
+        + "    EmptyDeclaration\n",
+        "WARNING: css property animation-name has bad value: ==>foo<==");
+  }
+
+
   public final void testVendorPrefix() throws Exception {
     runTest("a { -moz-animation-duration: 1s }",
         "StyleSheet\n"


--

--- You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to