better way to deal with undo+id collisions

(bzr r2680)

5 files changed, 51 insertions, 23 deletions

diff --git a/src/document-private.h b/src/document-private.h
index c89948290..1c03a8025 100644
--- a/src/document-private.h
+++ b/src/document-private.h
@@ -70,6 +70,8 @@ struct SPDocumentPrivate {
 
 	// XXX only for testing!
 	Inkscape::ConsoleOutputUndoObserver console_output_undo_observer;
+
+	bool seeking;
 };
 
 #endif
diff --git a/src/document-undo.cpp b/src/document-undo.cpp
index c7da41080..0462ff374 100644
--- a/src/document-undo.cpp
+++ b/src/document-undo.cpp
@@ -7,6 +7,7 @@
  *   Lauris Kaplinski <lauris@kaplinski.com>
  *   MenTaLguY <mental@rydia.net>
  *
+ * Copyright (C) 2007  MenTaLguY <mental@rydia.net>
  * Copyright (C) 1999-2003 authors
  * Copyright (C) 2001-2002 Ximian, Inc.
  *
@@ -222,6 +223,7 @@ sp_document_undo (SPDocument *doc)
 	g_assert (doc->priv->sensitive);
 
 	doc->priv->sensitive = FALSE;
+        doc->priv->seeking = true;
 
 	doc->actionkey = NULL;
 
@@ -244,6 +246,7 @@ sp_document_undo (SPDocument *doc)
 	sp_repr_begin_transaction (doc->rdoc);
 
 	doc->priv->sensitive = TRUE;
+        doc->priv->seeking = false;
 
 	if (ret)
 		inkscape_external_change();
@@ -266,6 +269,7 @@ sp_document_redo (SPDocument *doc)
 	g_assert (doc->priv->sensitive);
 
 	doc->priv->sensitive = FALSE;
+        doc->priv->seeking = true;
 
 	doc->actionkey = NULL;
 
@@ -288,6 +292,7 @@ sp_document_redo (SPDocument *doc)
 	sp_repr_begin_transaction (doc->rdoc);
 
 	doc->priv->sensitive = TRUE;
+        doc->priv->seeking = false;
 
 	if (ret)
 		inkscape_external_change();
diff --git a/src/document.cpp b/src/document.cpp
index d77d61d09..c6dc66a51 100644
--- a/src/document.cpp
+++ b/src/document.cpp
@@ -106,6 +106,7 @@ SPDocument::SPDocument() {
     p->history_size = 0;
     p->undo = NULL;
     p->redo = NULL;
+    p->seeking = false;
 
     priv = p;
 
@@ -1155,6 +1156,10 @@ vacuum_document(SPDocument *document)
     return start - newend;
 }
 
+bool SPDocument::isSeeking() const {
+    return priv->seeking;
+}
+
 
 /*
   Local Variables:
diff --git a/src/document.h b/src/document.h
index 12848aa68..6e4bd8d6c 100644
--- a/src/document.h
+++ b/src/document.h
@@ -117,6 +117,9 @@ struct SPDocument : public Inkscape::GC::Managed<>,
 
         bool _updateDocument();
 
+	/// Are we currently in a transition between two "known good" states of the document?
+	bool isSeeking() const;
+
 private:
 	SPDocument(SPDocument const &); // no copy
 	void operator=(SPDocument const &); // no assign
diff --git a/src/sp-object.cpp b/src/sp-object.cpp
index 03dc3ec28..5dbc4101e 100644
--- a/src/sp-object.cpp
+++ b/src/sp-object.cpp
@@ -821,20 +821,26 @@ sp_object_invoke_build(SPObject *object, SPDocument *document, Inkscape::XML::No
         object->document->bindObjectToRepr(object->repr, object);
 
         if (Inkscape::XML::id_permitted(object->repr)) {
-            /* If we are not cloned, force unique id */
+            /* If we are not cloned, and not seeking, force unique id */
             gchar const *id = object->repr->attribute("id");
-            gchar *realid = sp_object_get_unique_id(object, id);
-            g_assert(realid != NULL);
-
-            object->document->bindObjectToId(realid, object);
-            object->id = realid;
-
-            /* Redefine ID, if required */
-            if ((id == NULL) || (strcmp(id, realid) != 0)) {
-                bool saved = sp_document_get_undo_sensitive(document);
-                sp_document_set_undo_sensitive(document, false);
-                object->repr->setAttribute("id", realid);
-                sp_document_set_undo_sensitive(document, saved);
+            if (!document->isSeeking()) {
+                gchar *realid = sp_object_get_unique_id(object, id);
+                g_assert(realid != NULL);
+
+                object->document->bindObjectToId(realid, object);
+                object->id = realid;
+
+                /* Redefine ID, if required */
+                if ((id == NULL) || (strcmp(id, realid) != 0)) {
+                    object->repr->setAttribute("id", realid);
+                }
+            } else if (id) {
+                // bind if id, but no conflict -- otherwise, we can expect
+                // a subsequent setting of the id attribute
+                if (!object->document->getObjectById(id)) {
+                    object->document->bindObjectToId(id, object);
+                    object->id = g_strdup(id);
+                }
             }
         }
     } else {
@@ -945,16 +951,23 @@ sp_object_private_set(SPObject *object, unsigned int key, gchar const *value)
                 SPDocument *document=object->document;
                 SPObject *conflict=NULL;
 
-                if (value) {
-                    conflict = document->getObjectById((char const *)value);
+                gchar const *new_id = value;
+
+                if (new_id) {
+                    conflict = document->getObjectById((char const *)new_id);
                 }
+
                 if ( conflict && conflict != object ) {
-                    sp_object_ref(conflict, NULL);
-                    // give the conflicting object a new ID
-                    gchar *new_conflict_id = sp_object_get_unique_id(conflict, NULL);
-                    SP_OBJECT_REPR(conflict)->setAttribute("id", new_conflict_id);
-                    g_free(new_conflict_id);
-                    sp_object_unref(conflict, NULL);
+                    if (!document->isSeeking()) {
+                        sp_object_ref(conflict, NULL);
+                        // give the conflicting object a new ID
+                        gchar *new_conflict_id = sp_object_get_unique_id(conflict, NULL);
+                        SP_OBJECT_REPR(conflict)->setAttribute("id", new_conflict_id);
+                        g_free(new_conflict_id);
+                        sp_object_unref(conflict, NULL);
+                    } else {
+                        new_id = NULL;
+                    }
                 }
 
                 if (object->id) {
@@ -962,8 +975,8 @@ sp_object_private_set(SPObject *object, unsigned int key, gchar const *value)
                     g_free(object->id);
                 }
 
-                if (value) {
-                    object->id = g_strdup((char const*)value);
+                if (new_id) {
+                    object->id = g_strdup((char const*)new_id);
                     document->bindObjectToId(object->id, object);
                 } else {
                     object->id = NULL;