Fix regression in loop prevention

Fixed bugs:
  - https://launchpad.net/bugs/1636533

(bzr r15193)

3 files changed, 16 insertions, 27 deletions

diff --git a/src/sp-object.cpp b/src/sp-object.cpp
index 21d8bcd93..8d4c4f0d1 100644
--- a/src/sp-object.cpp
+++ b/src/sp-object.cpp
@@ -119,7 +119,7 @@ static gchar *sp_object_get_unique_id(SPObject    *object,
  * Constructor, sets all attributes to default values.
  */
 SPObject::SPObject()
-    : cloned(0), uflags(0), mflags(0), hrefcount(0), _total_hrefcount(0),
+    : cloned(0), clone_original(NULL), uflags(0), mflags(0), hrefcount(0), _total_hrefcount(0),
       document(NULL), parent(NULL), id(NULL), repr(NULL), refCount(1), hrefList(std::list<SPObject*>()),
       _successor(NULL), _collection_policy(SPObject::COLLECT_WITH_PARENT),
       _label(NULL), _default_label(NULL)
@@ -663,6 +663,8 @@ void SPObject::build(SPDocument *document, Inkscape::XML::Node *repr) {
     object->readAttr("xml:space");
     object->readAttr("inkscape:label");
     object->readAttr("inkscape:collect");
+    if(object->cloned)
+        object->clone_original = document->getObjectById(repr->attribute("id"));
 
     for (Inkscape::XML::Node *rchild = repr->firstChild() ; rchild != NULL; rchild = rchild->next()) {
         const std::string typeString = NodeTraits::get_type_string(*rchild);
diff --git a/src/sp-object.h b/src/sp-object.h
index ac3d0c851..355f837b5 100644
--- a/src/sp-object.h
+++ b/src/sp-object.h
@@ -201,6 +201,7 @@ public:
     virtual ~SPObject();
 
     unsigned int cloned : 1;
+    SPObject *clone_original;
     unsigned int uflags : 8;
     unsigned int mflags : 8;
     SPIXmlSpace xml_space;
diff --git a/src/uri-references.cpp b/src/uri-references.cpp
index 07f2d168b..170c98beb 100644
--- a/src/uri-references.cpp
+++ b/src/uri-references.cpp
@@ -3,6 +3,7 @@
  *
  * Authors:
  *   Lauris Kaplinski <lauris@kaplinski.com>
+ *   Marc Jeanmougin
  *
  * Copyright (C) 2001-2002 Lauris Kaplinski
  * Copyright (C) 2001 Ximian, Inc.
@@ -48,13 +49,10 @@ URIReference::~URIReference() { detach(); }
  * The main ideas here are:
  * (1) "If we are inside a clone, then we can accept if and only if our "original thing" can accept the reference"
  * (this caused problems when there are clones because a change in ids triggers signals for the object hrefing this id,
- *but also its cloned reprs
- * (descendants of <use> referencing an ancestor of the href'ing object)). The way it is done here is *atrocious*, but i
- *could not find a better way.
- * FIXME: find a better and safer way to find the "original object" of anyone with the flag ->cloned
+ * but also its cloned reprs(descendants of <use> referencing an ancestor of the href'ing object)). 
  *
  * (2) Once we have an (potential owner) object, it can accept a href to obj, iff the graph of objects where directed
- *edges are
+ * edges are
  * either parent->child relations , *** or href'ing to href'ed *** relations, stays acyclic.
  * We can go either from owner and up in the tree, or from obj and down, in either case this will be in the worst case
  *linear in the number of objects.
@@ -70,29 +68,17 @@ bool URIReference::_acceptObject(SPObject *obj) const
     SPObject *owner = getOwner();
     if (!owner)
         return true;
+    
     while (owner->cloned) {
-        std::vector<int> positions;
-        while (owner->cloned) {
-            int position = 0;
-            for (auto &child: owner->parent->children) {
-                if(&child == owner) {
-                    break;
-                }
-                position++;
-            }
-            positions.push_back(position);
+        if(!owner->clone_original)//happens when the clone is existing and linking to something, even before the original objects exists.
+                                  //for instance, it can happen when you paste a filtered object in a already cloned group: The construction of the 
+                                  //clone representation of the filtered object will finish before the original object, so the cloned repr will
+                                  //have to _accept the filter even though the original does not exist yet. In that case, we'll accept iff the parent of the 
+                                  //original can accept it: loops caused by other relations than parent-child would be prevented when created on their base object.
+                                  //Fixes bug 1636533.
             owner = owner->parent;
-        }
-        if (dynamic_cast<SPUse *>(owner))
-            owner = ((SPUse *)owner)->get_original();
-        else if (dynamic_cast<SPTagUse *>(owner))
-            owner = ((SPTagUse *)owner)->get_original();
-        else {
-            g_warning("cloned object with no known type\n");
-            return false;
-        }
-        for (int i = (int) (positions.size() - 2); i >= 0; i--)
-            owner = owner->childList(false)[positions[i]];
+        else
+            owner = owner->clone_original;
     }
     // once we have the "original" object (hopefully) we look at who is referencing it
     if (obj == owner)