Enforce token syntax for CssClassSet arguments

Committed: https://code.google.com/p/dart/source/detail?r=45265

Reverted: https://code.google.com/p/dart/source/detail?r=45266

R=terry@google.com

Review URL: https://codereview.chromium.org//1077203004

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@45301 260f80e4-7a28-3924-810f-c04153c831b5

sdk/lib/html/dart2js/html_dart2js.dart

@@ -36383,6 +36383,10 @@ abstract class CssClassSet implements Set<String> {
    * operation.
    *
    * If this corresponds to many elements, `null` is always returned.
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.  To toggle multiple classes, use
+   * [toggleAll].
    */
   bool toggle(String value, [bool shouldAdd]);
 
@@ -36397,19 +36401,26 @@ abstract class CssClassSet implements Set<String> {
    *
    * This is the Dart equivalent of jQuery's
    * [hasClass](http://api.jquery.com/hasClass/).
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.
    */
   bool contains(String value);
 
   /**
    * Add the class [value] to element.
    *
-   * This is the Dart equivalent of jQuery's
+   * [add] and [addAll] are the Dart equivalent of jQuery's
    * [addClass](http://api.jquery.com/addClass/).
    *
-   * If this corresponds to one element. Returns true if [value] was added to
-   * the set, otherwise false.
+   * If this CssClassSet corresponds to one element. Returns true if [value] was
+   * added to the set, otherwise false.
    *
    * If this corresponds to many elements, `null` is always returned.
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.  To add multiple classes use
+   * [addAll].
    */
   bool add(String value);
 
@@ -36417,24 +36428,34 @@ abstract class CssClassSet implements Set<String> {
    * Remove the class [value] from element, and return true on successful
    * removal.
    *
-   * This is the Dart equivalent of jQuery's
+   * [remove] and [removeAll] are the Dart equivalent of jQuery's
    * [removeClass](http://api.jquery.com/removeClass/).
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.  To remove multiple classes, use
+   * [removeAll].
    */
   bool remove(Object value);
 
   /**
    * Add all classes specified in [iterable] to element.
    *
-   * This is the Dart equivalent of jQuery's
+   * [add] and [addAll] are the Dart equivalent of jQuery's
    * [addClass](http://api.jquery.com/addClass/).
+   *
+   * Each element of [iterable] must be a valid 'token' representing a single
+   * class, i.e. a non-empty string containing no whitespace.
    */
   void addAll(Iterable<String> iterable);
 
   /**
    * Remove all classes specified in [iterable] from element.
    *
-   * This is the Dart equivalent of jQuery's
+   * [remove] and [removeAll] are the Dart equivalent of jQuery's
    * [removeClass](http://api.jquery.com/removeClass/).
+   *
+   * Each element of [iterable] must be a valid 'token' representing a single
+   * class, i.e. a non-empty string containing no whitespace.
    */
   void removeAll(Iterable<String> iterable);
 
@@ -36447,6 +36468,9 @@ abstract class CssClassSet implements Set<String> {
    * If [shouldAdd] is true, then we always add all the classes in [iterable]
    * element. If [shouldAdd] is false then we always remove all the classes in
    * [iterable] from the element.
+   *
+   * Each element of [iterable] must be a valid 'token' representing a single
+   * class, i.e. a non-empty string containing no whitespace.
    */
   void toggleAll(Iterable<String> iterable, [bool shouldAdd]);
 }

sdk/lib/html/dartium/html_dartium.dart

@@ -37305,6 +37305,10 @@ abstract class CssClassSet implements Set<String> {
    * operation.
    *
    * If this corresponds to many elements, `null` is always returned.
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.  To toggle multiple classes, use
+   * [toggleAll].
    */
   bool toggle(String value, [bool shouldAdd]);
 
@@ -37319,19 +37323,26 @@ abstract class CssClassSet implements Set<String> {
    *
    * This is the Dart equivalent of jQuery's
    * [hasClass](http://api.jquery.com/hasClass/).
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.
    */
   bool contains(String value);
 
   /**
    * Add the class [value] to element.
    *
-   * This is the Dart equivalent of jQuery's
+   * [add] and [addAll] are the Dart equivalent of jQuery's
    * [addClass](http://api.jquery.com/addClass/).
    *
-   * If this corresponds to one element. Returns true if [value] was added to
-   * the set, otherwise false.
+   * If this CssClassSet corresponds to one element. Returns true if [value] was
+   * added to the set, otherwise false.
    *
    * If this corresponds to many elements, `null` is always returned.
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.  To add multiple classes use
+   * [addAll].
    */
   bool add(String value);
 
@@ -37339,24 +37350,34 @@ abstract class CssClassSet implements Set<String> {
    * Remove the class [value] from element, and return true on successful
    * removal.
    *
-   * This is the Dart equivalent of jQuery's
+   * [remove] and [removeAll] are the Dart equivalent of jQuery's
    * [removeClass](http://api.jquery.com/removeClass/).
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.  To remove multiple classes, use
+   * [removeAll].
    */
   bool remove(Object value);
 
   /**
    * Add all classes specified in [iterable] to element.
    *
-   * This is the Dart equivalent of jQuery's
+   * [add] and [addAll] are the Dart equivalent of jQuery's
    * [addClass](http://api.jquery.com/addClass/).
+   *
+   * Each element of [iterable] must be a valid 'token' representing a single
+   * class, i.e. a non-empty string containing no whitespace.
    */
   void addAll(Iterable<String> iterable);
 
   /**
    * Remove all classes specified in [iterable] from element.
    *
-   * This is the Dart equivalent of jQuery's
+   * [remove] and [removeAll] are the Dart equivalent of jQuery's
    * [removeClass](http://api.jquery.com/removeClass/).
+   *
+   * Each element of [iterable] must be a valid 'token' representing a single
+   * class, i.e. a non-empty string containing no whitespace.
    */
   void removeAll(Iterable<String> iterable);
 
@@ -37369,6 +37390,9 @@ abstract class CssClassSet implements Set<String> {
    * If [shouldAdd] is true, then we always add all the classes in [iterable]
    * element. If [shouldAdd] is false then we always remove all the classes in
    * [iterable] from the element.
+   *
+   * Each element of [iterable] must be a valid 'token' representing a single
+   * class, i.e. a non-empty string containing no whitespace.
    */
   void toggleAll(Iterable<String> iterable, [bool shouldAdd]);
 }

sdk/lib/html/html_common/css_class_set.dart

@@ -6,6 +6,13 @@ part of html_common;
 
 abstract class CssClassSetImpl implements CssClassSet {
 
+  final RegExp _validTokenRE = new RegExp(r'^\S+$');
+
+  String _validateToken(String value) {
+    if (_validTokenRE.hasMatch(value)) return value;
+    throw new ArgumentError.value(value, 'value', 'Not a valid class token');
+  }
+
   String toString() {
     return readClasses().join(' ');
   }
@@ -18,6 +25,7 @@ abstract class CssClassSetImpl implements CssClassSet {
    * [shouldAdd] is false then we always remove [value] from the element.
    */
   bool toggle(String value, [bool shouldAdd]) {
+    _validateToken(value);
     Set<String> s = readClasses();
     bool result = false;
     if (shouldAdd == null) shouldAdd = !s.contains(value);
@@ -81,7 +89,11 @@ abstract class CssClassSetImpl implements CssClassSet {
    * This is the Dart equivalent of jQuery's
    * [hasClass](http://api.jquery.com/hasClass/).
    */
-  bool contains(Object value) => readClasses().contains(value);
+  bool contains(Object value) {
+    if (value is! String) return false;
+    _validateToken(value);
+    return readClasses().contains(value);
+  }
 
   /** Lookup from the Set interface. Not interesting for a String set. */
   String lookup(Object value) => contains(value) ? value : null;
@@ -93,6 +105,7 @@ abstract class CssClassSetImpl implements CssClassSet {
    * [addClass](http://api.jquery.com/addClass/).
    */
   bool add(String value) {
+    _validateToken(value);
     // TODO - figure out if we need to do any validation here
     // or if the browser natively does enough.
     return modify((s) => s.add(value));
@@ -106,6 +119,7 @@ abstract class CssClassSetImpl implements CssClassSet {
    * [removeClass](http://api.jquery.com/removeClass/).
    */
   bool remove(Object value) {
+    _validateToken(value);
     if (value is! String) return false;
     Set<String> s = readClasses();
     bool result = s.remove(value);
@@ -121,7 +135,7 @@ abstract class CssClassSetImpl implements CssClassSet {
    */
   void addAll(Iterable<String> iterable) {
     // TODO - see comment above about validation.
-    modify((s) => s.addAll(iterable));
+    modify((s) => s.addAll(iterable.map(_validateToken)));
   }
 
   /**
@@ -131,7 +145,7 @@ abstract class CssClassSetImpl implements CssClassSet {
    * [removeClass](http://api.jquery.com/removeClass/).
    */
   void removeAll(Iterable<Object> iterable) {
-    modify((s) => s.removeAll(iterable));
+    modify((s) => s.removeAll(iterable.map(_validateToken)));
   }
 
   /**

tests/html/element_classes_test.dart

@@ -129,6 +129,15 @@ main() {
     expect(makeClassSet().contains('qux'), isFalse);
   });
 
+  test('contains-bad', () {
+    // Non-strings return `false`.
+    // Strings need to be valid tokens.
+    final classes = makeClassSet();
+    expect(classes.contains(1), isFalse);
+    expect(() => classes.contains(''), throws);
+    expect(() => classes.contains('foo bar'), throws);
+  });
+
   test('add', () {
     final classes = makeClassSet();
     var added = classes.add('qux');
@@ -143,6 +152,12 @@ main() {
         reason: "The class set shouldn't have duplicate elements.");
   });
 
+  test('add-bad', () {
+    final classes = makeClassSet();
+    expect(() => classes.add(''), throws);
+    expect(() => classes.add('foo bar'), throws);
+  });
+
   test('remove', () {
     final classes = makeClassSet();
     classes.remove('bar');
@@ -151,6 +166,12 @@ main() {
     expect(classes, orderedEquals(['foo', 'baz']));
   });
 
+  test('remove-bad', () {
+    final classes = makeClassSet();
+    expect(() => classes.remove(''), throws);
+    expect(() => classes.remove('foo bar'), throws);
+  });
+
   test('toggle', () {
     final classes = makeClassSet();
     classes.toggle('bar');
@@ -168,6 +189,16 @@ main() {
     expect(classes, orderedEquals(['foo', 'baz', 'qux']));
   });
 
+  test('toggle-bad', () {
+    final classes = makeClassSet();
+    expect(() => classes.toggle(''), throws);
+    expect(() => classes.toggle('', true), throws);
+    expect(() => classes.toggle('', false), throws);
+    expect(() => classes.toggle('foo bar'), throws);
+    expect(() => classes.toggle('foo bar', true), throws);
+    expect(() => classes.toggle('foo bar', false), throws);
+  });
+
   test('addAll', () {
     final classes = makeClassSet();
     classes.addAll(['bar', 'qux', 'bip']);

tests/html/svgelement_test.dart

@@ -424,6 +424,32 @@ main() {
       classes.toggle('foo');
       expect(el.classes.length, 0);
     });
+
+    test('classes-add-bad', () {
+      var el = new svg.CircleElement();
+      expect(() => el.classes.add(''), throws);
+      expect(() => el.classes.add('foo bar'), throws);
+    });
+    test('classes-remove-bad', () {
+      expect(() => el.classes.remove(''), throws);
+      expect(() => el.classes.remove('foo bar'), throws);
+    });
+    test('classes-toggle-token', () {
+      var el = new svg.CircleElement();
+      expect(() => el.classes.toggle(''), throws);
+      expect(() => el.classes.toggle('', true), throws);
+      expect(() => el.classes.toggle('', false), throws);
+      expect(() => el.classes.toggle('foo bar'), throws);
+      expect(() => el.classes.toggle('foo bar', true), throws);
+      expect(() => el.classes.toggle('foo bar', false), throws);
+    });
+    test('classes-contains-bad', () {
+      var el = new svg.CircleElement();
+      // Non-strings => false, strings must be valid tokens.
+      expect(el.classes.contains(1), isFalse);
+      expect(() => el.classes.contains(''), throws);
+      expect(() => el.classes.contains('foo bar'), throws);
+    });
   });
 
   group('getBoundingClientRect', () {

tools/dom/src/CssClassSet.dart

@@ -19,6 +19,10 @@ abstract class CssClassSet implements Set<String> {
    * operation.
    *
    * If this corresponds to many elements, `null` is always returned.
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.  To toggle multiple classes, use
+   * [toggleAll].
    */
   bool toggle(String value, [bool shouldAdd]);
 
@@ -33,19 +37,26 @@ abstract class CssClassSet implements Set<String> {
    *
    * This is the Dart equivalent of jQuery's
    * [hasClass](http://api.jquery.com/hasClass/).
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.
    */
   bool contains(String value);
 
   /**
    * Add the class [value] to element.
    *
-   * This is the Dart equivalent of jQuery's
+   * [add] and [addAll] are the Dart equivalent of jQuery's
    * [addClass](http://api.jquery.com/addClass/).
    *
-   * If this corresponds to one element. Returns true if [value] was added to
-   * the set, otherwise false.
+   * If this CssClassSet corresponds to one element. Returns true if [value] was
+   * added to the set, otherwise false.
    *
    * If this corresponds to many elements, `null` is always returned.
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.  To add multiple classes use
+   * [addAll].
    */
   bool add(String value);
 
@@ -53,24 +64,34 @@ abstract class CssClassSet implements Set<String> {
    * Remove the class [value] from element, and return true on successful
    * removal.
    *
-   * This is the Dart equivalent of jQuery's
+   * [remove] and [removeAll] are the Dart equivalent of jQuery's
    * [removeClass](http://api.jquery.com/removeClass/).
+   *
+   * [value] must be a valid 'token' representing a single class, i.e. a
+   * non-empty string containing no whitespace.  To remove multiple classes, use
+   * [removeAll].
    */
   bool remove(Object value);
 
   /**
    * Add all classes specified in [iterable] to element.
    *
-   * This is the Dart equivalent of jQuery's
+   * [add] and [addAll] are the Dart equivalent of jQuery's
    * [addClass](http://api.jquery.com/addClass/).
+   *
+   * Each element of [iterable] must be a valid 'token' representing a single
+   * class, i.e. a non-empty string containing no whitespace.
    */
   void addAll(Iterable<String> iterable);
 
   /**
    * Remove all classes specified in [iterable] from element.
    *
-   * This is the Dart equivalent of jQuery's
+   * [remove] and [removeAll] are the Dart equivalent of jQuery's
    * [removeClass](http://api.jquery.com/removeClass/).
+   *
+   * Each element of [iterable] must be a valid 'token' representing a single
+   * class, i.e. a non-empty string containing no whitespace.
    */
   void removeAll(Iterable<String> iterable);
 
@@ -83,6 +104,9 @@ abstract class CssClassSet implements Set<String> {
    * If [shouldAdd] is true, then we always add all the classes in [iterable]
    * element. If [shouldAdd] is false then we always remove all the classes in
    * [iterable] from the element.
+   *
+   * Each element of [iterable] must be a valid 'token' representing a single
+   * class, i.e. a non-empty string containing no whitespace.
    */
   void toggleAll(Iterable<String> iterable, [bool shouldAdd]);
 }