Skip to content

Commit

Permalink
Enhance context acquisition on chart creation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
simonbrunel authored and etimberg committed Oct 16, 2016
1 parent 6ec6a92 commit 4a5b5a0
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 57 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@

.DS_Store
.idea
.vscode
bower.json
1 change: 1 addition & 0 deletions docs/00-Getting-Started.md
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
61 changes: 52 additions & 9 deletions src/core/core.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down
19 changes: 3 additions & 16 deletions src/core/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
70 changes: 69 additions & 1 deletion test/core.controller.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
90 changes: 59 additions & 31 deletions test/mockContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
});
Expand Down

0 comments on commit 4a5b5a0

Please sign in to comment.