From 5c079ff6de121f48744b53df9c0a92362f7400c4 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Sun, 25 Jun 2017 12:14:28 +0200 Subject: [PATCH] Fix non-passive event listener warning in Chrome Deprecate `addEvent` and `removeEvent`, and move implementation in `platform.dom.js`. Add 'options' feature detection to register event listeners as passive and prevent warning in Chrome. --- src/core/core.helpers.js | 18 ------- src/platforms/platform.dom.js | 67 +++++++++++++++++++++++-- test/specs/global.deprecations.tests.js | 27 ++++++++++ 3 files changed, 90 insertions(+), 22 deletions(-) diff --git a/src/core/core.helpers.js b/src/core/core.helpers.js index 98d98793214..08470fa7b47 100644 --- a/src/core/core.helpers.js +++ b/src/core/core.helpers.js @@ -666,24 +666,6 @@ module.exports = function(Chart) { }; }; - helpers.addEvent = function(node, eventType, method) { - if (node.addEventListener) { - node.addEventListener(eventType, method); - } else if (node.attachEvent) { - node.attachEvent('on' + eventType, method); - } else { - node['on' + eventType] = method; - } - }; - helpers.removeEvent = function(node, eventType, handler) { - if (node.removeEventListener) { - node.removeEventListener(eventType, handler, false); - } else if (node.detachEvent) { - node.detachEvent('on' + eventType, handler); - } else { - node['on' + eventType] = helpers.noop; - } - }; // Private helper function to convert max-width/max-height values that may be percentages into a number function parseMaxStyle(styleValue, node, parentProperty) { diff --git a/src/platforms/platform.dom.js b/src/platforms/platform.dom.js index 09c23820bf7..b00b6c833fb 100644 --- a/src/platforms/platform.dom.js +++ b/src/platforms/platform.dom.js @@ -92,6 +92,43 @@ module.exports = function(Chart) { return canvas; } + // Detects support for the passive option to addEventListener + // https://github.com/chartjs/Chart.js/issues/4287 + // https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Safely_detecting_option_support + var supportsPassiveOption = (function() { + var supports = false; + try { + window.addEventListener('test', null, Object.defineProperty({}, 'passive', { + get: function() { + supports = true; + } + })); + } catch (e) { + // continue regardless of error + } + return supports; + }()); + + function addEventListener(node, type, listener) { + if (node.addEventListener) { + node.addEventListener(type, listener, supportsPassiveOption? {passive: true} : false); + } else if (node.attachEvent) { + node.attachEvent('on' + type, listener); + } else { + node['on' + type] = listener; + } + } + + function removeEventListener(node, type, listener) { + if (node.removeEventListener) { + node.removeEventListener(type, listener, supportsPassiveOption? {passive: true} : false); + } else if (node.detachEvent) { + node.detachEvent('on' + type, listener); + } else { + node['on' + type] = helpers.noop; + } + } + function createEvent(type, chart, x, y, nativeEvent) { return { type: type, @@ -137,8 +174,8 @@ module.exports = function(Chart) { // If the iframe is re-attached to the DOM, the resize listener is removed because the // content is reloaded, so make sure to install the handler after the iframe is loaded. // https://github.com/chartjs/Chart.js/issues/3521 - helpers.addEvent(iframe, 'load', function() { - helpers.addEvent(iframe.contentWindow || iframe, 'resize', handler); + addEventListener(iframe, 'load', function() { + addEventListener(iframe.contentWindow || iframe, 'resize', handler); // The iframe size might have changed while loading, which can also // happen if the size has been changed while detached from the DOM. @@ -186,6 +223,28 @@ module.exports = function(Chart) { delete node._chartjs; } + /** + * Provided for backward compatibility, use EventTarget.addEventListener instead. + * EventTarget.addEventListener compatibility: Chrome, Opera 7, Safari, FF1.5+, IE9+ + * @see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener + * @function Chart.helpers.addEvent + * @deprecated since version 2.7.0 + * @todo remove at version 3 + * @private + */ + helpers.addEvent = addEventListener; + + /** + * Provided for backward compatibility, use EventTarget.removeEventListener instead. + * EventTarget.removeEventListener compatibility: Chrome, Opera 7, Safari, FF1.5+, IE9+ + * @see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener + * @function Chart.helpers.removeEvent + * @deprecated since version 2.7.0 + * @todo remove at version 3 + * @private + */ + helpers.removeEvent = removeEventListener; + return { acquireContext: function(item, config) { if (typeof item === 'string') { @@ -263,7 +322,7 @@ module.exports = function(Chart) { listener(fromNativeEvent(event, chart)); }; - helpers.addEvent(canvas, type, proxy); + addEventListener(canvas, type, proxy); }, removeEventListener: function(chart, type, listener) { @@ -281,7 +340,7 @@ module.exports = function(Chart) { return; } - helpers.removeEvent(canvas, type, proxy); + removeEventListener(canvas, type, proxy); } }; }; diff --git a/test/specs/global.deprecations.tests.js b/test/specs/global.deprecations.tests.js index 5eeded72580..5731da24639 100644 --- a/test/specs/global.deprecations.tests.js +++ b/test/specs/global.deprecations.tests.js @@ -101,6 +101,33 @@ describe('Deprecations', function() { expect(Chart.helpers.canvas.roundedRect).toHaveBeenCalledWith(ctx, 10, 20, 30, 40, 5); }); }); + + describe('Chart.helpers.addEvent', function() { + it('should be defined and a function', function() { + expect(Chart.helpers.addEvent).toBeDefined(); + expect(typeof Chart.helpers.addEvent).toBe('function'); + }); + it('should correctly add event listener', function() { + var listener = jasmine.createSpy('spy'); + Chart.helpers.addEvent(window, 'test', listener); + window.dispatchEvent(new Event('test')); + expect(listener).toHaveBeenCalled(); + }); + }); + + describe('Chart.helpers.removeEvent', function() { + it('should be defined and a function', function() { + expect(Chart.helpers.removeEvent).toBeDefined(); + expect(typeof Chart.helpers.removeEvent).toBe('function'); + }); + it('should correctly remove event listener', function() { + var listener = jasmine.createSpy('spy'); + Chart.helpers.addEvent(window, 'test', listener); + Chart.helpers.removeEvent(window, 'test', listener); + window.dispatchEvent(new Event('test')); + expect(listener).not.toHaveBeenCalled(); + }); + }); }); describe('Version 2.6.0', function() {