Fix for bugs 1318657 and 1298967

(bzr r13417)

3 files changed, 174 insertions, 37 deletions

diff --git a/src/document.cpp b/src/document.cpp
index dc7ed254c..f79a00178 100644
--- a/src/document.cpp
+++ b/src/document.cpp
@@ -1529,7 +1529,7 @@ void SPDocument::setModifiedSinceSave(bool modified) {
 
 
 /**
- * Paste SVG defs from the document retrieved from the clipboard into the active document.
+ * Paste SVG defs from the document retrieved from the clipboard or imported document into the active document.
  * @param clipdoc The document to paste.
  * @pre @c clipdoc != NULL and pasting into the active document is possible.
  */
@@ -1540,27 +1540,117 @@ void SPDocument::importDefs(SPDocument *source)
     Inkscape::XML::Node *target_defs = this->getDefs()->getRepr();
 
     prevent_id_clashes(source, this);
+    
+    int stagger=0;
 
+    /*  Note, "clipboard" throughout the comments means "the document that is either the clipboard
+        or an imported document", as importDefs is called in both contexts.
+        
+        The order of the records in the clipboard is unpredictable and there may be both
+        forward and backwards references to other records within it.  There may be definitions in
+        the clipboard that duplicate definitions in the present document OR that duplicate other
+        definitions in the clipboard.  (Inkscape will not have created these, but they may be read
+        in from other SVG sources.)
+         
+        There are 3 passes to clean this up:
+
+        In the first find and mark definitions in the clipboard that are duplicates of those in the
+        present document.  Change the ID to "RESERVED_FOR_INKSCAPE_DUPLICATE_DEF_XXXXXXXXX".
+        (Inkscape will not reuse an ID, and the XXXXXXXXX keeps it from automatically creating new ones.)
+        References in the clipboard to the old clipboard name are converted to the name used
+        in the current document. 
+
+        In the second find and mark definitions in the clipboard that are duplicates of earlier 
+        definitions in the clipbard.  Unfortunately this is O(n^2) and could be very slow for a large
+        SVG with thousands of definitions.  As before, references are adjusted to reflect the name
+        going forward.
+
+        In the final cycle copy over those records not marked with that ID.
+
+        If an SVG file uses the special ID it will cause problems!
+        
+        If this function is called because of the paste of a true clipboard the caller will have passed in a
+        COPY of the clipboard items.  That is good, because this routine modifies that document.  If the calling
+        behavior ever changes, so that the same document is passed in on multiple pastes, this routine will break
+        as in the following example:
+        1.  Paste clipboard containing B same as A into document containing A.  Result, B is dropped
+        and all references to it will point to A.
+        2.  Paste same clipboard into a new document.  It will not contain A, so there will be unsatisfied
+        references in that window.
+    */
+
+    std::string DuplicateDefString = "RESERVED_FOR_INKSCAPE_DUPLICATE_DEF";
+    
+    /* First pass: remove duplicates in clipboard of definitions in document */
     for (Inkscape::XML::Node *def = defs->firstChild() ; def ; def = def->next()) {
 
-        gboolean duplicate = false;
+        /* If this  clipboard has been pasted into one document, and is now being pasted into another,
+        or pasted again into the same, it will already have been processed.  If we detect that then 
+        skip the rest of this pass. */
+
+        Glib::ustring defid = def->attribute("id");
+        if( defid.find( DuplicateDefString ) != Glib::ustring::npos )break;
+
         SPObject *src = source->getObjectByRepr(def);
 
         // Prevent duplicates of solid swatches by checking if equivalent swatch already exists
         if (src && SP_IS_GRADIENT(src)) {
-            SPGradient *gr = SP_GRADIENT(src);
+            SPGradient *s_gr = SP_GRADIENT(src);
             for (SPObject *trg = this->getDefs()->firstChild() ; trg ; trg = trg->getNext()) {
-                if (trg && SP_IS_GRADIENT(trg) && src != trg) {
-                    if (gr->isEquivalent(SP_GRADIENT(trg)) &&
-                        gr->isAligned(SP_GRADIENT(trg))) {
-                        // Change object references to the existing equivalent gradient
-                        change_def_references(src, trg);
-                        duplicate = true;
-                        break;
+                if (trg && (src != trg) && SP_IS_GRADIENT(trg)) {
+                    SPGradient *t_gr = SP_GRADIENT(trg);
+                    if (t_gr && s_gr->isEquivalent(t_gr)) {
+                         // Change object references to the existing equivalent gradient
+                         Glib::ustring newid = trg->getId();
+                         if(newid != defid){  // id could be the same if it is a second paste into the same document
+                             change_def_references(src, trg);
+                         }
+                         gchar *longid = g_strdup_printf("%s_%9.9d", DuplicateDefString.c_str(), stagger++);
+                         def->setAttribute("id", longid );
+                         g_free(longid);
+                         // do NOT break here, there could be more than 1 duplicate!
                     }
                 }
             }
         }
+    }
+
+    /* Second pass: remove duplicates in clipboard of earlier definitions in clipboard */
+    for (Inkscape::XML::Node *def = defs->firstChild() ; def ; def = def->next()) {
+        Glib::ustring defid = def->attribute("id");
+        if( defid.find( DuplicateDefString ) != Glib::ustring::npos )continue; // this one already handled
+        SPObject *src = source->getObjectByRepr(def);
+        if (src && SP_IS_GRADIENT(src)) {
+            SPGradient *s_gr = SP_GRADIENT(src);
+            for (Inkscape::XML::Node *laterDef = def->next() ; laterDef ; laterDef = laterDef->next()) {
+                SPObject *trg = source->getObjectByRepr(laterDef);
+                if(trg && (src != trg) && SP_IS_GRADIENT(trg)) {
+                     Glib::ustring newid = trg->getId();
+                     if( newid.find( DuplicateDefString ) != Glib::ustring::npos )continue; // this one already handled
+                     SPGradient *t_gr = SP_GRADIENT(trg);
+                     if (t_gr && s_gr->isEquivalent(t_gr)) {
+                         // Change object references to the existing equivalent gradient
+                         // two id's in the clipboard should never be the same, so always change references
+                         change_def_references(trg, src);
+                         gchar *longid = g_strdup_printf("%s_%9.9d", DuplicateDefString.c_str(), stagger++);
+                         laterDef->setAttribute("id", longid );
+                         g_free(longid);
+                         // do NOT break here, there could be more than 1 duplicate!
+                     }
+                }
+            }
+        }
+    }
+
+    /* Final pass: copy over those parts which are not duplicates  */
+    for (Inkscape::XML::Node *def = defs->firstChild() ; def ; def = def->next()) {
+
+        /* Ignore duplicate defs marked in the first pass */
+        Glib::ustring defid = def->attribute("id");
+        if( defid.find( DuplicateDefString ) != Glib::ustring::npos )continue;
+
+        bool duplicate = false;
+        SPObject *src = source->getObjectByRepr(def);
 
         // Prevent duplication of symbols... could be more clever.
         // The tag "_inkscape_duplicate" is added to "id" by ClipboardManagerImpl::copySymbol(). 
@@ -1597,7 +1687,6 @@ void SPDocument::importDefs(SPDocument *source)
             Inkscape::GC::release(dup);
         }
     }
-
 }
 
 /*
diff --git a/src/id-clash.cpp b/src/id-clash.cpp
index 66357b75b..94cdfef59 100644
--- a/src/id-clash.cpp
+++ b/src/id-clash.cpp
@@ -216,8 +216,7 @@ change_clashing_ids(SPDocument *imported_doc, SPDocument *current_doc,
 
             if (cd_obj && SP_IS_GRADIENT(cd_obj)) {
                 SPGradient *cd_gr = SP_GRADIENT(cd_obj);
-                if ( cd_gr->isEquivalent(SP_GRADIENT(elem)) &&
-                    cd_gr->isAligned(SP_GRADIENT(elem))) {
+                if ( cd_gr->isEquivalent(SP_GRADIENT(elem))) {
                     fix_clashing_ids = false;
                  }
              }
@@ -326,8 +325,26 @@ change_def_references(SPObject *from_obj, SPObject *to_obj)
         std::list<IdReference>::const_iterator it;
         const std::list<IdReference>::const_iterator it_end = pos->second.end();
         for (it = pos->second.begin(); it != it_end; ++it) {
-            if (it->type == REF_STYLE) {
+            if (it->type == REF_HREF) {
+                gchar *new_uri = g_strdup_printf("#%s", to_obj->getId());
+                it->elem->getRepr()->setAttribute(it->attr, new_uri);
+                g_free(new_uri);
+            } else if (it->type == REF_STYLE) {
                 sp_style_set_property_url(it->elem, it->attr, to_obj, false);
+            } else if (it->type == REF_URL) {
+                gchar *url = g_strdup_printf("url(#%s)", to_obj->getId());
+                it->elem->getRepr()->setAttribute(it->attr, url);
+                g_free(url);
+            } else if (it->type == REF_CLIPBOARD) {
+                SPCSSAttr *style = sp_repr_css_attr(it->elem->getRepr(), "style");
+                gchar *url = g_strdup_printf("url(#%s)", to_obj->getId());
+                sp_repr_css_set_property(style, it->attr, url);
+                g_free(url);
+                Glib::ustring style_string;
+                sp_repr_css_write_string(style, style_string);
+                it->elem->getRepr()->setAttribute("style", style_string.c_str());
+            } else {
+                g_assert(0); // shouldn't happen
             }
         }
     }
diff --git a/src/sp-gradient.cpp b/src/sp-gradient.cpp
index 115cb754a..1479acd69 100644
--- a/src/sp-gradient.cpp
+++ b/src/sp-gradient.cpp
@@ -117,12 +117,17 @@ gboolean SPGradient::isEquivalent(SPGradient *that)
         if (this->getStopCount() != that->getStopCount()) { break; }
         if (this->hasStops() != that->hasStops()) { break; }
         if (!this->getVector() || !that->getVector()) { break; }
-        if ( (SP_IS_LINEARGRADIENT(this) && SP_IS_LINEARGRADIENT(that)) ||
-             (SP_IS_RADIALGRADIENT(this) && SP_IS_RADIALGRADIENT(that)) ||
-             (SP_IS_MESHGRADIENT(this) && SP_IS_MESHGRADIENT(that))) {
-             /* OK! */ 
+        if (this->isSwatch() != that->isSwatch()) {  break; }
+        if ( this->isSwatch() ){
+           // drop down to check stops.
         }
-        else { break; }
+        else if (
+            (SP_IS_LINEARGRADIENT(this) && SP_IS_LINEARGRADIENT(that)) ||
+            (SP_IS_RADIALGRADIENT(this) && SP_IS_RADIALGRADIENT(that)) ||
+            (SP_IS_MESHGRADIENT(this) && SP_IS_MESHGRADIENT(that))) {
+            if(!this->isAligned(that))break;
+        }
+        else { break; }  // this should never happen, some unhandled type of gradient
 
         SPStop *as = this->getVector()->getFirstStop();
         SPStop *bs = that->getVector()->getFirstStop();
@@ -156,6 +161,18 @@ gboolean SPGradient::isAligned(SPGradient *that)
 {
     bool status = FALSE;
     
+   /*  Some gradients have coordinates/other values specified, some don't.  
+           yes/yes check the coordinates/other values
+           no/no   aligned (because both have all default values)
+           yes/no  not aligned
+           no/yes  not aligned
+       It is NOT safe to just compare the computed values because if that field has
+       not been set the computed value could be full of garbage.
+       
+       In theory the yes/no and no/yes cases could be aligned if the specified value
+       matches the default value.
+    */
+
     while(1){ // not really a loop, used to avoid deep nesting or multiple exit points from function
         if(this->gradientTransform_set != that->gradientTransform_set) { break; }
         if(this->gradientTransform_set && 
@@ -164,31 +181,45 @@ gboolean SPGradient::isAligned(SPGradient *that)
             SPLinearGradient *sg=SP_LINEARGRADIENT(this);
             SPLinearGradient *tg=SP_LINEARGRADIENT(that);
  
-            if( !sg->x1._set     ||    !tg->x1._set  || // assume that if these are set so will be all the others
-                (sg->x1.computed != tg->x1.computed) ||
-                (sg->y1.computed != tg->y1.computed) ||
-                (sg->x2.computed != tg->x2.computed) ||
-                (sg->y2.computed != tg->y2.computed)
-            ) { break; }
+            if( sg->x1._set != tg->x1._set) { break; }
+            if( sg->y1._set != tg->y1._set) { break; }
+            if( sg->x2._set != tg->x2._set) { break; }
+            if( sg->y2._set != tg->y2._set) { break; }
+            if( sg->x1._set && sg->y1._set && sg->x2._set && sg->y2._set) {
+                if( (sg->x1.computed != tg->x1.computed) ||
+                    (sg->y1.computed != tg->y1.computed) ||
+                    (sg->x2.computed != tg->x2.computed) ||
+                    (sg->y2.computed != tg->y2.computed) )  { break; }
+            } else if( sg->x1._set || sg->y1._set || sg->x2._set || sg->y2._set) { break; } // some mix of set and not set
+            // none set? assume aligned and fall through
         } else if (SP_IS_RADIALGRADIENT(this) && SP_IS_LINEARGRADIENT(that)) {
             SPRadialGradient *sg=SP_RADIALGRADIENT(this);
             SPRadialGradient *tg=SP_RADIALGRADIENT(that);
-            if( !sg->cx._set     ||    !tg->cx._set  || // assume that if these are set so will be all the others
-                (sg->cx.computed != tg->cx.computed) ||
-                (sg->cy.computed != tg->cy.computed) ||
-                (sg->r.computed  != tg->r.computed ) ||
-                (sg->fx.computed != tg->fx.computed) ||
-                (sg->fy.computed != tg->fy.computed)
-            ) { break; }
+
+            if( sg->cx._set != tg->cx._set) { break; }
+            if( sg->cy._set != tg->cy._set) { break; }
+            if( sg->r._set  != tg->r._set)  { break; }
+            if( sg->fx._set != tg->fx._set) { break; }
+            if( sg->fy._set != tg->fy._set) { break; }
+            if( sg->cx._set && sg->cy._set && sg->fx._set && sg->fy._set && sg->r._set) {
+                if( (sg->cx.computed != tg->cx.computed) ||
+                    (sg->cy.computed != tg->cy.computed) ||
+                    (sg->r.computed  != tg->r.computed ) ||
+                    (sg->fx.computed != tg->fx.computed) ||
+                    (sg->fy.computed != tg->fy.computed)  ) { break; }
+            } else if(  sg->cx._set || sg->cy._set || sg->fx._set || sg->fy._set || sg->r._set ) { break; } // some mix of set and not set
+            // none set? assume aligned and fall through
         } else if (SP_IS_MESHGRADIENT(this) && SP_IS_MESHGRADIENT(that)) {
             SPMeshGradient *sg=SP_MESHGRADIENT(this);
             SPMeshGradient *tg=SP_MESHGRADIENT(that);
  
-            if( !sg->x._set     ||    !tg->x._set  ||
-                !sg->y._set     ||    !tg->y._set  ||
-                (sg->x.computed != tg->x.computed) ||
-                (sg->y.computed != tg->y.computed)
-            ) { break; }
+            if( sg->x._set  !=  !tg->x._set) { break; }
+            if( sg->y._set  !=  !tg->y._set) { break; }
+            if( sg->x._set  &&  sg->y._set) { 
+                if( (sg->x.computed != tg->x.computed) ||
+                    (sg->y.computed != tg->y.computed) ) { break; }
+            } else if( sg->x._set || sg->y._set) { break; } // some mix of set and not set
+            // none set? assume aligned and fall through
          } else {
             break;
         }