From cd7d43e1cd793c3c2a0d485f8ec24c027a8de468 Mon Sep 17 00:00:00 2001 From: Stefan Hayden Date: Fri, 9 Feb 2018 16:55:03 -0500 Subject: [PATCH 1/3] do not mutate passed object to fromObject --- src/shapes/image.class.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/shapes/image.class.js b/src/shapes/image.class.js index d0dbf537c36..089db6a07d3 100644 --- a/src/shapes/image.class.js +++ b/src/shapes/image.class.js @@ -630,7 +630,8 @@ * @param {Object} object Object to create an instance from * @param {Function} callback Callback to invoke when an image instance is created */ - fabric.Image.fromObject = function(object, callback) { + fabric.Image.fromObject = function(_object, callback) { + var object = fabric.util.object.clone(_object); fabric.util.loadImage(object.src, function(img, error) { if (error) { callback && callback(null, error); From 1ce19b9854a8124e56a3ef92c959027cd16044ac Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sat, 10 Feb 2018 00:25:42 +0100 Subject: [PATCH 2/3] add a test --- test/unit/image.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/unit/image.js b/test/unit/image.js index 679a4ca891a..c89402b6859 100644 --- a/test/unit/image.js +++ b/test/unit/image.js @@ -336,6 +336,40 @@ done(); }); }); + + QUnit.test('fromObject does not mutate data', function(assert) { + var done = assert.async(); + assert.ok(typeof fabric.Image.fromObject === 'function'); + + // should not throw error when no callback is given + var obj = fabric.util.object.extend(fabric.util.object.clone(REFERENCE_IMG_OBJECT), { + src: IMG_SRC + }); + var brightness = { + type: Brightness, + brightness: 0.1, + }; + var contrast = { + type: Contrast, + contrast: 0.1, + }; + obj.filters = [brightness] + obj.resizeFilter = contrast; + var copyOfFilters = obj.filters; + var copyOfBrighteness = brightness; + var copyOfContrast = contrast; + var copyOfObject = obj; + fabric.Image.fromObject(obj, function(instance){ + assert.ok(copyOfFilters === obj.filters, 'filters array did not mutate'); + assert.ok(copyOfBrighteness === copyOfFilters[0], 'filter is same object'); + assert.deepEqual(copyOfBrighteness, obj.filters[0], 'did not mutate filter'); + assert.deepEqual(copyOfFilters, obj.filters, 'did not mutate array'); + assert.deepEqual(copyOfContrast, obj.resizeFilter, 'did not mutate object'); + assert.deepEqual(copyOfObject, obj, 'did not change any value'); + assert.ok(copyOfContrast === obj.resizeFilter, 'resizefilter is same object'); + done(); + }); + }); QUnit.test('fromURL', function(assert) { var done = assert.async(); From 384e4a0cc5888d98ee737b27e30f8739801e7be0 Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sat, 10 Feb 2018 00:31:24 +0100 Subject: [PATCH 3/3] this is what you get for editing in github.com --- test/unit/image.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/test/unit/image.js b/test/unit/image.js index c89402b6859..c7e05c6e5ea 100644 --- a/test/unit/image.js +++ b/test/unit/image.js @@ -336,30 +336,29 @@ done(); }); }); - + QUnit.test('fromObject does not mutate data', function(assert) { var done = assert.async(); assert.ok(typeof fabric.Image.fromObject === 'function'); - // should not throw error when no callback is given var obj = fabric.util.object.extend(fabric.util.object.clone(REFERENCE_IMG_OBJECT), { src: IMG_SRC }); var brightness = { - type: Brightness, - brightness: 0.1, + type: 'Brightness', + brightness: 0.1 }; var contrast = { - type: Contrast, - contrast: 0.1, + type: 'Contrast', + contrast: 0.1 }; - obj.filters = [brightness] + obj.filters = [brightness]; obj.resizeFilter = contrast; var copyOfFilters = obj.filters; var copyOfBrighteness = brightness; var copyOfContrast = contrast; var copyOfObject = obj; - fabric.Image.fromObject(obj, function(instance){ + fabric.Image.fromObject(obj, function(){ assert.ok(copyOfFilters === obj.filters, 'filters array did not mutate'); assert.ok(copyOfBrighteness === copyOfFilters[0], 'filter is same object'); assert.deepEqual(copyOfBrighteness, obj.filters[0], 'did not mutate filter');