Enforce token syntax for CssClassSet arguments

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

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

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@45266 260f80e4-7a28-3924-810f-c04153c831b5
This commit is contained in:
sra@google.com 2015-04-18 01:58:20 +00:00
parent 08676965c4
commit efc7d09945
6 changed files with 21 additions and 164 deletions

View file

@ -36383,10 +36383,6 @@ 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]);
@ -36401,26 +36397,19 @@ 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.
*
* [add] and [addAll] are the Dart equivalent of jQuery's
* This is the Dart equivalent of jQuery's
* [addClass](http://api.jquery.com/addClass/).
*
* If this CssClassSet corresponds to one element. Returns true if [value] was
* added to the set, otherwise false.
* If this 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);
@ -36428,34 +36417,24 @@ abstract class CssClassSet implements Set<String> {
* Remove the class [value] from element, and return true on successful
* removal.
*
* [remove] and [removeAll] are the Dart equivalent of jQuery's
* This is 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.
*
* [add] and [addAll] are the Dart equivalent of jQuery's
* This is 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.
*
* [remove] and [removeAll] are the Dart equivalent of jQuery's
* This is 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);
@ -36468,9 +36447,6 @@ 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]);
}

View file

@ -37305,10 +37305,6 @@ 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]);
@ -37323,26 +37319,19 @@ 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.
*
* [add] and [addAll] are the Dart equivalent of jQuery's
* This is the Dart equivalent of jQuery's
* [addClass](http://api.jquery.com/addClass/).
*
* If this CssClassSet corresponds to one element. Returns true if [value] was
* added to the set, otherwise false.
* If this 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);
@ -37350,34 +37339,24 @@ abstract class CssClassSet implements Set<String> {
* Remove the class [value] from element, and return true on successful
* removal.
*
* [remove] and [removeAll] are the Dart equivalent of jQuery's
* This is 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.
*
* [add] and [addAll] are the Dart equivalent of jQuery's
* This is 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.
*
* [remove] and [removeAll] are the Dart equivalent of jQuery's
* This is 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);
@ -37390,9 +37369,6 @@ 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]);
}

View file

@ -6,13 +6,6 @@ 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(' ');
}
@ -25,7 +18,6 @@ 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);
@ -89,11 +81,7 @@ abstract class CssClassSetImpl implements CssClassSet {
* This is the Dart equivalent of jQuery's
* [hasClass](http://api.jquery.com/hasClass/).
*/
bool contains(Object value) {
if (value is! String) return false;
_validateToken(value);
return readClasses().contains(value);
}
bool contains(Object value) => readClasses().contains(value);
/** Lookup from the Set interface. Not interesting for a String set. */
String lookup(Object value) => contains(value) ? value : null;
@ -105,7 +93,6 @@ 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));
@ -119,7 +106,6 @@ 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);
@ -135,7 +121,7 @@ abstract class CssClassSetImpl implements CssClassSet {
*/
void addAll(Iterable<String> iterable) {
// TODO - see comment above about validation.
modify((s) => s.addAll(iterable.map(_validateToken)));
modify((s) => s.addAll(iterable));
}
/**
@ -145,7 +131,7 @@ abstract class CssClassSetImpl implements CssClassSet {
* [removeClass](http://api.jquery.com/removeClass/).
*/
void removeAll(Iterable<Object> iterable) {
modify((s) => s.removeAll(iterable.map(_validateToken)));
modify((s) => s.removeAll(iterable));
}
/**

View file

@ -129,15 +129,6 @@ 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');
@ -152,12 +143,6 @@ 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');
@ -166,12 +151,6 @@ 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');
@ -189,16 +168,6 @@ 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']);

View file

@ -424,32 +424,6 @@ 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', () {

View file

@ -19,10 +19,6 @@ 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]);
@ -37,26 +33,19 @@ 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.
*
* [add] and [addAll] are the Dart equivalent of jQuery's
* This is the Dart equivalent of jQuery's
* [addClass](http://api.jquery.com/addClass/).
*
* If this CssClassSet corresponds to one element. Returns true if [value] was
* added to the set, otherwise false.
* If this 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);
@ -64,34 +53,24 @@ abstract class CssClassSet implements Set<String> {
* Remove the class [value] from element, and return true on successful
* removal.
*
* [remove] and [removeAll] are the Dart equivalent of jQuery's
* This is 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.
*
* [add] and [addAll] are the Dart equivalent of jQuery's
* This is 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.
*
* [remove] and [removeAll] are the Dart equivalent of jQuery's
* This is 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);
@ -104,9 +83,6 @@ 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]);
}