Enhance context acquisition on chart creation

Add support for creating a chart from the canvas id and prevent exceptions, at construction time, when the given item doesn't provide a valid CanvasRenderingContext2D or when the getContext API is not accessible (e.g. undefined by add-ons to prevent fingerprinting). New jasmine matcher to verify chart validity.

.gitignore

@@ -6,4 +6,5 @@
 
 .DS_Store
 .idea
+.vscode
 bower.json

docs/00-Getting-Started.md

@@ -71,6 +71,7 @@ To create a chart, we need to instantiate the `Chart` class. To do this, we need
 var ctx = document.getElementById("myChart");
 var ctx = document.getElementById("myChart").getContext("2d");
 var ctx = $("#myChart");
+var ctx = "myChart";
 ```
 
 Once you have the element or context, you're ready to instantiate a pre-defined chart-type or create your own!

src/core/core.controller.js

@@ -112,6 +112,36 @@ module.exports = function(Chart) {
 		delete canvas._chartjs;
 	}
 
+	/**
+	 * TODO(SB) Move this method in the upcoming core.platform class.
+	 */
+	function acquireContext(item, config) {
+		if (typeof item === 'string') {
+			item = document.getElementById(item);
+		} else if (item.length) {
+			// Support for array based queries (such as jQuery)
+			item = item[0];
+		}
+
+		if (item && item.canvas) {
+			// Support for any object associated to a canvas (including a context2d)
+			item = item.canvas;
+		}
+
+		if (item instanceof HTMLCanvasElement) {
+			// To prevent canvas fingerprinting, some add-ons undefine the getContext
+			// method, for example: https://github.com/kkapsner/CanvasBlocker
+			// https://github.com/chartjs/Chart.js/issues/2807
+			var context = item.getContext && item.getContext('2d');
+			if (context instanceof CanvasRenderingContext2D) {
+				initCanvas(item, config);
+				return context;
+			}
+		}
+
+		return null;
+	}
+
 	/**
 	 * Initializes the given config with global and chart default values.
 	 */
@@ -136,21 +166,22 @@ module.exports = function(Chart) {
 	 * @class Chart.Controller
 	 * The main controller of a chart.
 	 */
-	Chart.Controller = function(context, config, instance) {
+	Chart.Controller = function(item, config, instance) {
 		var me = this;
-		var canvas;
 
 		config = initConfig(config);
-		canvas = initCanvas(context.canvas, config);
+
+		var context = acquireContext(item, config);
+		var canvas = context && context.canvas;
+		var height = canvas && canvas.height;
+		var width = canvas && canvas.width;
 
 		instance.ctx = context;
 		instance.canvas = canvas;
 		instance.config = config;
-		instance.width = canvas.width;
+		instance.width = width;
-		instance.height = canvas.height;
+		instance.height = height;
-		instance.aspectRatio = canvas.width / canvas.height;
+		instance.aspectRatio = height? width / height : null;
-
-		helpers.retinaScale(instance);
 
 		me.id = helpers.uid();
 		me.chart = instance;
@@ -167,6 +198,17 @@ module.exports = function(Chart) {
 			}
 		});
 
+		if (!context || !canvas) {
+			// The given item is not a compatible context2d element, let's return before finalizing
+			// the chart initialization but after setting basic chart / controller properties that
+			// can help to figure out that the chart is not valid (e.g chart.canvas !== null);
+			// https://github.com/chartjs/Chart.js/issues/2807
+			console.error("Failed to create chart: can't acquire context from the given item");
+			return me;
+		}
+
+		helpers.retinaScale(instance);
+
 		// Responsiveness is currently based on the use of an iframe, however this method causes
 		// performance issues and could be troublesome when used with ad blockers. So make sure
 		// that the user is still able to create a chart without iframe when responsive is false.
@@ -593,7 +635,6 @@ module.exports = function(Chart) {
 			var meta, i, ilen;
 
 			me.stop();
-			me.clear();
 
 			// dataset controllers need to cleanup associated data
 			for (i = 0, ilen = me.data.datasets.length; i < ilen; ++i) {
@@ -607,8 +648,10 @@ module.exports = function(Chart) {
 			if (canvas) {
 				helpers.unbindEvents(me, me.events);
 				helpers.removeResizeListener(canvas.parentNode);
+				helpers.clear(me.chart);
 				releaseCanvas(canvas);
 				me.chart.canvas = null;
+				me.chart.ctx = null;
 			}
 
 			// if we scaled the canvas in response to a devicePixelRatio !== 1, we need to undo that transform here

src/core/core.js

@@ -3,22 +3,9 @@
 module.exports = function() {
 
 	// Occupy the global variable of Chart, and create a simple base class
-	var Chart = function(context, config) {
+	var Chart = function(item, config) {
-		var me = this;
+		this.controller = new Chart.Controller(item, config, this);
-
+		return this.controller;
-		// Support a jQuery'd canvas element
-		if (context.length && context[0].getContext) {
-			context = context[0];
-		}
-
-		// Support a canvas domnode
-		if (context.getContext) {
-			context = context.getContext('2d');
-		}
-
-		me.controller = new Chart.Controller(context, config, me);
-
-		return me.controller;
 	};
 
 	// Globally expose the defaults to allow for user updating/changing

test/core.controller.tests.js

@@ -4,7 +4,75 @@ describe('Chart.Controller', function() {
 		setTimeout(callback, 100);
 	}
 
-	describe('config', function() {
+	describe('context acquisition', function() {
+		var canvasId = 'chartjs-canvas';
+
+		beforeEach(function() {
+			var canvas = document.createElement('canvas');
+			canvas.setAttribute('id', canvasId);
+			window.document.body.appendChild(canvas);
+		});
+
+		afterEach(function() {
+			document.getElementById(canvasId).remove();
+		});
+
+		// see https://github.com/chartjs/Chart.js/issues/2807
+		it('should gracefully handle invalid item', function() {
+			var chart = new Chart('foobar');
+
+			expect(chart).not.toBeValidChart();
+
+			chart.destroy();
+		});
+
+		it('should accept a DOM element id', function() {
+			var canvas = document.getElementById(canvasId);
+			var chart = new Chart(canvasId);
+
+			expect(chart).toBeValidChart();
+			expect(chart.chart.canvas).toBe(canvas);
+			expect(chart.chart.ctx).toBe(canvas.getContext('2d'));
+
+			chart.destroy();
+		});
+
+		it('should accept a canvas element', function() {
+			var canvas = document.getElementById(canvasId);
+			var chart = new Chart(canvas);
+
+			expect(chart).toBeValidChart();
+			expect(chart.chart.canvas).toBe(canvas);
+			expect(chart.chart.ctx).toBe(canvas.getContext('2d'));
+
+			chart.destroy();
+		});
+
+		it('should accept a canvas context2D', function() {
+			var canvas = document.getElementById(canvasId);
+			var context = canvas.getContext('2d');
+			var chart = new Chart(context);
+
+			expect(chart).toBeValidChart();
+			expect(chart.chart.canvas).toBe(canvas);
+			expect(chart.chart.ctx).toBe(context);
+
+			chart.destroy();
+		});
+
+		it('should accept an array containing canvas', function() {
+			var canvas = document.getElementById(canvasId);
+			var chart = new Chart([canvas]);
+
+			expect(chart).toBeValidChart();
+			expect(chart.chart.canvas).toBe(canvas);
+			expect(chart.chart.ctx).toBe(canvas.getContext('2d'));
+
+			chart.destroy();
+		});
+	});
+
+	describe('config initialization', function() {
 		it('should create missing config.data properties', function() {
 			var chart = acquireChart({});
 			var data = chart.data;

test/mockContext.js

@@ -158,22 +158,50 @@
 		};
 	}
 
+	function toBeValidChart() {
+		return {
+			compare: function(actual) {
+				var chart = actual && actual.chart;
+				var message = null;
+
+				if (!(actual instanceof Chart.Controller)) {
+					message = 'Expected ' + actual + ' to be an instance of Chart.Controller';
+				} else if (!(chart instanceof Chart)) {
+					message = 'Expected chart to be an instance of Chart';
+				} else if (!(chart.canvas instanceof HTMLCanvasElement)) {
+					message = 'Expected canvas to be an instance of HTMLCanvasElement';
+				} else if (!(chart.ctx instanceof CanvasRenderingContext2D)) {
+					message = 'Expected context to be an instance of CanvasRenderingContext2D';
+				} else if (typeof chart.height !== 'number' || !isFinite(chart.height)) {
+					message = 'Expected height to be a strict finite number';
+				} else if (typeof chart.width !== 'number' || !isFinite(chart.width)) {
+					message = 'Expected width to be a strict finite number';
+				}
+
+				return {
+					message: message? message : 'Expected ' + actual + ' to be valid chart',
+					pass: !message
+				};
+			}
+		};
+	}
+
 	function toBeChartOfSize() {
 		return {
 			compare: function(actual, expected) {
-				var message = null;
+				var res = toBeValidChart().compare(actual);
-				var chart, canvas, style, dh, dw, rh, rw;
+				if (!res.pass) {
+					return res;
+				}
 
-				if (!actual || !(actual instanceof Chart.Controller)) {
+				var message = null;
-					message = 'Expected ' + actual + ' to be an instance of Chart.Controller.';
+				var chart = actual.chart;
-				} else {
+				var canvas = chart.ctx.canvas;
-					chart = actual.chart;
+				var style = getComputedStyle(canvas);
-					canvas = chart.ctx.canvas;
+				var dh = parseInt(style.height, 10);
-					style = getComputedStyle(canvas);
+				var dw = parseInt(style.width, 10);
-					dh = parseInt(style.height);
+				var rh = canvas.height;
-					dw = parseInt(style.width);
+				var rw = canvas.width;
-					rh = canvas.height;
-					rw = canvas.width;
 
 				// sanity checks
 				if (chart.height !== rh) {
@@ -192,20 +220,20 @@
 				} else if (rw !== expected.rw) {
 					message = 'Expected render width ' + rw + ' to be equal to ' + expected.rw;
 				}
-				}
 
 				return {
 					message: message? message : 'Expected ' + actual + ' to be a chart of size ' + expected,
 					pass: !message
+				};
 			}
-			}
+		};
-		}
 	}
 
 	beforeEach(function() {
 		jasmine.addMatchers({
 			toBeCloseToPixel: toBeCloseToPixel,
 			toEqualOneOf: toEqualOneOf,
+			toBeValidChart: toBeValidChart,
 			toBeChartOfSize: toBeChartOfSize
 		});
 	});