Inject iframe for responsive charts only

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.

diff --git a/src/core/core.controller.js b/src/core/core.controller.js
index 1d169adeac27701848b4a7273009d6d2f2c3e5a4..e335c3a3a7c9958522c373d2321c8b1506f24d37 100644 (file)
--- a/src/core/core.controller.js
+++ b/src/core/core.controller.js
@@ -151,8 +151,11 @@ module.exports = function(Chart) {
 
                me.id = helpers.uid();
                me.chart = instance;
-               me.config = instance.config;
-               me.options = me.config.options;
+               me.config = config;
+               me.options = config.options;
+
+               // Add the chart instance to the global namespace
+               Chart.instances[me.id] = me;
 
                Object.defineProperty(me, 'data', {
                        get: function() {
@@ -160,18 +163,16 @@ module.exports = function(Chart) {
                        }
                });
 
-               // Always bind this so that if the responsive state changes we still work
-               helpers.addResizeListener(canvas.parentNode, function() {
-                       if (me.config.options.responsive) {
+               // 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.
+               // See https://github.com/chartjs/Chart.js/issues/2210
+               if (me.options.responsive) {
+                       helpers.addResizeListener(canvas.parentNode, function() {
                                me.resize();
-                       }
-               });
-
-               // Add the chart instance to the global namespace
-               Chart.instances[me.id] = me;
+                       });
 
-               if (me.options.responsive) {
-                       // Silent resize before chart draws
+                       // Initial resize before chart draws (must be silent to preserve initial animations).
                        me.resize(true);
                }
 
@@ -684,11 +685,11 @@ module.exports = function(Chart) {
                        me.stop();
                        me.clear();
 
-                       helpers.unbindEvents(me, me.events);
-
                        if (canvas) {
+                               helpers.unbindEvents(me, me.events);
                                helpers.removeResizeListener(canvas.parentNode);
                                releaseCanvas(canvas);
+                               me.chart.canvas = 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.helpers.js b/src/core/core.helpers.js
index e404b61b1cdaf90adb49d2df33243fa82a689289..6d63284a7099d31ccf57d74a553a84a7b7fee8ca 100644 (file)
--- a/src/core/core.helpers.js
+++ b/src/core/core.helpers.js
@@ -980,15 +980,24 @@ module.exports = function(Chart) {
                // Insert the iframe so that contentWindow is available
                node.insertBefore(iframe, node.firstChild);
 
+               // Let's keep track of this added iframe and thus avoid DOM query when removing it.
+               node._chartjs = {
+                       resizer: iframe
+               };
+
                this.addEvent(iframe.contentWindow || iframe, 'resize', callback);
        };
        helpers.removeResizeListener = function(node) {
-               var hiddenIframe = node.querySelector('.chartjs-hidden-iframe');
+               if (!node || !node._chartjs) {
+                       return;
+               }
 
-               // Remove the resize detect iframe
-               if (hiddenIframe) {
-                       hiddenIframe.parentNode.removeChild(hiddenIframe);
+               var iframe = node._chartjs.resizer;
+               if (iframe) {
+                       iframe.parentNode.removeChild(iframe);
                }
+
+               delete node._chartjs;
        };
        helpers.isArray = Array.isArray?
                function(obj) {

diff --git a/test/core.controller.tests.js b/test/core.controller.tests.js
index 83eba3baf4d0031fa75d7a8fe8e8543cae29b86d..37661b707307e02864cd10fc62f14208fd3fede7 100644 (file)
--- a/test/core.controller.tests.js
+++ b/test/core.controller.tests.js
@@ -149,6 +149,18 @@ describe('Chart.Controller', function() {
                                rw: 165, rh: 85,
                        });
                });
+
+               it('should NOT inject the resizer element', function() {
+                       var chart = acquireChart({
+                               options: {
+                                       responsive: false
+                               }
+                       });
+
+                       var wrapper = chart.chart.canvas.parentNode;
+                       expect(wrapper.childNodes.length).toBe(1);
+                       expect(wrapper.firstChild.tagName).toBe('CANVAS');
+               });
        });
 
        describe('config.options.responsive: true (maintainAspectRatio: false)', function() {
@@ -449,7 +461,6 @@ describe('Chart.Controller', function() {
        describe('controller.destroy', function() {
                it('should restore canvas (and context) initial values', function(done) {
                        var chart = acquireChart({
-                               type: 'line',
                                options: {
                                        responsive: true,
                                        maintainAspectRatio: false
@@ -484,5 +495,24 @@ describe('Chart.Controller', function() {
                                done();
                        });
                });
+
+               it('should remove the resizer element when responsive: true', function() {
+                       var chart = acquireChart({
+                               options: {
+                                       responsive: true
+                               }
+                       });
+
+                       var wrapper = chart.chart.canvas.parentNode;
+                       var resizer = wrapper.firstChild;
+
+                       expect(wrapper.childNodes.length).toBe(2);
+                       expect(resizer.tagName).toBe('IFRAME');
+
+                       chart.destroy();
+
+                       expect(wrapper.childNodes.length).toBe(1);
+                       expect(wrapper.firstChild.tagName).toBe('CANVAS');
+               });
        });
 });

diff --git a/test/mockContext.js b/test/mockContext.js
index 48e5ce30406e3c7279993af9078e8428b249131f..83b73e31501da03d0520bd648557098a186abf50 100644 (file)
--- a/test/mockContext.js
+++ b/test/mockContext.js
@@ -253,23 +253,23 @@
                window.document.body.appendChild(wrapper);
 
                chart = new Chart(canvas.getContext("2d"), config);
-               chart.__test_persistent = options.persistent;
+               chart._test_persistent = options.persistent;
+               chart._test_wrapper = wrapper;
                charts[chart.id] = chart;
                return chart;
        }
 
        function releaseChart(chart) {
                chart.destroy();
-               chart.chart.canvas.parentNode.remove();
+               chart._test_wrapper.remove();
                delete charts[chart.id];
-               delete chart;
        }
 
        afterEach(function() {
                // Auto releasing acquired charts
                for (var id in charts) {
                        var chart = charts[id];
-                       if (!chart.__test_persistent) {
+                       if (!chart._test_persistent) {
                                releaseChart(chart);
                        }
                }