From 4a5b5a0e7eba85ca44f658375cb0c78e6af93e5c Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Sat, 15 Oct 2016 23:40:22 +0200 Subject: [PATCH] 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 | 1 + docs/00-Getting-Started.md | 1 + src/core/core.controller.js | 61 ++++++++++++++++++++---- src/core/core.js | 19 ++------ test/core.controller.tests.js | 70 ++++++++++++++++++++++++++- test/mockContext.js | 90 +++++++++++++++++++++++------------ 6 files changed, 185 insertions(+), 57 deletions(-) diff --git a/.gitignore b/.gitignore index 478edb5eca4..8ef0139ea96 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,5 @@ .DS_Store .idea +.vscode bower.json diff --git a/docs/00-Getting-Started.md b/docs/00-Getting-Started.md index 910ea5bea0c..b7eb09dfc08 100644 --- a/docs/00-Getting-Started.md +++ b/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! diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 5e3632e4fde..c08d0192864 100644 --- a/src/core/core.controller.js +++ b/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.height = canvas.height; - instance.aspectRatio = canvas.width / canvas.height; - - helpers.retinaScale(instance); + instance.width = width; + instance.height = height; + instance.aspectRatio = height? width / height : null; 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 diff --git a/src/core/core.js b/src/core/core.js index 319219f5d20..cea3cf30bae 100755 --- a/src/core/core.js +++ b/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 me = this; - - // 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; + var Chart = function(item, config) { + this.controller = new Chart.Controller(item, config, this); + return this.controller; }; // Globally expose the defaults to allow for user updating/changing diff --git a/test/core.controller.tests.js b/test/core.controller.tests.js index d7548d4ce1f..a7b504ecdcc 100644 --- a/test/core.controller.tests.js +++ b/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; diff --git a/test/mockContext.js b/test/mockContext.js index 83b73e31501..8904cf88612 100644 --- a/test/mockContext.js +++ b/test/mockContext.js @@ -158,54 +158,82 @@ }; } + 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 res = toBeValidChart().compare(actual); + if (!res.pass) { + return res; + } + var message = null; - var chart, canvas, style, dh, dw, rh, rw; - - if (!actual || !(actual instanceof Chart.Controller)) { - message = 'Expected ' + actual + ' to be an instance of Chart.Controller.'; - } else { - chart = actual.chart; - canvas = chart.ctx.canvas; - style = getComputedStyle(canvas); - dh = parseInt(style.height); - dw = parseInt(style.width); - rh = canvas.height; - rw = canvas.width; - - // sanity checks - if (chart.height !== rh) { - message = 'Expected chart height ' + chart.height + ' to be equal to render height ' + rh; - } else if (chart.width !== rw) { - message = 'Expected chart width ' + chart.width + ' to be equal to render width ' + rw; - } + var chart = actual.chart; + var canvas = chart.ctx.canvas; + var style = getComputedStyle(canvas); + var dh = parseInt(style.height, 10); + var dw = parseInt(style.width, 10); + var rh = canvas.height; + var rw = canvas.width; + + // sanity checks + if (chart.height !== rh) { + message = 'Expected chart height ' + chart.height + ' to be equal to render height ' + rh; + } else if (chart.width !== rw) { + message = 'Expected chart width ' + chart.width + ' to be equal to render width ' + rw; + } - // validity checks - if (dh !== expected.dh) { - message = 'Expected display height ' + dh + ' to be equal to ' + expected.dh; - } else if (dw !== expected.dw) { - message = 'Expected display width ' + dw + ' to be equal to ' + expected.dw; - } else if (rh !== expected.rh) { - message = 'Expected render height ' + rh + ' to be equal to ' + expected.rh; - } else if (rw !== expected.rw) { - message = 'Expected render width ' + rw + ' to be equal to ' + expected.rw; - } + // validity checks + if (dh !== expected.dh) { + message = 'Expected display height ' + dh + ' to be equal to ' + expected.dh; + } else if (dw !== expected.dw) { + message = 'Expected display width ' + dw + ' to be equal to ' + expected.dw; + } else if (rh !== expected.rh) { + message = 'Expected render height ' + rh + ' to be equal to ' + expected.rh; + } 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 }); });