Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix non-passive event listener warning in Chrome #4424

Merged
merged 1 commit into from Jun 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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