Skip to content

Commit

Permalink
Fix non-passive event listener warning in Chrome
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
simonbrunel authored and etimberg committed Jun 25, 2017
1 parent 7fa6052 commit 548edc6
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 22 deletions.
18 changes: 0 additions & 18 deletions src/core/core.helpers.js
Expand Up @@ -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) {
Expand Down
62 changes: 58 additions & 4 deletions src/platforms/platform.dom.js
Expand Up @@ -92,6 +92,38 @@ module.exports = function(Chart) {
return canvas;
}

/**
* Detects support for options object argument in addEventListener.
* https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Safely_detecting_option_support
* @private
*/
var supportsEventListenerOptions = (function() {
var supports = false;
try {
var options = Object.defineProperty({}, 'passive', {
get: function() {
supports = true;
}
});
window.addEventListener('e', null, options);
} catch (e) {
// continue regardless of error
}
return supports;
}());

// Default passive to true as expected by Chrome for 'touchstart' and 'touchend' events.
// https://github.com/chartjs/Chart.js/issues/4287
var eventListenerOptions = supportsEventListenerOptions? {passive: true} : false;

function addEventListener(node, type, listener) {
node.addEventListener(type, listener, eventListenerOptions);
}

function removeEventListener(node, type, listener) {
node.removeEventListener(type, listener, eventListenerOptions);
}

function createEvent(type, chart, x, y, nativeEvent) {
return {
type: type,
Expand Down Expand Up @@ -137,8 +169,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.
Expand Down Expand Up @@ -186,6 +218,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') {
Expand Down Expand Up @@ -263,7 +317,7 @@ module.exports = function(Chart) {
listener(fromNativeEvent(event, chart));
};

helpers.addEvent(canvas, type, proxy);
addEventListener(canvas, type, proxy);
},

removeEventListener: function(chart, type, listener) {
Expand All @@ -281,7 +335,7 @@ module.exports = function(Chart) {
return;
}

helpers.removeEvent(canvas, type, proxy);
removeEventListener(canvas, type, proxy);
}
};
};
27 changes: 27 additions & 0 deletions test/specs/global.deprecations.tests.js
Expand Up @@ -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() {
Expand Down

0 comments on commit 548edc6

Please sign in to comment.