From 5012dcbdaefa7a11a8dfcd9ff264655e9860ee50 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Mon, 31 Oct 2016 08:47:28 +0100 Subject: [PATCH] Fix iframe resize handler when re-attached to DOM (#3527) When the iframe is attached to the DOM, its content is reloaded (invaliding the resize listener) so make sure to install the handler after the iframe is loaded. Optimize resize events by throttling resize process until the next animation frame. Rewrite the unit test "waitForResize" method, the previous one (timeout) was too weak and most tests was failing on FF. --- src/core/core.helpers.js | 37 +++++++++++---- test/core.controller.tests.js | 85 +++++++++++++++++++++++++++++------ 2 files changed, 100 insertions(+), 22 deletions(-) diff --git a/src/core/core.helpers.js b/src/core/core.helpers.js index 2cafdcb6c..8d911fb15 100644 --- a/src/core/core.helpers.js +++ b/src/core/core.helpers.js @@ -943,9 +943,7 @@ module.exports = function(Chart) { return color(c); }; helpers.addResizeListener = function(node, callback) { - // Hide an iframe before the node var iframe = document.createElement('iframe'); - iframe.className = 'chartjs-hidden-iframe'; iframe.style.cssText = 'display:block;'+ @@ -966,15 +964,37 @@ module.exports = function(Chart) { // https://github.com/chartjs/Chart.js/issues/3090 iframe.tabIndex = -1; - // 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 + var stub = node._chartjs = { + resizer: iframe, + ticking: false + }; + + // Throttle the callback notification until the next animation frame. + var notify = function() { + if (!stub.ticking) { + stub.ticking = true; + helpers.requestAnimFrame.call(window, function() { + if (stub.resizer) { + stub.ticking = false; + return callback(); + } + }); + } }; - this.addEvent(iframe.contentWindow || iframe, 'resize', callback); + // 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', notify); + + // The iframe size might have changed while loading, which can also + // happen if the size has been changed while detached from the DOM. + notify(); + }); + + node.insertBefore(iframe, node.firstChild); }; helpers.removeResizeListener = function(node) { if (!node || !node._chartjs) { @@ -984,6 +1004,7 @@ module.exports = function(Chart) { var iframe = node._chartjs.resizer; if (iframe) { iframe.parentNode.removeChild(iframe); + node._chartjs.resizer = null; } delete node._chartjs; diff --git a/test/core.controller.tests.js b/test/core.controller.tests.js index 07e008f89..13b381060 100644 --- a/test/core.controller.tests.js +++ b/test/core.controller.tests.js @@ -1,7 +1,16 @@ describe('Chart.Controller', function() { - function processResizeEvent(chart, callback) { - setTimeout(callback, 100); + function waitForResize(chart, callback) { + var resizer = chart.chart.canvas.parentNode._chartjs.resizer; + var content = resizer.contentWindow || resizer; + var state = content.document.readyState || 'complete'; + var handler = function() { + Chart.helpers.removeEvent(content, 'load', handler); + Chart.helpers.removeEvent(content, 'resize', handler); + setTimeout(callback, 50); + }; + + Chart.helpers.addEvent(content, state !== 'complete'? 'load' : 'resize', handler); } describe('context acquisition', function() { @@ -358,14 +367,14 @@ describe('Chart.Controller', function() { var wrapper = chart.chart.canvas.parentNode; wrapper.style.width = '455px'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 455, dh: 350, rw: 455, rh: 350, }); wrapper.style.width = '150px'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 150, dh: 350, rw: 150, rh: 350, @@ -398,14 +407,14 @@ describe('Chart.Controller', function() { var wrapper = chart.chart.canvas.parentNode; wrapper.style.height = '455px'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 300, dh: 455, rw: 300, rh: 455, }); wrapper.style.height = '150px'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 300, dh: 150, rw: 300, rh: 150, @@ -440,7 +449,7 @@ describe('Chart.Controller', function() { var wrapper = chart.chart.canvas.parentNode; wrapper.style.height = '355px'; wrapper.style.width = '455px'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 455, dh: 355, rw: 455, rh: 355, @@ -467,7 +476,7 @@ describe('Chart.Controller', function() { var canvas = chart.chart.canvas; canvas.style.display = 'block'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 320, dh: 350, rw: 320, rh: 350, @@ -494,7 +503,7 @@ describe('Chart.Controller', function() { var wrapper = chart.chart.canvas.parentNode; wrapper.style.display = 'block'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 460, dh: 380, rw: 460, rh: 380, @@ -503,6 +512,54 @@ describe('Chart.Controller', function() { done(); }); }); + + // https://github.com/chartjs/Chart.js/issues/3521 + it('should resize the canvas after the wrapper has been re-attached to the DOM', function(done) { + var chart = acquireChart({ + options: { + responsive: true, + maintainAspectRatio: false + } + }, { + canvas: { + style: '' + }, + wrapper: { + style: 'width: 320px; height: 350px' + } + }); + + expect(chart).toBeChartOfSize({ + dw: 320, dh: 350, + rw: 320, rh: 350, + }); + + var wrapper = chart.chart.canvas.parentNode; + var parent = wrapper.parentNode; + parent.removeChild(wrapper); + parent.appendChild(wrapper); + wrapper.style.height = '355px'; + + waitForResize(chart, function() { + expect(chart).toBeChartOfSize({ + dw: 320, dh: 355, + rw: 320, rh: 355, + }); + + parent.removeChild(wrapper); + wrapper.style.width = '455px'; + parent.appendChild(wrapper); + + waitForResize(chart, function() { + expect(chart).toBeChartOfSize({ + dw: 455, dh: 355, + rw: 455, rh: 355, + }); + + done(); + }); + }); + }); }); describe('config.options.responsive: true (maintainAspectRatio: true)', function() { @@ -550,14 +607,14 @@ describe('Chart.Controller', function() { var wrapper = chart.chart.canvas.parentNode; wrapper.style.width = '450px'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 450, dh: 225, rw: 450, rh: 225, }); wrapper.style.width = '150px'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 150, dh: 75, rw: 150, rh: 75, @@ -590,14 +647,14 @@ describe('Chart.Controller', function() { var wrapper = chart.chart.canvas.parentNode; wrapper.style.height = '455px'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 320, dh: 160, rw: 320, rh: 160, }); wrapper.style.height = '150px'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 320, dh: 160, rw: 320, rh: 160, @@ -629,7 +686,7 @@ describe('Chart.Controller', function() { var canvas = chart.chart.canvas; var wrapper = canvas.parentNode; wrapper.style.width = '475px'; - processResizeEvent(chart, function() { + waitForResize(chart, function() { expect(chart).toBeChartOfSize({ dw: 475, dh: 450, rw: 475, rh: 450, -- 2.47.3