diff --git a/docs/change-log.md b/docs/change-log.md index f8f29e03..722d5975 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -1,6 +1,6 @@ # Change log -- [v2.0.0](#v113) +- [v2.0.0](#v200-wip) - [v1.1.4](#v113) - [v1.1.3](#v113) - [v1.1.2](#v112) @@ -22,6 +22,7 @@ #### News - API: Introduction of `m.redraw.sync()` ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592)) +- API: Event handlers may also be objects with `handleEvent` methods ([#1939](https://github.com/MithrilJS/mithril.js/issues/1939)). #### Ospec improvements: @@ -33,6 +34,10 @@ - API: `m.route.set()` causes all mount points to be redrawn ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592)) - API: If a user sets the Content-Type header within a request's options, that value will be the entire header value rather than being appended to the default value ([#1924](https://github.com/MithrilJS/mithril.js/pull/1924)) - API: Using style objects in hyperscript calls will now properly diff style properties from one render to another as opposed to re-writing all element style properties every render. +- core: `addEventListener` and `removeEventListener` are always used to manage event subscriptions, preventing external interference. +- core: Event listeners allocate less memory, swap at low cost, and are properly diffed now when rendered via `m.mount()`/`m.redraw()`. +- core: `Object.prototype` properties can no longer interfere with event listener calls. +- API: Event handlers, when set to literally `undefined` (or any non-function), are now correctly removed. --- diff --git a/render/render.js b/render/render.js index cc6afd51..d8de6e0d 100644 --- a/render/render.js +++ b/render/render.js @@ -472,13 +472,11 @@ module.exports = function($window) { } } function setAttr(vnode, key, old, value, ns) { + if (key === "key" || key === "is" || isLifecycleMethod(key)) return + if (key[0] === "o" && key[1] === "n") return updateEvent(vnode, key, value) + if ((old === value && !isFormAttribute(vnode, key)) && typeof value !== "object" || value === undefined) return var element = vnode.dom - if (key === "key" || key === "is" || (old === value && !isFormAttribute(vnode, key)) && typeof value !== "object" || typeof value === "undefined" || isLifecycleMethod(key)) return - var nsLastIndex = key.indexOf(":") - if (nsLastIndex > -1 && key.substr(0, nsLastIndex) === "xlink") { - element.setAttributeNS("http://www.w3.org/1999/xlink", key.slice(nsLastIndex + 1), value) - } - else if (key[0] === "o" && key[1] === "n" && typeof value === "function") updateEvent(vnode, key, value) + if (key.slice(0, 6) === "xlink:") element.setAttributeNS("http://www.w3.org/1999/xlink", key.slice(6), value) else if (key === "style") updateStyle(element, old, value) else if (key in element && !isAttribute(key) && ns === undefined && !isCustomElement(vnode)) { if (key === "value") { @@ -575,24 +573,39 @@ module.exports = function($window) { } } + // Here's an explanation of how this works: + // 1. The event names are always (by design) prefixed by `on`. + // 2. The EventListener interface accepts either a function or an object + // with a `handleEvent` method. + // 3. The object does not inherit from `Object.prototype`, to avoid + // any potential interference with that (e.g. setters). + // 4. The event name is remapped to the handler before calling it. + // 5. In function-based event handlers, `ev.target === this`. We replicate + // that below. + function EventDict() {} + EventDict.prototype = Object.create(null) + EventDict.prototype.handleEvent = function (ev) { + var handler = this["on" + ev.type] + if (typeof handler === "function") handler.call(ev.target, ev) + else if (typeof handler.handleEvent === "function") handler.handleEvent(ev) + if (typeof onevent === "function") onevent.call(ev.target, ev) + } + //event function updateEvent(vnode, key, value) { - var element = vnode.dom - var callback = typeof onevent !== "function" ? value : function(e) { - var result = value.call(element, e) - onevent.call(element, e) - return result - } - if (key in element) element[key] = typeof value === "function" ? callback : null - else { - var eventName = key.slice(2) - if (vnode.events === undefined) vnode.events = {} - if (vnode.events[key] === callback) return - if (vnode.events[key] != null) element.removeEventListener(eventName, vnode.events[key], false) - if (typeof value === "function") { - vnode.events[key] = callback - element.addEventListener(eventName, vnode.events[key], false) + if (vnode.events != null) { + if (vnode.events[key] === value) return + if (value != null && (typeof value === "function" || typeof value === "object")) { + if (vnode.events[key] == null) vnode.dom.addEventListener(key.slice(2), vnode.events, false) + vnode.events[key] = value + } else { + if (vnode.events[key] != null) vnode.dom.removeEventListener(key.slice(2), vnode.events, false) + vnode.events[key] = undefined } + } else if (value != null && (typeof value === "function" || typeof value === "object")) { + vnode.events = new EventDict() + vnode.dom.addEventListener(key.slice(2), vnode.events, false) + vnode.events[key] = value } } diff --git a/render/tests/test-event.js b/render/tests/test-event.js index 1f7eec2a..b6cb1540 100644 --- a/render/tests/test-event.js +++ b/render/tests/test-event.js @@ -33,7 +33,27 @@ o.spec("event", function() { o(onevent.args[0].type).equals("click") o(onevent.args[0].target).equals(div.dom) }) - + + o("handles click EventListener object", function() { + var spy = o.spy() + var listener = {handleEvent: spy} + var div = {tag: "div", attrs: {onclick: listener}} + var e = $window.document.createEvent("MouseEvents") + e.initEvent("click", true, true) + + render(root, [div]) + div.dom.dispatchEvent(e) + + o(spy.callCount).equals(1) + o(spy.this).equals(listener) + o(spy.args[0].type).equals("click") + o(spy.args[0].target).equals(div.dom) + o(onevent.callCount).equals(1) + o(onevent.this).equals(div.dom) + o(onevent.args[0].type).equals("click") + o(onevent.args[0].target).equals(div.dom) + }) + o("removes event", function() { var spy = o.spy() var vnode = {tag: "a", attrs: {onclick: spy}} @@ -45,7 +65,130 @@ o.spec("event", function() { var e = $window.document.createEvent("MouseEvents") e.initEvent("click", true, true) vnode.dom.dispatchEvent(e) - + + o(spy.callCount).equals(0) + }) + + o("removes event when null", function() { + var spy = o.spy() + var vnode = {tag: "a", attrs: {onclick: spy}} + var updated = {tag: "a", attrs: {onclick: null}} + + render(root, [vnode]) + render(root, [updated]) + + var e = $window.document.createEvent("MouseEvents") + e.initEvent("click", true, true) + vnode.dom.dispatchEvent(e) + + o(spy.callCount).equals(0) + }) + + o("removes event when undefined", function() { + var spy = o.spy() + var vnode = {tag: "a", attrs: {onclick: spy}} + var updated = {tag: "a", attrs: {onclick: undefined}} + + render(root, [vnode]) + render(root, [updated]) + + var e = $window.document.createEvent("MouseEvents") + e.initEvent("click", true, true) + vnode.dom.dispatchEvent(e) + + o(spy.callCount).equals(0) + }) + + o("removes event added via addEventListener when null", function() { + var spy = o.spy() + var vnode = {tag: "a", attrs: {ontouchstart: spy}} + var updated = {tag: "a", attrs: {ontouchstart: null}} + + render(root, [vnode]) + render(root, [updated]) + + var e = $window.document.createEvent("TouchEvents") + e.initEvent("touchstart", true, true) + vnode.dom.dispatchEvent(e) + + o(spy.callCount).equals(0) + }) + + o("removes event added via addEventListener", function() { + var spy = o.spy() + var vnode = {tag: "a", attrs: {ontouchstart: spy}} + var updated = {tag: "a", attrs: {}} + + render(root, [vnode]) + render(root, [updated]) + + var e = $window.document.createEvent("TouchEvents") + e.initEvent("touchstart", true, true) + vnode.dom.dispatchEvent(e) + + o(spy.callCount).equals(0) + }) + + o("removes event added via addEventListener when undefined", function() { + var spy = o.spy() + var vnode = {tag: "a", attrs: {ontouchstart: spy}} + var updated = {tag: "a", attrs: {ontouchstart: undefined}} + + render(root, [vnode]) + render(root, [updated]) + + var e = $window.document.createEvent("TouchEvents") + e.initEvent("touchstart", true, true) + vnode.dom.dispatchEvent(e) + + o(spy.callCount).equals(0) + }) + + o("removes EventListener object", function() { + var spy = o.spy() + var listener = {handleEvent: spy} + var vnode = {tag: "a", attrs: {onclick: listener}} + var updated = {tag: "a", attrs: {}} + + render(root, [vnode]) + render(root, [updated]) + + var e = $window.document.createEvent("MouseEvents") + e.initEvent("click", true, true) + vnode.dom.dispatchEvent(e) + + o(spy.callCount).equals(0) + }) + + o("removes EventListener object when null", function() { + var spy = o.spy() + var listener = {handleEvent: spy} + var vnode = {tag: "a", attrs: {onclick: listener}} + var updated = {tag: "a", attrs: {onclick: null}} + + render(root, [vnode]) + render(root, [updated]) + + var e = $window.document.createEvent("MouseEvents") + e.initEvent("click", true, true) + vnode.dom.dispatchEvent(e) + + o(spy.callCount).equals(0) + }) + + o("removes EventListener object when undefined", function() { + var spy = o.spy() + var listener = {handleEvent: spy} + var vnode = {tag: "a", attrs: {onclick: listener}} + var updated = {tag: "a", attrs: {onclick: undefined}} + + render(root, [vnode]) + render(root, [updated]) + + var e = $window.document.createEvent("MouseEvents") + e.initEvent("click", true, true) + vnode.dom.dispatchEvent(e) + o(spy.callCount).equals(0) }) @@ -72,6 +215,30 @@ o.spec("event", function() { o(div.dom.attributes["id"].value).equals("b") }) + o("fires click EventListener object only once after redraw", function() { + var spy = o.spy() + var listener = {handleEvent: spy} + var div = {tag: "div", attrs: {id: "a", onclick: listener}} + var updated = {tag: "div", attrs: {id: "b", onclick: listener}} + var e = $window.document.createEvent("MouseEvents") + e.initEvent("click", true, true) + + render(root, [div]) + render(root, [updated]) + div.dom.dispatchEvent(e) + + o(spy.callCount).equals(1) + o(spy.this).equals(listener) + o(spy.args[0].type).equals("click") + o(spy.args[0].target).equals(div.dom) + o(onevent.callCount).equals(1) + o(onevent.this).equals(div.dom) + o(onevent.args[0].type).equals("click") + o(onevent.args[0].target).equals(div.dom) + o(div.dom).equals(updated.dom) + o(div.dom.attributes["id"].value).equals("b") + }) + o("handles ontransitionend", function() { var spy = o.spy() var div = {tag: "div", attrs: {ontransitionend: spy}} @@ -90,4 +257,24 @@ o.spec("event", function() { o(onevent.args[0].type).equals("transitionend") o(onevent.args[0].target).equals(div.dom) }) + + o("handles transitionend EventListener object", function() { + var spy = o.spy() + var listener = {handleEvent: spy} + var div = {tag: "div", attrs: {ontransitionend: listener}} + var e = $window.document.createEvent("HTMLEvents") + e.initEvent("transitionend", true, true) + + render(root, [div]) + div.dom.dispatchEvent(e) + + o(spy.callCount).equals(1) + o(spy.this).equals(listener) + o(spy.args[0].type).equals("transitionend") + o(spy.args[0].target).equals(div.dom) + o(onevent.callCount).equals(1) + o(onevent.this).equals(div.dom) + o(onevent.args[0].type).equals("transitionend") + o(onevent.args[0].target).equals(div.dom) + }) }) diff --git a/test-utils/domMock.js b/test-utils/domMock.js index 060ba4f7..d99dce47 100644 --- a/test-utils/domMock.js +++ b/test-utils/domMock.js @@ -278,7 +278,9 @@ module.exports = function(options) { e.target = this if (events[e.type] != null) { for (var i = 0; i < events[e.type].length; i++) { - events[e.type][i].call(this, e) + var handler = events[e.type][i] + if (typeof handler === "function") handler.call(this, e) + else handler.handleEvent(e) } } e.preventDefault = function() {