Skip to content

Commit

Permalink
[BUGFIX beta] Remove false attribute from Ember.View
Browse files Browse the repository at this point in the history
In IE8 (or older), `jQuery.fn.prop` always update DOM against our
expectation.(This is a jQuery bug but it couldn't fix)
ref: http://bugs.jquery.com/ticket/13558

So the following test is failed due to `view.$().attr('foo')` returns `"false"`.:
* https://github.com/emberjs/ember.js/blob/a449d445/packages/ember-views/tests/views/view/attribute_bindings_test.js#L126

To avoid this issue, `removeAttr` should be called when `null`, `undefined` or `false` is given as value.

But this way contains incompatible changes:

Before:
``` javascript
view.set('checked', false);
view.$().prop('checked'); //=> false
```

After:
``` javascript
view.set('checked', false);
view.$().prop('checked'); //=> undefined
```
  • Loading branch information
tricknotes committed Jan 4, 2014
1 parent 8f95996 commit 2d89fe1
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 9 deletions.
13 changes: 5 additions & 8 deletions packages/ember-views/lib/views/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -2479,15 +2479,12 @@ Ember.View.applyAttributeBindings = function(elem, name, value) {
elem.attr(name, value);
}
} else if (name === 'value' || type === 'boolean') {
// We can't set properties to undefined or null
if (Ember.isNone(value)) { value = ''; }

if (!value) {
if (Ember.isNone(value) || value === false) {
// `null`, `undefined` or `false` should remove attribute
elem.removeAttr(name);
}

if (value !== elem.prop(name)) {
// value and booleans should always be properties
elem.prop(name, '');
} else if (value !== elem.prop(name)) {
// value should always be properties
elem.prop(name, value);
}
} else if (!value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ test("handles attribute bindings for properties", function() {
view.set('checked', false);
});

equal(view.$().prop('checked'), false, 'changes to unchecked');
equal(!!view.$().prop('checked'), false, 'changes to unchecked');
});

test("handles `undefined` value for properties", function() {
Expand Down

0 comments on commit 2d89fe1

Please sign in to comment.