Bug 897094 - Mismatched parenthesis in some CSS functions do not prevent parsing of subsequent CSS properties. r=heycam
authorMax Vujovic <mvujovic@adobe.com>
Tue, 30 Jul 2013 15:38:01 -0400
changeset 140604 80be1fb00dc5ec060da07da46887f5cb5c6b91a5
parent 140603 b03886bb3cecc5f52dc0df1e5e3dda866d0b78b5
child 140605 0eccf52bf215128b8ba14b4c3d43200ad21a286d
push id25033
push userryanvm@gmail.com
push dateWed, 31 Jul 2013 01:29:46 +0000
treeherdermozilla-central@c2b375f3a909 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs897094
milestone25.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 897094 - Mismatched parenthesis in some CSS functions do not prevent parsing of subsequent CSS properties. r=heycam
layout/style/nsCSSParser.cpp
layout/style/test/Makefile.in
layout/style/test/test_css_function_mismatched_parenthesis.html
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -9661,49 +9661,57 @@ CSSParserImpl::ParseTextOverflow(nsCSSVa
     aValue = left;
   }
   return true;
 }
 
 ///////////////////////////////////////////////////////
 // transform Parsing Implementation
 
-/* Reads a function list of arguments.  Do not call this function
- * directly; it's mean to be caled from ParseFunction.
+/* Reads a function list of arguments and consumes the closing parenthesis.
+ * Do not call this function directly; it's meant to be called from
+ * ParseFunction.
  */
 bool
 CSSParserImpl::ParseFunctionInternals(const int32_t aVariantMask[],
                                       int32_t aVariantMaskAll,
                                       uint16_t aMinElems,
                                       uint16_t aMaxElems,
                                       InfallibleTArray<nsCSSValue> &aOutput)
 {
   NS_ASSERTION((aVariantMask && !aVariantMaskAll) ||
                (!aVariantMask && aVariantMaskAll),
                "only one of the two variant mask parameters can be set");
 
   for (uint16_t index = 0; index < aMaxElems; ++index) {
     nsCSSValue newValue;
     int32_t m = aVariantMaskAll ? aVariantMaskAll : aVariantMask[index];
     if (!ParseVariant(newValue, m, nullptr)) {
-      return false;
+      break;
     }
 
     aOutput.AppendElement(newValue);
 
-    // See whether to continue or whether to look for end of function.
-    if (!ExpectSymbol(',', true)) {
-      // We need to read the closing parenthesis, and also must take care
-      // that we haven't read too few symbols.
-      return ExpectSymbol(')', true) && (index + 1) >= aMinElems;
-    }
-  }
-
-  // If we're here, we finished looping without hitting the end, so we read too
-  // many elements.
+    if (ExpectSymbol(',', true)) {
+      // Move on to the next argument if we see a comma.
+      continue;
+    }
+
+    if (ExpectSymbol(')', true)) {
+      // Make sure we've read enough symbols if we see a closing parenthesis.
+      return (index + 1) >= aMinElems;
+    }
+
+    // Only a comma or a closing parenthesis is valid after an argument.
+    break;
+  }
+
+  // If we're here, we've hit an error without seeing a closing parenthesis or
+  // we've read too many elements without seeing a closing parenthesis.
+  SkipUntil(')');
   return false;
 }
 
 /* Parses a function [ input of the form (a [, b]*) ] and stores it
  * as an nsCSSValue that holds a function of the form
  * function-name arg1 arg2 ... argN
  *
  * On error, the return value is false.
--- a/layout/style/test/Makefile.in
+++ b/layout/style/test/Makefile.in
@@ -92,16 +92,17 @@ MOCHITEST_FILES =	test_acid3_test46.html
 		test_compute_data_with_start_struct.html \
 		test_computed_style.html \
 		test_computed_style_no_pseudo.html \
 		test_condition_text.html \
 		test_condition_text_assignment.html \
 		test_default_computed_style.html \
 		test_css_cross_domain.html \
 		test_css_eof_handling.html \
+		test_css_function_mismatched_parenthesis.html \
 		test_css_supports.html \
 		test_default_bidi_css.html \
 		test_descriptor_storage.html \
 		test_descriptor_syntax_errors.html \
 		test_dont_use_document_colors.html \
 		file_flexbox_align_self_auto.html \
 		test_flexbox_align_self_auto.html \
 		file_flexbox_child_display_values.xhtml \
new file mode 100644
--- /dev/null
+++ b/layout/style/test/test_css_function_mismatched_parenthesis.html
@@ -0,0 +1,63 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=897094
+
+This test verifies that:
+(1) Mismatched parentheses in a CSS function prevent parsing of subsequent CSS
+properties.
+(2) Properly matched parentheses do not prevent parsing of subsequent CSS
+properties.
+-->
+<head>
+  <title>Test for Bug 897094</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=897094">Mozilla Bug 897094</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  <div id="target"></div>
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 897094 **/
+function check_parens(declaration, parens_are_balanced)
+{
+  var element = document.getElementById("target");
+  element.setAttribute("style",
+                       "background-color: " + (parens_are_balanced ? "red" : "green") + "; " +
+                       declaration + "; " +
+                       "background-color: " + (parens_are_balanced ? "green" : "red") + "; ");
+  var resultColor = element.style.getPropertyValue("background-color");
+  is(resultColor, "green", "parenthesis balancing within " + declaration);
+}
+
+check_parens("transform: scale()", true);
+check_parens("transform: scale(", false);
+check_parens("transform: scale(,)", true);
+check_parens("transform: scale(,", false);
+check_parens("transform: scale(1)", true);
+check_parens("transform: scale(1", false);
+check_parens("transform: scale(1,)", true);
+check_parens("transform: scale(1,", false);
+check_parens("transform: scale(1,1)", true);
+check_parens("transform: scale(1,1", false);
+check_parens("transform: scale(1,1,)", true);
+check_parens("transform: scale(1,1,", false);
+check_parens("transform: scale(1,1,1)", true);
+check_parens("transform: scale(1,1,1", false);
+check_parens("transform: scale(1,1,1,)", true);
+check_parens("transform: scale(1,1,1,", false);
+check_parens("transform: scale(1px)", true);
+check_parens("transform: scale(1px", false);
+check_parens("transform: scale(1px,)", true);
+check_parens("transform: scale(1px,", false);
+
+</script>
+</pre>
+</body>
+</html>