Fix MithrilJS#1956

Note: this doesn't actually provide proper error handling. This just kicks the can down the road by addressing the file input as a one-off fix (since it's the only such DOM property I'm aware of that can throw on set).
dead-claudia · Mar 16, 2020 · b7f392f · b7f392f
1 parent 455e511
commit b7f392f
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 4 deletions.
diff --git a/render/render.js b/render/render.js
@@ -726,11 +726,12 @@ module.exports = function($window) {
 
 	//attrs
 	function setAttrs(vnode, attrs, ns) {
+		var isFileInput = attrs != null && vnode.tag === "input" && attrs.type === "file"
 		for (var key in attrs) {
-			setAttr(vnode, key, null, attrs[key], ns)
+			setAttr(vnode, key, null, attrs[key], ns, isFileInput)
 		}
 	}
-	function setAttr(vnode, key, old, value, ns) {
+	function setAttr(vnode, key, old, value, ns, isFileInput) {
 		if (key === "key" || key === "is" || value == null || isLifecycleMethod(key) || (old === value && !isFormAttribute(vnode, key)) && typeof value !== "object") return
 		if (key[0] === "o" && key[1] === "n") return updateEvent(vnode, key, value)
 		if (key.slice(0, 6) === "xlink:") vnode.dom.setAttributeNS("http://www.w3.org/1999/xlink", key.slice(6), value)
@@ -740,11 +741,15 @@ module.exports = function($window) {
 				// Only do the coercion if we're actually going to check the value.
 				/* eslint-disable no-implicit-coercion */
 				//setting input[value] to same value by typing on focused element moves cursor to end in Chrome
-				if ((vnode.tag === "input" || vnode.tag === "textarea") && vnode.dom.value === "" + value && vnode.dom === activeElement()) return
+				//setting input[type=file][value] to same value causes an error to be generated if it's non-empty
+				if ((vnode.tag === "input" || vnode.tag === "textarea") && vnode.dom.value === "" + value && (isFileInput || vnode.dom === activeElement())) return
 				//setting select[value] to same value while having select open blinks select dropdown in Chrome
 				if (vnode.tag === "select" && old !== null && vnode.dom.value === "" + value) return
 				//setting option[value] to same value while having select open blinks select dropdown in Chrome
 				if (vnode.tag === "option" && old !== null && vnode.dom.value === "" + value) return
+				//setting input[type=file][value] to different value is an error if it's non-empty
+				// Not ideal, but it at least works around the most common source of uncaught exceptions for now.
+				if (isFileInput && "" + value !== "") { console.error("`value` is read-only on file inputs!"); return }
 				/* eslint-enable no-implicit-coercion */
 			}
 			// If you assign an input type that is not supported by IE 11 with an assignment expression, an error will occur.
@@ -793,8 +798,9 @@ module.exports = function($window) {
 	}
 	function updateAttrs(vnode, old, attrs, ns) {
 		if (attrs != null) {
+			var isFileInput = vnode.tag === "input" && attrs.type === "file"
 			for (var key in attrs) {
-				setAttr(vnode, key, old && old[key], attrs[key], ns)
+				setAttr(vnode, key, old && old[key], attrs[key], ns, isFileInput)
 			}
 		}
 		var val

diff --git a/render/tests/test-input.js b/render/tests/test-input.js
@@ -86,6 +86,89 @@ o.spec("form inputs", function() {
 
 			o(updated.dom.checked).equals(true)
 		})
+
+		o("syncs file input value attribute if DOM value differs from vdom value and is empty", function() {
+			var input = {tag: "input", attrs: {type: "file", value: "", onclick: function() {}}}
+			var updated = {tag: "input", attrs: {type: "file", value: "", onclick: function() {}}}
+			var spy = o.spy()
+			var error = console.error
+
+			render(root, [input])
+
+			input.dom.value = "test.png"
+
+			try {
+				console.error = spy
+				render(root, [updated])
+			} finally {
+				console.error = error
+			}
+
+			o(updated.dom.value).equals("")
+			o(spy.callCount).equals(0)
+		})
+
+		o("warns and ignores file input value attribute if DOM value differs from vdom value and is non-empty", function() {
+			var input = {tag: "input", attrs: {type: "file", value: "", onclick: function() {}}}
+			var updated = {tag: "input", attrs: {type: "file", value: "other.png", onclick: function() {}}}
+			var spy = o.spy()
+			var error = console.error
+
+			render(root, [input])
+
+			input.dom.value = "test.png"
+
+			try {
+				console.error = spy
+				render(root, [updated])
+			} finally {
+				console.error = error
+			}
+
+			o(updated.dom.value).equals("test.png")
+			o(spy.callCount).equals(1)
+		})
+
+		o("retains file input value attribute if DOM value is the same as vdom value and is non-empty", function() {
+			var $window = domMock(o)
+			var render = vdom($window)
+			var root = $window.document.createElement("div")
+			$window.document.body.appendChild(root)
+			var input = {tag: "input", attrs: {type: "file", value: "", onclick: function() {}}}
+			var updated1 = {tag: "input", attrs: {type: "file", value: "test.png", onclick: function() {}}}
+			var updated2 = {tag: "input", attrs: {type: "file", value: "test.png", onclick: function() {}}}
+			var spy = o.spy()
+			var error = console.error
+
+			render(root, [input])
+
+			// Verify our assumptions about the outer element state
+			o($window.__getSpies(input.dom).valueSetter.callCount).equals(0)
+			input.dom.value = "test.png"
+			o($window.__getSpies(input.dom).valueSetter.callCount).equals(1)
+
+			try {
+				console.error = spy
+				render(root, [updated1])
+			} finally {
+				console.error = error
+			}
+
+			o(updated1.dom.value).equals("test.png")
+			o(spy.callCount).equals(0)
+			o($window.__getSpies(updated1.dom).valueSetter.callCount).equals(1)
+
+			try {
+				console.error = spy
+				render(root, [updated2])
+			} finally {
+				console.error = error
+			}
+
+			o(updated2.dom.value).equals("test.png")
+			o(spy.callCount).equals(0)
+			o($window.__getSpies(updated2.dom).valueSetter.callCount).equals(1)
+		})
 	})
 
 	o.spec("select", function() {