[Vue 2.0] Fix BooleanAttr & EnumeratedAttr serialization (#2810)

* fix attr serialization

* Add attribute test cases

src/platforms/web/runtime/modules/attrs.js

@@ -1,4 +1,4 @@
-import { isBooleanAttr, isEnumeratedAttr, isXlink, xlinkNS } from 'web/util/index'
+import { isBooleanAttr, isEnumeratedAttr, isXlink, xlinkNS, getXlinkProp } from 'web/util/index'
 
 function updateAttrs (oldVnode, vnode) {
   if (!oldVnode.data.attrs && !vnode.data.attrs) {
@@ -19,8 +19,8 @@ function updateAttrs (oldVnode, vnode) {
   for (key in oldAttrs) {
     if (attrs[key] == null) {
       if (isXlink(key)) {
-        elm.removeAttributeNS(xlinkNS, key)
-      } else {
+        elm.removeAttributeNS(xlinkNS, getXlinkProp(key))
+      } else if (!isEnumeratedAttr(key)) {
         elm.removeAttribute(key)
       }
     }
@@ -29,17 +29,27 @@ function updateAttrs (oldVnode, vnode) {
 
 function setAttr (el, key, value) {
   if (isBooleanAttr(key)) {
-    if (value == null) {
+    // set attribute for blank value
+    // e.g. <option disabled>Select one</option>
+    if (value == null || value === false) {
       el.removeAttribute(key)
     } else {
       el.setAttribute(key, key)
     }
   } else if (isEnumeratedAttr(key)) {
-    el.setAttribute(key, value == null ? 'false' : 'true')
+    el.setAttribute(key, value ? 'true' : 'false')
   } else if (isXlink(key)) {
-    el.setAttributeNS(xlinkNS, key, value)
+    if (value == null || value === false) {
+      el.removeAttributeNS(xlinkNS, getXlinkProp(key))
+    } else {
+      el.setAttributeNS(xlinkNS, key, value === true ? '' : value)
+    }
   } else {
-    el.setAttribute(key, value)
+    if (value == null || value === false) {
+      el.removeAttribute(key)
+    } else {
+      el.setAttribute(key, value === true ? '' : value)
+    }
   }
 }
 

src/platforms/web/server/modules/attrs.js

@@ -23,13 +23,16 @@ function serialize (attrs, asProps) {
     if (asProps) {
       key = propsToAttrMap[key] || key.toLowerCase()
     }
-    if (attrs[key] != null) {
-      if (isBooleanAttr(key)) {
+    const value = attrs[key]
+    if (isBooleanAttr(key)) {
+      if (!(value == null || value === false)) {
         res += ` ${key}="${key}"`
-      } else if (isEnumeratedAttr(key)) {
-        res += ` ${key}="true"`
-      } else {
-        res += ` ${key}="${attrs[key]}"`
+      }
+    } else if (isEnumeratedAttr(key)) {
+      res += ` ${key}="${value ? 'true' : 'false'}"`
+    } else {
+      if (!(value == null || value === false)) {
+        res += ` ${key}="${value === true ? '' : value}"`
       }
     }
   }

src/platforms/web/util/attrs.js

@@ -23,3 +23,4 @@ export const propsToAttrMap = {
 
 export const xlinkNS = 'http://www.w3.org/1999/xlink'
 export const isXlink = name => name.charAt(5) === ':' && name.slice(0, 5) === 'xlink'
+export const getXlinkProp = name => isXlink(name) ? name.slice(6, name.length) : ''

src/server/create-streaming-renderer.js

@@ -24,7 +24,7 @@ export function createStreamingRenderer (modules, directives, isUnaryTag) {
     if (isRoot) {
       if (!el.data) el.data = {}
       if (!el.data.attrs) el.data.attrs = {}
-      el.data.attrs['server-rendered'] = true
+      el.data.attrs['server-rendered'] = 'true'
     }
     const startTag = renderStartingTag(el, modules, directives)
     const endTag = `</${el.tag}>`

src/server/create-sync-renderer.js

@@ -21,7 +21,7 @@ export function createSyncRenderer (modules, directives, isUnaryTag) {
     if (isRoot) {
       if (!el.data) el.data = {}
       if (!el.data.attrs) el.data.attrs = {}
-      el.data.attrs['server-rendered'] = true
+      el.data.attrs['server-rendered'] = 'true'
     }
     const startTag = renderStartingTag(el, modules, directives)
     const endTag = `</${el.tag}>`

test/ssr/ssr.sync.spec.js

@@ -125,6 +125,68 @@ describe('SSR: renderToString', () => {
       '</div>'
     )
   })
+
+  it('normal attr', () => {
+    expect(renderVmWithOptions({
+      template: `
+        <div>
+          <span :test="'ok'">hello</span>
+          <span :test="null">hello</span>
+          <span :test="false">hello</span>
+          <span :test="true">hello</span>
+          <span :test="0">hello</span>
+        </div>
+      `
+    })).toContain(
+      '<div server-rendered="true">' +
+        '<span test="ok">hello</span>' +
+        '<span>hello</span>' +
+        '<span>hello</span>' +
+        '<span test="">hello</span>' +
+        '<span test="0">hello</span>' +
+      '</div>'
+    )
+  })
+
+  it('enumrated attr', () => {
+    expect(renderVmWithOptions({
+      template: `
+        <div>
+          <span :draggable="true">hello</span>
+          <span :draggable="'ok'">hello</span>
+          <span :draggable="null">hello</span>
+          <span :draggable="''">hello</span>
+        </div>
+      `
+    })).toContain(
+      '<div server-rendered="true">' +
+        '<span draggable="true">hello</span>' +
+        '<span draggable="true">hello</span>' +
+        '<span draggable="false">hello</span>' +
+        '<span draggable="true">hello</span>' +
+      '</div>'
+    )
+  })
+
+  it('boolean attr', () => {
+    expect(renderVmWithOptions({
+      template: `
+        <div>
+          <span :disabled="true">hello</span>
+          <span :disabled="'ok'">hello</span>
+          <span :disabled="null">hello</span>
+          <span :disabled="''">hello</span>
+        </div>
+      `
+    })).toContain(
+      '<div server-rendered="true">' +
+        '<span disabled="disabled">hello</span>' +
+        '<span disabled="disabled">hello</span>' +
+        '<span>hello</span>' +
+        '<span disabled="disabled">hello</span>' +
+      '</div>'
+    )
+  })
 })
 
 function renderVmWithOptions (options) {

test/unit/features/directives/bind.spec.js

@@ -0,0 +1,115 @@
+import Vue from 'vue'
+
+describe('Directive v-bind', () => {
+  it('normal attr', done => {
+    const vm = new Vue({
+      el: '#app',
+      template: '<div><span :test="foo">hello</span></div>',
+      data: { foo: 'ok' }
+    })
+    expect(vm.$el.firstChild.getAttribute('test')).toBe('ok')
+    vm.foo = 'again'
+    waitForUpdate(() => {
+      expect(vm.$el.firstChild.getAttribute('test')).toBe('again')
+      vm.foo = null
+    }).then(() => {
+      expect(vm.$el.firstChild.hasAttribute('test')).toBe(false)
+      vm.foo = false
+    }).then(() => {
+      expect(vm.$el.firstChild.hasAttribute('test')).toBe(false)
+      vm.foo = true
+    }).then(() => {
+      expect(vm.$el.firstChild.getAttribute('test')).toBe('')
+      vm.foo = 0
+    }).then(() => {
+      expect(vm.$el.firstChild.getAttribute('test')).toBe('0')
+      done()
+    }).catch(done)
+  })
+
+  it('should set property for input value', done => {
+    const vm = new Vue({
+      el: '#app',
+      template: `
+        <div>
+          <input type="text" :value="foo">
+          <input type="checkbox" :checked="bar">
+        </div>
+      `,
+      data: {
+        foo: 'ok',
+        bar: false
+      }
+    })
+    expect(vm.$el.firstChild.value).toBe('ok')
+    expect(vm.$el.lastChild.checked).toBe(false)
+    vm.bar = true
+    waitForUpdate(() => {
+      expect(vm.$el.lastChild.checked).toBe(true)
+      done()
+    }).catch(done)
+  })
+
+  it('xlink', done => {
+    const vm = new Vue({
+      el: '#app',
+      template: '<svg><a :xlink:special="foo"></a></svg>',
+      data: {
+        foo: 'ok'
+      }
+    })
+    const xlinkNS = 'http://www.w3.org/1999/xlink'
+    expect(vm.$el.firstChild.getAttributeNS(xlinkNS, 'special')).toBe('ok')
+    vm.foo = 'again'
+    waitForUpdate(() => {
+      expect(vm.$el.firstChild.getAttributeNS(xlinkNS, 'special')).toBe('again')
+      vm.foo = null
+    }).then(() => {
+      expect(vm.$el.firstChild.hasAttributeNS(xlinkNS, 'special')).toBe(false)
+      vm.foo = true
+    }).then(() => {
+      expect(vm.$el.firstChild.getAttributeNS(xlinkNS, 'special')).toBe('')
+      done()
+    }).catch(done)
+  })
+
+  it('enumrated attr', done => {
+    const vm = new Vue({
+      el: '#app',
+      template: '<div><span :draggable="foo">hello</span></div>',
+      data: { foo: true }
+    })
+    expect(vm.$el.firstChild.getAttribute('draggable')).toBe('true')
+    vm.foo = 'again'
+    waitForUpdate(() => {
+      expect(vm.$el.firstChild.getAttribute('draggable')).toBe('true')
+      vm.foo = null
+    }).then(() => {
+      expect(vm.$el.firstChild.getAttribute('draggable')).toBe('false')
+      vm.foo = ''
+    }).then(() => {
+      expect(vm.$el.firstChild.getAttribute('draggable')).toBe('false')
+      done()
+    }).catch(done)
+  })
+
+  it('boolean attr', done => {
+    const vm = new Vue({
+      el: '#app',
+      template: '<div><span :disabled="foo">hello</span></div>',
+      data: { foo: true }
+    })
+    expect(vm.$el.firstChild.getAttribute('disabled')).toBe('disabled')
+    vm.foo = 'again'
+    waitForUpdate(() => {
+      expect(vm.$el.firstChild.getAttribute('disabled')).toBe('disabled')
+      vm.foo = null
+    }).then(() => {
+      expect(vm.$el.firstChild.hasAttribute('disabled')).toBe(false)
+      vm.foo = ''
+    }).then(() => {
+      expect(vm.$el.firstChild.hasAttribute('disabled')).toBe(true)
+      done()
+    }).catch(done)
+  })
+})