From 536c90e525335f42b37c765d69427e1d1329d69e Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Sat, 3 Aug 2019 12:47:49 -0400 Subject: [PATCH 1/7] If image source is already in data uri form, process synchronously This causes the image source to be updated at the same time as the image location. --- src/components/images/draw.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/components/images/draw.js b/src/components/images/draw.js index 3fe1080fc32..3c0032d4c39 100644 --- a/src/components/images/draw.js +++ b/src/components/images/draw.js @@ -84,6 +84,11 @@ module.exports = function draw(gd) { var img = new Image(); this.img = img; + if (d.source && d.source.slice(0, 5) === 'data:') { + img.src = d.source; + thisImage.attr('xlink:href', d.source); + resolve(); + } else { // If not set, a `tainted canvas` error is thrown img.setAttribute('crossOrigin', 'anonymous'); img.onerror = errorHandler; @@ -109,6 +114,7 @@ module.exports = function draw(gd) { thisImage.on('error', errorHandler); img.src = d.source; + } function errorHandler() { thisImage.remove(); From 0a397c4eafa29de329b32b3bc154444238adf2b8 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Sat, 3 Aug 2019 12:49:47 -0400 Subject: [PATCH 2/7] Apply indentation This commit is the indentation that was omitted from previous commit for clarity --- src/components/images/draw.js | 36 +++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/components/images/draw.js b/src/components/images/draw.js index 3c0032d4c39..2b80d5b7597 100644 --- a/src/components/images/draw.js +++ b/src/components/images/draw.js @@ -89,31 +89,31 @@ module.exports = function draw(gd) { thisImage.attr('xlink:href', d.source); resolve(); } else { - // If not set, a `tainted canvas` error is thrown - img.setAttribute('crossOrigin', 'anonymous'); - img.onerror = errorHandler; - img.onload = function() { - var canvas = document.createElement('canvas'); - canvas.width = this.width; - canvas.height = this.height; + // If not set, a `tainted canvas` error is thrown + img.setAttribute('crossOrigin', 'anonymous'); + img.onerror = errorHandler; + img.onload = function() { + var canvas = document.createElement('canvas'); + canvas.width = this.width; + canvas.height = this.height; - var ctx = canvas.getContext('2d'); - ctx.drawImage(this, 0, 0); + var ctx = canvas.getContext('2d'); + ctx.drawImage(this, 0, 0); - var dataURL = canvas.toDataURL('image/png'); + var dataURL = canvas.toDataURL('image/png'); - thisImage.attr('xlink:href', dataURL); + thisImage.attr('xlink:href', dataURL); - // resolve promise in onload handler instead of on 'load' to support IE11 - // see https://github.com/plotly/plotly.js/issues/1685 - // for more details - resolve(); - }; + // resolve promise in onload handler instead of on 'load' to support IE11 + // see https://github.com/plotly/plotly.js/issues/1685 + // for more details + resolve(); + }; - thisImage.on('error', errorHandler); + thisImage.on('error', errorHandler); - img.src = d.source; + img.src = d.source; } function errorHandler() { From 611064daf63b646677c17eae797a41ccd9b478cc Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Sat, 3 Aug 2019 13:34:05 -0400 Subject: [PATCH 3/7] lint fix --- src/components/images/draw.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/images/draw.js b/src/components/images/draw.js index 2b80d5b7597..bc895cce5ed 100644 --- a/src/components/images/draw.js +++ b/src/components/images/draw.js @@ -84,7 +84,7 @@ module.exports = function draw(gd) { var img = new Image(); this.img = img; - if (d.source && d.source.slice(0, 5) === 'data:') { + if(d.source && d.source.slice(0, 5) === '') { img.src = d.source; thisImage.attr('xlink:href', d.source); resolve(); From 86a3fc000f82edc3748a4e7623c66fd476f210dc Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Sat, 3 Aug 2019 13:35:42 -0400 Subject: [PATCH 4/7] Restore "data:" string --- src/components/images/draw.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/images/draw.js b/src/components/images/draw.js index bc895cce5ed..a114b36bef8 100644 --- a/src/components/images/draw.js +++ b/src/components/images/draw.js @@ -84,7 +84,7 @@ module.exports = function draw(gd) { var img = new Image(); this.img = img; - if(d.source && d.source.slice(0, 5) === '') { + if(d.source && d.source.slice(0, 5) === 'data:') { img.src = d.source; thisImage.attr('xlink:href', d.source); resolve(); From c310c92ac0372cfe4c21e413cea73381ab008b4d Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 7 Aug 2019 06:14:08 -0400 Subject: [PATCH 5/7] Don't create promise or invoke onload for data uri images - Stash prior input image src string as the this._imgSrc property - Pull datauti code outside of the promise, so that we skip creating the promise and skip creating an Image() object, and skip running the onload function. --- src/components/images/draw.js | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/components/images/draw.js b/src/components/images/draw.js index a114b36bef8..603e46b059e 100644 --- a/src/components/images/draw.js +++ b/src/components/images/draw.js @@ -74,21 +74,20 @@ module.exports = function draw(gd) { function setImage(d) { var thisImage = d3.select(this); - if(this.img && this.img.src === d.source) { + if(this._imgSrc === d.source) { return; } thisImage.attr('xmlns', xmlnsNamespaces.svg); - var imagePromise = new Promise(function(resolve) { - var img = new Image(); - this.img = img; + if(d.source && d.source.slice(0, 5) === 'data:') { + thisImage.attr('xlink:href', d.source); + this._imgSrc = d.source; + } else { + var imagePromise = new Promise(function(resolve) { + var img = new Image(); + this.img = img; - if(d.source && d.source.slice(0, 5) === 'data:') { - img.src = d.source; - thisImage.attr('xlink:href', d.source); - resolve(); - } else { // If not set, a `tainted canvas` error is thrown img.setAttribute('crossOrigin', 'anonymous'); img.onerror = errorHandler; @@ -110,19 +109,19 @@ module.exports = function draw(gd) { resolve(); }; - thisImage.on('error', errorHandler); img.src = d.source; - } + this._imgSrc = d.source; - function errorHandler() { - thisImage.remove(); - resolve(); - } - }.bind(this)); + function errorHandler() { + thisImage.remove(); + resolve(); + } + }.bind(this)); - gd._promises.push(imagePromise); + gd._promises.push(imagePromise); + } } function applyAttributes(d) { From dd585b2fddc40d32bd4dd1b1cf5c40d0401b1ba5 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 7 Aug 2019 08:14:00 -0400 Subject: [PATCH 6/7] Add test that a canvas element is not created when image is a data uri --- test/jasmine/tests/layout_images_test.js | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/jasmine/tests/layout_images_test.js b/test/jasmine/tests/layout_images_test.js index ce1f47b9de6..0b994f309ff 100644 --- a/test/jasmine/tests/layout_images_test.js +++ b/test/jasmine/tests/layout_images_test.js @@ -12,6 +12,10 @@ var mouseEvent = require('../assets/mouse_event'); var jsLogo = 'https://images.plot.ly/language-icons/api-home/js-logo.png'; var pythonLogo = 'https://images.plot.ly/language-icons/api-home/python-logo.png'; +// Single red pixel png generated with http://png-pixel.com/ +var dataUriImage = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfF' + + 'cSJAAAADUlEQVR42mP8z8DwHwAFBQIAX8jx0gAAAABJRU5ErkJggg=='; + describe('Layout images', function() { describe('supplyLayoutDefaults', function() { var layoutIn, @@ -290,6 +294,31 @@ describe('Layout images', function() { afterEach(destroyGraphDiv); + it('should only create canvas if url image', function(done) { + var originalCreateElement = document.createElement; + var newCanvasElement; + spyOn(document, 'createElement').and.callFake(function(elementType) { + var element = originalCreateElement.call(document, elementType); + if(elementType === 'canvas') { + newCanvasElement = element; + spyOn(newCanvasElement, 'toDataURL'); + } + return element; + }); + + Plotly.relayout(gd, 'images[0].source', dataUriImage).then(function() { + setTimeout(function() { + expect(newCanvasElement).toBeUndefined(); + Plotly.relayout(gd, 'images[0].source', pythonLogo).then(function() { + expect(newCanvasElement).eventually.toBeDefined(); + expect(newCanvasElement.toDataURL).toHaveBeenCalledTimes(1); + }); + + done(); + }, 500); + }); + }); + it('should update the image if changed', function(done) { var img = Plotly.d3.select('image'); var url = img.attr('xlink:href'); From 4a1afbdf54fcb7e2fe1dae2518af40b758203c52 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 7 Aug 2019 10:42:38 -0400 Subject: [PATCH 7/7] Restructure test to avoid setTimeout --- test/jasmine/tests/layout_images_test.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/jasmine/tests/layout_images_test.js b/test/jasmine/tests/layout_images_test.js index 0b994f309ff..0cdc86faa30 100644 --- a/test/jasmine/tests/layout_images_test.js +++ b/test/jasmine/tests/layout_images_test.js @@ -306,17 +306,17 @@ describe('Layout images', function() { return element; }); - Plotly.relayout(gd, 'images[0].source', dataUriImage).then(function() { - setTimeout(function() { - expect(newCanvasElement).toBeUndefined(); - Plotly.relayout(gd, 'images[0].source', pythonLogo).then(function() { - expect(newCanvasElement).eventually.toBeDefined(); - expect(newCanvasElement.toDataURL).toHaveBeenCalledTimes(1); - }); - - done(); - }, 500); - }); + Plotly.relayout(gd, 'images[0].source', dataUriImage) + .then(function() { + expect(newCanvasElement).toBeUndefined(); + }) + .then(function() { return Plotly.relayout(gd, 'images[0].source', jsLogo); }) + .then(function() { + expect(newCanvasElement).toBeDefined(); + expect(newCanvasElement.toDataURL).toHaveBeenCalledTimes(1); + }) + .catch(failTest) + .then(done); }); it('should update the image if changed', function(done) {