Merge pull request #255 from snabbdom/set-boolean-attributes-correctly

Set boolean attributes correctly according to spec

src/modules/attributes.ts

@@ -12,13 +12,14 @@ const booleanAttrs = ["allowfullscreen", "async", "autofocus", "autoplay", "chec
                 "required", "reversed", "scoped", "seamless", "selected", "sortable", "spellcheck", "translate",
                 "truespeed", "typemustmatch", "visible"];
 
-const booleanAttrsDict = Object.create(null);
+const booleanAttrsDict: {[attribute: string]: boolean} = Object.create(null);
+
 for (let i = 0, len = booleanAttrs.length; i < len; i++) {
   booleanAttrsDict[booleanAttrs[i]] = true;
 }
 
 function updateAttrs(oldVnode: VNode, vnode: VNode): void {
-  var key: string, cur: any, old: any, elm: Element = vnode.elm as Element,
+  var key: string, elm: Element = vnode.elm as Element,
       oldAttrs = (oldVnode.data as VNodeData).attrs,
       attrs = (vnode.data as VNodeData).attrs, namespaceSplit: Array<string>;
 
@@ -29,21 +30,26 @@ function updateAttrs(oldVnode: VNode, vnode: VNode): void {
 
   // update modified attributes, add new attributes
   for (key in attrs) {
-    cur = attrs[key];
+    const cur = attrs[key];
-    old = oldAttrs[key];
+    const old = oldAttrs[key];
     if (old !== cur) {
-      if (!cur && booleanAttrsDict[key])
+      if (booleanAttrsDict[key]) {
-        elm.removeAttribute(key);
+        if (cur) {
-      else {
+          elm.setAttribute(key, "");
+        } else {
+          elm.removeAttribute(key);
+        }
+      } else {
         namespaceSplit = key.split(":");
-        if (namespaceSplit.length > 1 && NamespaceURIs.hasOwnProperty(namespaceSplit[0]))
+        if (namespaceSplit.length > 1 && NamespaceURIs.hasOwnProperty(namespaceSplit[0])) {
           elm.setAttributeNS((NamespaceURIs as any)[namespaceSplit[0]], key, cur);
-        else
+        } else {
           elm.setAttribute(key, cur);
+        }
       }
     }
   }
-  //remove removed attributes
+  // remove removed attributes
   // use `in` operator since the previous `for` iteration uses it (.i.e. add even attributes with undefined value)
   // the other option is to remove all attributes with value == undefined
   for (key in oldAttrs) {

test/attributes.js

@@ -57,12 +57,15 @@ describe('attributes', function() {
     assert.strictEqual(elm.textContent, 'Hello');
   });
   describe('boolean attribute', function() {
-    it('is present if the value is truthy', function() {
+    it('is present and empty string if the value is truthy', function() {
       var vnode1 = h('div', {attrs: {required: true, readonly: 1, noresize: 'truthy'}});
       elm = patch(vnode0, vnode1).elm;
-      assert.strictEqual(elm.getAttribute('required'), 'true');
+      assert.strictEqual(elm.hasAttribute('required'), true);
-      assert.strictEqual(elm.getAttribute('readonly'), '1');
+      assert.strictEqual(elm.getAttribute('required'), '');
-      assert.strictEqual(elm.getAttribute('noresize'), 'truthy');
+      assert.strictEqual(elm.hasAttribute('readonly'), true);
+      assert.strictEqual(elm.getAttribute('readonly'), '');
+      assert.strictEqual(elm.hasAttribute('noresize'), true);
+      assert.strictEqual(elm.getAttribute('noresize'), '');
     });
     it('is omitted if the value is falsy', function() {
       var vnode1 = h('div', {attrs: {required: false, readonly: 0, noresize: null}});
@@ -80,6 +83,6 @@ describe('attributes', function() {
       var vnode2 = h('div', {attrs: {constructor: false}});
       elm = patch(vnode0, vnode2).elm;
       assert.strictEqual(elm.getAttribute('constructor'), 'false');
-    })
+    });
   });
 });