From 7756e90ff9a33a86eae381a1217749bd7d4ec908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Sat, 19 Jan 2013 22:27:29 -0800 Subject: [PATCH] Make sure cloned items do not receive the same name when placed inside the same parent, use numbered versions instead. --- src/item/Item.js | 17 +++++++++++------ test/lib/helpers.js | 37 ++++++++++++++++++++++--------------- test/tests/Item_Cloning.js | 4 ++-- test/tests/SvgImport.js | 2 +- 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/item/Item.js b/src/item/Item.js index b8da4f96..f0e4e4b9 100644 --- a/src/item/Item.js +++ b/src/item/Item.js @@ -252,7 +252,7 @@ var Item = this.Item = Base.extend(Callback, /** @lends Item# */{ return this._name; }, - setName: function(name) { + setName: function(name, unique) { // Note: Don't check if the name has changed and bail out if it has not, // because setName is used internally also to update internal structures // when an item is moved from one parent to another. @@ -261,13 +261,18 @@ var Item = this.Item = Base.extend(Callback, /** @lends Item# */{ // parent's children object: if (this._name) this._removeFromNamed(); - this._name = name || undefined; if (name && this._parent) { var children = this._parent._children, - namedChildren = this._parent._namedChildren; + namedChildren = this._parent._namedChildren, + orig = name, + i = 1; + // If unique is true, make sure we're not overriding other names + while (unique && children[name]) + name = orig + ' ' + (i++); (namedChildren[name] = namedChildren[name] || []).push(this); children[name] = this; } + this._name = name || undefined; this._changed(/*#=*/ ChangeFlag.ATTRIBUTE); }, @@ -1022,10 +1027,10 @@ var Item = this.Item = Base.extend(Callback, /** @lends Item# */{ // Copy over the selection state, use setSelected so the item // is also added to Project#selectedItems if it is selected. copy.setSelected(this._selected); - // Only set name once the copy is moved, to avoid setting and unsettting - // name related structures. + // Clone the name too, but make sure we're not overriding the original + // in the same parent, by passing true for the unique parameter. if (this._name) - copy.setName(this._name); + copy.setName(this._name, true); return copy; }, diff --git a/test/lib/helpers.js b/test/lib/helpers.js index bfbad71f..fc244478 100644 --- a/test/lib/helpers.js +++ b/test/lib/helpers.js @@ -92,7 +92,7 @@ function compareGradientColors(gradientColor, gradientColor2, checkIdentity) { Base.each(['origin', 'destination', 'hilite'], function(key) { if (checkIdentity) { equals(function() { - return gradientColor[key] != gradientColor2[key]; + return gradientColor[key] !== gradientColor2[key]; }, true, 'Strict compare GradientColor#' + key); } equals(gradientColor[key].toString(), gradientColor2[key].toString(), @@ -106,7 +106,7 @@ function compareGradientColors(gradientColor, gradientColor2, checkIdentity) { function comparePathStyles(style, style2, checkIdentity) { if (checkIdentity) { equals(function() { - return style != style2; + return style !== style2; }, true); } Base.each(['fillColor', 'strokeColor'], function(key) { @@ -120,7 +120,7 @@ function comparePathStyles(style, style2, checkIdentity) { if (style[key] instanceof GradientColor) { if (checkIdentity) { equals(function() { - return style[key].gradient == style2[key].gradient; + return style[key].gradient === style2[key].gradient; }, true, 'The ' + key + '.gradient should point to the same object:'); } compareGradientColors(style[key], style2[key], checkIdentity); @@ -149,7 +149,7 @@ function comparePathStyles(style, style2, checkIdentity) { function compareObjects(name, keys, obj, obj2, checkIdentity) { if (checkIdentity) { equals(function() { - return obj != obj2; + return obj !== obj2; }, true); } Base.each(keys, function(key) { @@ -203,14 +203,14 @@ function compareSegmentLists(segmentList, segmentList2, checkIdentity) { } } -function compareItems(item, item2, checkIdentity) { +function compareItems(item, item2, cloned, checkIdentity) { if (checkIdentity) { equals(function() { - return item != item2; + return item !== item2; }, true); equals(function() { - return item.id != item2.id; + return item.id !== item2.id; }, true); } @@ -219,14 +219,21 @@ function compareItems(item, item2, checkIdentity) { }, true); var itemProperties = ['opacity', 'locked', 'visible', 'blendMode', 'name', - 'selected', 'clipMask']; + 'selected', 'clipMask']; Base.each(itemProperties, function(key) { - equals(item[key], item2[key], 'compare Item#' + key); + var value = item[key]; + // When item was cloned and had a name, the name will be versioned + equals( + key == 'name' && cloned && value + ? value + ' 1' + : value, + item2[key], + 'compare Item#' + key); }); if (checkIdentity) { equals(function() { - return item.bounds != item2.bounds; + return item.bounds !== item2.bounds; }, true); } @@ -235,7 +242,7 @@ function compareItems(item, item2, checkIdentity) { if (checkIdentity) { equals(function() { - return item.position != item2.position; + return item.position !== item2.position; }, true); } @@ -245,7 +252,7 @@ function compareItems(item, item2, checkIdentity) { if (item.matrix) { if (checkIdentity) { equals(function() { - return item.matrix != item2.matrix; + return item.matrix !== item2.matrix; }, true); } equals(item.matrix.toString(), item2.matrix.toString(), @@ -289,7 +296,7 @@ function compareItems(item, item2, checkIdentity) { if (item._canvas) { if (checkIdentity) { equals(function() { - return item._canvas != item2._canvas; + return item._canvas !== item2._canvas; }, true); } } @@ -313,7 +320,7 @@ function compareItems(item, item2, checkIdentity) { if (item instanceof PointText) { if (checkIdentity) { equals(function() { - return item.point != item2.point; + return item.point !== item2.point; }, true); } equals(item.point.toString(), item2.point.toString(), @@ -331,7 +338,7 @@ function compareItems(item, item2, checkIdentity) { return item.children.length == item2.children.length; }, true); for (var i = 0, l = item.children.length; i < l; i++) { - compareItems(item.children[i], item2.children[i], checkIdentity); + compareItems(item.children[i], item2.children[i], cloned, checkIdentity); } } } diff --git a/test/tests/Item_Cloning.js b/test/tests/Item_Cloning.js index 606419c2..3d489017 100644 --- a/test/tests/Item_Cloning.js +++ b/test/tests/Item_Cloning.js @@ -27,7 +27,7 @@ function cloneAndCompare(item) { return copy.parent.children[copy.name] == copy; }, true); } - compareItems(copy, item, true); + compareItems(item, copy, true, true); // Remove the cloned item to restore the document: copy.remove(); } @@ -140,7 +140,7 @@ test('Symbol#clone()', function() { path.selected = true; var symbol = new Symbol(path); var copy = symbol.clone(); - compareItems(copy.definition, symbol.definition); + compareItems(symbol.definition, copy.definition); equals(function() { return symbol.project == copy.project; }, true); diff --git a/test/tests/SvgImport.js b/test/tests/SvgImport.js index a8329d7e..eba5a1c3 100644 --- a/test/tests/SvgImport.js +++ b/test/tests/SvgImport.js @@ -21,7 +21,7 @@ module('SvgImport'); test('Import complex CompoundPath and clone', function() { var svg = createSvg(';'); var item = paper.project.importSvg(svg.getElementById('path')); - compareItems(item, item.clone()); + compareItems(item, item.clone(), true, true); }); test('make an svg line', function() {