From 6994b0419b3fd6c64e91e06591136f00a76aca26 Mon Sep 17 00:00:00 2001 From: Andrea Bogazzi Date: Sat, 11 Aug 2018 17:30:55 +0200 Subject: [PATCH] Make getObjects return a copy of the object array (#5162) * ok * ok * ok --- src/mixins/canvas_grouping.mixin.js | 2 +- src/mixins/collection.mixin.js | 17 +++++------ src/shapes/active_selection.class.js | 43 +++------------------------- src/shapes/group.class.js | 22 +++++++------- src/static_canvas.class.js | 8 +++--- test/unit/collection.js | 2 +- 6 files changed, 29 insertions(+), 65 deletions(-) diff --git a/src/mixins/canvas_grouping.mixin.js b/src/mixins/canvas_grouping.mixin.js index 20f268a4372..a44d2552b40 100644 --- a/src/mixins/canvas_grouping.mixin.js +++ b/src/mixins/canvas_grouping.mixin.js @@ -80,7 +80,7 @@ * @param {Object} target */ _createGroup: function(target) { - var objects = this.getObjects(), + var objects = this._objects, isActiveLower = objects.indexOf(this._activeObject) < objects.indexOf(target), groupObjects = isActiveLower ? [this._activeObject, target] diff --git a/src/mixins/collection.mixin.js b/src/mixins/collection.mixin.js index df7752c87c6..86dd68c6ea5 100644 --- a/src/mixins/collection.mixin.js +++ b/src/mixins/collection.mixin.js @@ -41,7 +41,7 @@ fabric.Collection = { * @chainable */ insertAt: function (object, index, nonSplicing) { - var objects = this.getObjects(); + var objects = this._objects; if (nonSplicing) { objects[index] = object; } @@ -60,7 +60,7 @@ fabric.Collection = { * @chainable */ remove: function() { - var objects = this.getObjects(), + var objects = this._objects, index, somethingRemoved = false; for (var i = 0, length = arguments.length; i < length; i++) { @@ -101,12 +101,13 @@ fabric.Collection = { /** * Returns an array of children objects of this instance * Type parameter introduced in 1.3.10 + * since 2.3.5 this method return always a COPY of the array; * @param {String} [type] When specified, only objects of this type are returned * @return {Array} */ getObjects: function(type) { if (typeof type === 'undefined') { - return this._objects; + return this._objects.concat(); } return this._objects.filter(function(o) { return o.type === type; @@ -119,7 +120,7 @@ fabric.Collection = { * @return {Self} thisArg */ item: function (index) { - return this.getObjects()[index]; + return this._objects[index]; }, /** @@ -127,7 +128,7 @@ fabric.Collection = { * @return {Boolean} true if collection is empty */ isEmpty: function () { - return this.getObjects().length === 0; + return this._objects.length === 0; }, /** @@ -135,7 +136,7 @@ fabric.Collection = { * @return {Number} Collection size */ size: function() { - return this.getObjects().length; + return this._objects.length; }, /** @@ -144,7 +145,7 @@ fabric.Collection = { * @return {Boolean} `true` if collection contains an object */ contains: function(object) { - return this.getObjects().indexOf(object) > -1; + return this._objects.indexOf(object) > -1; }, /** @@ -152,7 +153,7 @@ fabric.Collection = { * @return {Number} complexity */ complexity: function () { - return this.getObjects().reduce(function (memo, current) { + return this._objects.reduce(function (memo, current) { memo += current.complexity ? current.complexity() : 0; return memo; }, 0); diff --git a/src/shapes/active_selection.class.js b/src/shapes/active_selection.class.js index c3666c25429..7a47a63b0a3 100644 --- a/src/shapes/active_selection.class.js +++ b/src/shapes/active_selection.class.js @@ -57,16 +57,15 @@ * @return {fabric.Group} */ toGroup: function() { - var objects = this._objects; + var objects = this._objects.concat(); this._objects = []; - var options = this.toObject(); + var options = fabric.Object.prototype.toObject.call(this); var newGroup = new fabric.Group([]); - delete options.objects; + delete options.type; newGroup.set(options); - newGroup.type = 'group'; objects.forEach(function(object) { - object.group = newGroup; object.canvas.remove(object); + object.group = newGroup; }); newGroup._objects = objects; if (!this.canvas) { @@ -97,24 +96,6 @@ return '#'; }, - /** - * @private - */ - _set: function(key, value) { - var i = this._objects.length; - if (key === 'canvas') { - while (i--) { - this._objects[i].set(key, value); - } - } - if (this.useSetOnGroup) { - while (i--) { - this._objects[i].setOnGroup(key, value); - } - } - fabric.Object.prototype._set.call(this, key, value); - }, - /** * Decide if the object should cache or not. Create its own cache level * objectCaching is a global flag, wins over everything @@ -127,22 +108,6 @@ return false; }, - /** - * Check if this object or a child object will cast a shadow - * @return {Boolean} - */ - willDrawShadow: function() { - if (this.shadow) { - return this.callSuper('willDrawShadow'); - } - for (var i = 0, len = this._objects.length; i < len; i++) { - if (this._objects[i].willDrawShadow()) { - return true; - } - } - return false; - }, - /** * Check if this group or its parent group are caching, recursively up * @return {Boolean} diff --git a/src/shapes/group.class.js b/src/shapes/group.class.js index 36b38a5db02..abd562410f0 100644 --- a/src/shapes/group.class.js +++ b/src/shapes/group.class.js @@ -3,7 +3,6 @@ 'use strict'; var fabric = global.fabric || (global.fabric = { }), - extend = fabric.util.object.extend, min = fabric.util.array.min, max = fabric.util.array.max; @@ -208,12 +207,11 @@ } } if (key === 'canvas') { - i = this._objects.length; while (i--) { this._objects[i]._set(key, value); } } - this.callSuper('_set', key, value); + fabric.Object.prototype._set.call(this, key, value); }, /** @@ -222,16 +220,16 @@ * @return {Object} object representation of an instance */ toObject: function(propertiesToInclude) { - var objsToObject = this.getObjects().map(function(obj) { + var objsToObject = this._objects.map(function(obj) { var originalDefaults = obj.includeDefaultValues; obj.includeDefaultValues = obj.group.includeDefaultValues; var _obj = obj.toObject(propertiesToInclude); obj.includeDefaultValues = originalDefaults; return _obj; }); - return extend(this.callSuper('toObject', propertiesToInclude), { - objects: objsToObject - }); + var obj = fabric.Object.prototype.toObject.call(this, propertiesToInclude); + obj.objects = objsToObject; + return obj; }, /** @@ -245,7 +243,7 @@ objsToObject = sourcePath; } else { - objsToObject = this.getObjects().map(function(obj) { + objsToObject = this._objects.map(function(obj) { var originalDefaults = obj.includeDefaultValues; obj.includeDefaultValues = obj.group.includeDefaultValues; var _obj = obj.toDatalessObject(propertiesToInclude); @@ -253,9 +251,9 @@ return _obj; }); } - return extend(this.callSuper('toDatalessObject', propertiesToInclude), { - objects: objsToObject - }); + var obj = fabric.Object.prototype.toDatalessObject.call(this, propertiesToInclude); + obj.objects = objsToObject; + return obj; }, /** @@ -296,7 +294,7 @@ */ willDrawShadow: function() { if (this.shadow) { - return this.callSuper('willDrawShadow'); + return fabric.Object.prototype.willDrawShadow.call(this); } for (var i = 0, len = this._objects.length; i < len; i++) { if (this._objects[i].willDrawShadow()) { diff --git a/src/static_canvas.class.js b/src/static_canvas.class.js index dacc47bd634..db85ab39529 100644 --- a/src/static_canvas.class.js +++ b/src/static_canvas.class.js @@ -1126,7 +1126,7 @@ * @private */ _toObjects: function(methodName, propertiesToInclude) { - return this.getObjects().filter(function(object) { + return this._objects.filter(function(object) { return !object.excludeFromExport; }).map(function(instance) { return this._toObject(instance, methodName, propertiesToInclude); @@ -1328,7 +1328,7 @@ createSVGFontFacesMarkup: function() { var markup = '', fontList = { }, obj, fontFamily, style, row, rowIndex, _char, charIndex, i, len, - fontPaths = fabric.fontPaths, objects = this.getObjects(); + fontPaths = fabric.fontPaths, objects = this._objects; for (i = 0, len = objects.length; i < len; i++) { obj = objects[i]; @@ -1379,7 +1379,7 @@ * @private */ _setSVGObjects: function(markup, reviver) { - var instance, i, len, objects = this.getObjects(); + var instance, i, len, objects = this._objects; for (i = 0, len = objects.length; i < len; i++) { instance = objects[i]; if (instance.excludeFromExport) { @@ -1694,7 +1694,7 @@ */ toString: function () { return '#'; + '{ objects: ' + this._objects.length + ' }>'; } }); diff --git a/test/unit/collection.js b/test/unit/collection.js index d1d37911e74..bc1063d34e1 100644 --- a/test/unit/collection.js +++ b/test/unit/collection.js @@ -129,7 +129,7 @@ collection.add(obj2, obj); assert.ok(typeof collection.getObjects === 'function', 'has getObjects method'); var returned = collection.getObjects(); - assert.equal(returned, collection._objects, 'return the _objects array directly'); + assert.notEqual(returned, collection._objects, 'does not return a reference to _objects'); returned = collection.getObjects('a'); assert.notEqual(returned, collection._objects, 'return a new array'); assert.equal(returned.indexOf(obj2), -1, 'object of type B is not included');