Skip to content

Commit

Permalink
fix setAttribute issue in Chrome (see #1066); fix using function as `…
Browse files Browse the repository at this point in the history
…with` argument (fixes #2285)
  • Loading branch information
mbest committed Oct 1, 2017
1 parent 253d39c commit 2606678
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 13 deletions.
27 changes: 26 additions & 1 deletion spec/defaultBindings/withBehaviors.js
Expand Up @@ -199,4 +199,29 @@ describe('Binding: With', function() {
// subscription count is stable
expect(item.getSubscriptionsCount('change')).toEqual(3);
});
});

it('Should update if given a function', function () {
// See knockout/knockout#2285
testNode.innerHTML = '<div data-bind="with: getTotal">Total: <div data-bind="text: $data"></div>';

function ViewModel() {
var self = this;
self.items = ko.observableArray([{ x: ko.observable(4) }])
self.getTotal = function() {
var total = 0;
ko.utils.arrayForEach(self.items(), function(item) { total += item.x();});
return total;
}
}

var model = new ViewModel();
ko.applyBindings(model, testNode);
expect(testNode).toContainText("Total: 4");

model.items.push({ x: ko.observable(15) });
expect(testNode).toContainText("Total: 19");

model.items()[0].x(10);
expect(testNode).toContainText("Total: 25");
});
});
18 changes: 8 additions & 10 deletions src/binding/defaultBindings/attr.js
Expand Up @@ -6,20 +6,18 @@ ko.bindingHandlers['attr'] = {
attrValue = ko.utils.unwrapObservable(attrValue);

// Find the namespace of this attribute, if any.
// Defaulting to `null` should be ok, as
//
// element.setAttributeNS( null, name, value ) ~ element.setAttribute( name, value )
// element.removetAttributeNS( null, name ) ~ element.removeAttribute( name )
//
var prefixLen = attrName.indexOf(':');
var namespace = prefixLen < 0 ? null : element.lookupNamespaceURI( attrName.substr(0, prefixLen) );
var namespace = prefixLen > 0 && element.lookupNamespaceURI( attrName.substr(0, prefixLen) );

// To cover cases like "attr: { checked:someProp }", we want to remove the attribute entirely
// when someProp is a "no value"-like value (strictly null, false, or undefined)
// (because the absence of the "checked" attr is how to mark an element as not checked, etc.)
var toRemove = (attrValue === false) || (attrValue === null) || (attrValue === undefined);
if (toRemove)
element.removeAttributeNS(namespace, attrName);
if (toRemove) {
namespace ? element.removeAttributeNS(namespace, attrName) : element.removeAttribute(attrName);
} else {
attrValue = attrValue.toString();
}

// In IE <= 7 and IE8 Quirks Mode, you have to use the JavaScript property name instead of the
// HTML attribute name for certain attributes. IE8 Standards Mode supports the correct behavior,
Expand All @@ -32,15 +30,15 @@ ko.bindingHandlers['attr'] = {
else
element[attrName] = attrValue;
} else if (!toRemove) {
element.setAttributeNS(namespace, attrName, attrValue.toString());
namespace ? element.setAttributeNS(namespace, attrName, attrValue) : element.setAttribute(attrName, attrValue);
}

// Treat "name" specially - although you can think of it as an attribute, it also needs
// special handling on older versions of IE (https://github.com/SteveSanderson/knockout/pull/333)
// Deliberately being case-sensitive here because XHTML would regard "Name" as a different thing
// entirely, and there's no strong reason to allow for such casing in HTML.
if (attrName === "name") {
ko.utils.setElementName(element, toRemove ? "" : attrValue.toString());
ko.utils.setElementName(element, toRemove ? "" : attrValue);
}
});
}
Expand Down
5 changes: 3 additions & 2 deletions src/binding/defaultBindings/ifIfnotWith.js
Expand Up @@ -8,7 +8,8 @@ function makeWithIfBinding(bindingKey, isWith, isNot) {
}, null, { disposeWhenNodeIsRemoved: element });

ko.computed(function() {
var shouldDisplay = isWith ? !!ko.utils.unwrapObservable(valueAccessor()) : ifCondition(),
var rawWithValue = isWith && ko.utils.unwrapObservable(valueAccessor()),
shouldDisplay = isWith ? !!rawWithValue : ifCondition(),
isFirstRender = !savedNodes;

// Save a copy of the inner nodes on the initial update, but only if we have dependencies.
Expand All @@ -22,7 +23,7 @@ function makeWithIfBinding(bindingKey, isWith, isNot) {
}
ko.applyBindingsToDescendants(
isWith ?
bindingContext['createChildContext'](valueAccessor) :
bindingContext['createChildContext'](typeof rawWithValue == "function" ? rawWithValue : valueAccessor) :
ifCondition.isActive() ?
bindingContext['extend'](function() { ifCondition(); return null; }) :
bindingContext,
Expand Down

0 comments on commit 2606678

Please sign in to comment.