Keep generic attributes with "Simplify" et. al

Keep generic attributes and child nodes when doing path conversion like
"Simplify", "Object To Path", "Combine", Inset/Outset or Boolean operations.

Fixes https://bugs.launchpad.net/inkscape/+bug/373140

3 files changed, 159 insertions, 189 deletions

diff --git a/src/path-chemistry.cpp b/src/path-chemistry.cpp
index 8efd27be8..81bc531c1 100644
--- a/src/path-chemistry.cpp
+++ b/src/path-chemistry.cpp
@@ -95,9 +95,7 @@ ObjectSet::combine(bool skip_undo)
 
     // remember the position, id, transform and style of the topmost path, they will be assigned to the combined one
     gint position = 0;
-    char const *id = nullptr;
     char const *transform = nullptr;
-    char const *style = nullptr;
     char const *path_effect = nullptr;
 
     SPCurve* curve = nullptr;
@@ -126,10 +124,8 @@ ObjectSet::combine(bool skip_undo)
             first = item;
             parent = first->getRepr()->parent();
             position = first->getRepr()->position();
-            id = first->getRepr()->attribute("id");
             transform = first->getRepr()->attribute("transform");
             // FIXME: merge styles of combined objects instead of using the first one's style
-            style = first->getRepr()->attribute("style");
             path_effect = first->getRepr()->attribute("inkscape:path-effect");
             //c->transform(item->transform);
             curve = c;
@@ -149,18 +145,18 @@ ObjectSet::combine(bool skip_undo)
 
 
     if (did) {
-        first->deleteObject(false);
-        // delete the topmost.
-
         Inkscape::XML::Document *xml_doc = doc->getReprDoc();
         Inkscape::XML::Node *repr = xml_doc->createElement("svg:path");
 
+        Inkscape::copy_object_properties(repr, first->getRepr());
+
+        // delete the topmost.
+        first->deleteObject(false);
+
         // restore id, transform, path effect, and style
-        repr->setAttribute("id", id);
         if (transform) {
             repr->setAttribute("transform", transform);
         }
-        repr->setAttribute("style", style);
 
         repr->setAttribute("inkscape:path-effect", path_effect);
 
@@ -447,14 +443,6 @@ sp_item_list_to_curves(const std::vector<SPItem*> &items, std::vector<SPItem*>&
         Inkscape::XML::Node *parent = item->getRepr()->parent();
         // remember class
         char const *class_attr = item->getRepr()->attribute("class");
-        // remember title
-        gchar *title = item->title();
-        // remember description
-        gchar *desc = item->desc();
-        // remember highlight color
-        guint32 highlight_color = 0;
-        if (item->isHighlightSet())
-            highlight_color = item->highlight_color();
 
         // It's going to resurrect, so we delete without notifying listeners.
         item->deleteObject(false);
@@ -465,18 +453,6 @@ sp_item_list_to_curves(const std::vector<SPItem*> &items, std::vector<SPItem*>&
         repr->setAttribute("class", class_attr);
         // add the new repr to the parent
         parent->appendChild(repr);
-        SPObject* newObj = document->getObjectByRepr(repr);
-        if (title && newObj) {
-            newObj->setTitle(title);
-            g_free(title);
-        }
-        if (desc && newObj) {
-            newObj->setDesc(desc);
-            g_free(desc);
-        }
-        if (highlight_color && newObj) {
-                SP_ITEM(newObj)->setHighlightColor( highlight_color );
-        }
 
         // move to the saved position
         repr->setPosition(pos > 0 ? pos : 0);
@@ -511,17 +487,8 @@ sp_selected_item_to_curved_repr(SPItem *item, guint32 /*text_grouping_policy*/)
         }
 
         g_repr->setAttribute("transform", item->getRepr()->attribute("transform"));
-        /* Mask */
-        gchar *mask_str = (gchar *) item->getRepr()->attribute("mask");
-        if ( mask_str )
-            g_repr->setAttribute("mask", mask_str);
-        /* Clip path */
-        gchar *clip_path_str = (gchar *) item->getRepr()->attribute("clip-path");
-        if ( clip_path_str )
-            g_repr->setAttribute("clip-path", clip_path_str);
-        /* Rotation center */
-        g_repr->setAttribute("inkscape:transform-center-x", item->getRepr()->attribute("inkscape:transform-center-x"), false);
-        g_repr->setAttribute("inkscape:transform-center-y", item->getRepr()->attribute("inkscape:transform-center-y"), false);
+
+        Inkscape::copy_object_properties(g_repr, item->getRepr());
 
         /* Whole text's style */
         Glib::ustring style_str =
@@ -599,6 +566,9 @@ sp_selected_item_to_curved_repr(SPItem *item, guint32 /*text_grouping_policy*/)
     }
 
     Inkscape::XML::Node *repr = xml_doc->createElement("svg:path");
+
+    Inkscape::copy_object_properties(repr, item->getRepr());
+
     /* Transformation */
     repr->setAttribute("transform", item->getRepr()->attribute("transform"));
 
@@ -607,20 +577,6 @@ sp_selected_item_to_curved_repr(SPItem *item, guint32 /*text_grouping_policy*/)
         item->style->write( SP_STYLE_FLAG_IFDIFF, SP_STYLE_SRC_UNSET, item->parent ? item->parent->style : nullptr); // TODO investigate possibility
     repr->setAttribute("style", style_str.c_str());
 
-    /* Mask */
-    gchar *mask_str = (gchar *) item->getRepr()->attribute("mask");
-    if ( mask_str )
-        repr->setAttribute("mask", mask_str);
-
-    /* Clip path */
-    gchar *clip_path_str = (gchar *) item->getRepr()->attribute("clip-path");
-    if ( clip_path_str )
-        repr->setAttribute("clip-path", clip_path_str);
-
-    /* Rotation center */
-    repr->setAttribute("inkscape:transform-center-x", item->getRepr()->attribute("inkscape:transform-center-x"), false);
-    repr->setAttribute("inkscape:transform-center-y", item->getRepr()->attribute("inkscape:transform-center-y"), false);
-
     /* Definition */
     gchar *def_str = sp_svg_write_path(curve->get_pathvector());
     repr->setAttribute("d", def_str);
@@ -688,6 +644,114 @@ ObjectSet::pathReverse()
     }
 }
 
+
+/**
+ * Copy generic attributes, like those from the "Object Properties" dialog,
+ * but also style and transformation center.
+ *
+ * @param dest XML node to copy attributes to
+ * @param src XML node to copy attributes from
+ */
+static void ink_copy_generic_attributes( //
+    Inkscape::XML::Node *dest,           //
+    Inkscape::XML::Node const *src)
+{
+    static char const *const keys[] = {
+        // core
+        "id",
+
+        // clip & mask
+        "clip-path",
+        "mask",
+
+        // style
+        "style",
+
+        // inkscape
+        "inkscape:highlight-color",
+        "inkscape:label",
+        "inkscape:transform-center-x",
+        "inkscape:transform-center-y",
+
+        // interactivity
+        "onclick",
+        "onmouseover",
+        "onmouseout",
+        "onmousedown",
+        "onmouseup",
+        "onmousemove",
+        "onfocusin",
+        "onfocusout",
+        "onload",
+    };
+
+    for (auto *key : keys) {
+        auto *value = src->attribute(key);
+        if (value) {
+            dest->setAttribute(key, value);
+        }
+    }
+}
+
+
+/**
+ * Copy generic child elements, like those from the "Object Properties" dialog
+ * (title and description) but also XML comments.
+ *
+ * Does not check if children of the same type already exist in dest.
+ *
+ * @param dest XML node to copy children to
+ * @param src XML node to copy children from
+ */
+static void ink_copy_generic_children( //
+    Inkscape::XML::Node *dest,         //
+    Inkscape::XML::Node const *src)
+{
+    static std::set<std::string> const names{
+        // descriptive elements
+        "svg:title",
+        "svg:desc",
+    };
+
+    for (const auto *child = src->firstChild(); child != nullptr; child = child->next()) {
+        // check if this child should be copied
+        if (!(child->type() == Inkscape::XML::COMMENT_NODE || //
+              (child->name() && names.count(child->name())))) {
+            continue;
+        }
+
+        auto dchild = child->duplicate(dest->document());
+        dest->appendChild(dchild);
+        dchild->release();
+    }
+}
+
+
+/**
+ * Copy generic object properties, like:
+ * - id
+ * - label
+ * - title
+ * - description
+ * - style
+ * - clip
+ * - mask
+ * - transformation center
+ * - highlight color
+ * - interactivity (event attributes)
+ *
+ * @param dest XML node to copy to
+ * @param src XML node to copy from
+ */
+void Inkscape::copy_object_properties( //
+    Inkscape::XML::Node *dest,         //
+    Inkscape::XML::Node const *src)
+{
+    ink_copy_generic_attributes(dest, src);
+    ink_copy_generic_children(dest, src);
+}
+
+
 /*
   Local Variables:
   mode:c++
diff --git a/src/path-chemistry.h b/src/path-chemistry.h
index 0276a6133..e5e711ae7 100644
--- a/src/path-chemistry.h
+++ b/src/path-chemistry.h
@@ -23,6 +23,8 @@ class ObjectSet;
 namespace XML {
 class Node;
 } // namespace XML
+
+void copy_object_properties(XML::Node *dest, XML::Node const *src);
 } // namespace Inkscape
 
 typedef unsigned int guint32;
diff --git a/src/splivarot.cpp b/src/splivarot.cpp
index 38d4c65ff..23895fe64 100644
--- a/src/splivarot.cpp
+++ b/src/splivarot.cpp
@@ -34,6 +34,7 @@
 #include "document.h"
 #include "layer-model.h"
 #include "message-stack.h"
+#include "path-chemistry.h"
 #include "selection.h"
 #include "text-editing.h"
 #include "verbs.h"
@@ -727,19 +728,10 @@ BoolOpErrors Inkscape::ObjectSet::pathBoolOp(bool_op bop, const bool skip_undo,
     gint pos = repr_source->position();
     Inkscape::XML::Node *parent = repr_source->parent();
     gchar const *id = repr_source->attribute("id");
-    gchar const *style = repr_source->attribute("style");
-    gchar const *mask = repr_source->attribute("mask");
-    gchar const *clip_path = repr_source->attribute("clip-path");
-    gchar *title = source->title();
-    gchar *desc = source->desc();
     // remove source paths
     clear();
     for (std::vector<SPItem*>::const_iterator l = il.begin(); l != il.end(); l++){
-        // if this is the bottommost object,
-        if (!strcmp(reinterpret_cast<SPObject *>(*l)->getRepr()->attribute("id"), id)) {
-            // delete it so that its clones don't get alerted; this object will be restored shortly, with the same id
-            (*l)->deleteObject(false);
-        } else {
+        if ((*l) != item_source) {
             // delete the object for real, so that its clones can take appropriate action
             (*l)->deleteObject();
         }
@@ -778,11 +770,14 @@ BoolOpErrors Inkscape::ObjectSet::pathBoolOp(bool_op bop, const bool skip_undo,
 
             Inkscape::XML::Document *xml_doc = doc->getReprDoc();
             Inkscape::XML::Node *repr = xml_doc->createElement("svg:path");
-            repr->setAttribute("style", style);
-            if (mask)
-                repr->setAttribute("mask", mask);
-            if (clip_path)
-                repr->setAttribute("clip-path", clip_path);
+
+            Inkscape::copy_object_properties(repr, repr_source);
+
+            // Delete source on last iteration (after we don't need repr_source anymore). As a consequence, the last
+            // item will inherit the original's id.
+            if (i + 1 == nbRP) {
+                item_source->deleteObject(false);
+            }
 
             repr->setAttribute("d", d);
             g_free(d);
@@ -799,11 +794,6 @@ BoolOpErrors Inkscape::ObjectSet::pathBoolOp(bool_op bop, const bool skip_undo,
                 sp_repr_css_attr_unref(css);
             }
 
-            // we assign the same id on all pieces, but it on adding to document, it will be changed on all except one
-            // this means it's basically random which of the pieces inherits the original's id and clones
-            // a better algorithm might figure out e.g. the biggest piece
-            repr->setAttribute("id", id);
-
             repr->setAttribute("transform", transform);
 
             // add the new repr to the parent
@@ -824,27 +814,18 @@ BoolOpErrors Inkscape::ObjectSet::pathBoolOp(bool_op bop, const bool skip_undo,
 
         Inkscape::XML::Document *xml_doc = doc->getReprDoc();
         Inkscape::XML::Node *repr = xml_doc->createElement("svg:path");
-        repr->setAttribute("style", style);
 
-        if ( mask )
-            repr->setAttribute("mask", mask);
+        Inkscape::copy_object_properties(repr, repr_source);
 
-        if ( clip_path )
-            repr->setAttribute("clip-path", clip_path);
+        // delete it so that its clones don't get alerted; this object will be restored shortly, with the same id
+        item_source->deleteObject(false);
 
         repr->setAttribute("d", d);
         g_free(d);
 
         repr->setAttribute("transform", transform);
 
-        repr->setAttribute("id", id);
         parent->appendChild(repr);
-        if (title) {
-            doc->getObjectByRepr(repr)->setTitle(title);
-        }
-        if (desc) {
-            doc->getObjectByRepr(repr)->setDesc(desc);
-        }
         repr->setPosition(pos > 0 ? pos : 0);
 
         add(doc->getObjectByRepr(repr));
@@ -852,8 +833,6 @@ BoolOpErrors Inkscape::ObjectSet::pathBoolOp(bool_op bop, const bool skip_undo,
     }
 
     g_free(transform);
-    if (title) g_free(title);
-    if (desc) g_free(desc);
 
     delete res;
 
@@ -1260,8 +1239,6 @@ sp_item_path_outline(SPItem *item, SPDesktop *desktop, bool legacy)
 
         Geom::Affine const transform(item->transform);
         float const scale = transform.descrim();
-        gchar const *mask = item->getRepr()->attribute("mask");
-        gchar const *clip_path = item->getRepr()->attribute("clip-path");
 
         Path *orig = new Path;
         Path *res = new Path;
@@ -1375,12 +1352,6 @@ sp_item_path_outline(SPItem *item, SPDesktop *desktop, bool legacy)
         gint pos = item->getRepr()->position();
         // remember parent
         Inkscape::XML::Node *parent = item->getRepr()->parent();
-        // remember id
-        char const *id = item->getRepr()->attribute("id");
-        // remember title
-        gchar *title = item->title();
-        // remember description
-        gchar *desc = item->desc();
         
         if (res->descr_cmd.size() > 1) { // if there's 0 or 1 node left, drop this path altogether
 
@@ -1399,15 +1370,13 @@ sp_item_path_outline(SPItem *item, SPDesktop *desktop, bool legacy)
                 gchar *str = orig->svg_dump_path();
                 stroke->setAttribute("d", str);
                 g_free(str);
-
-                if (mask)
-                    stroke->setAttribute("mask", mask);
-                if (clip_path)
-                    stroke->setAttribute("clip-path", clip_path);
             }
 
             if (SP_IS_SHAPE(item)) {
                 Inkscape::XML::Node *g_repr = xml_doc->createElement("svg:g");
+                Inkscape::copy_object_properties(g_repr, item->getRepr());
+                // drop copied style, children will be re-styled (stroke becomes fill)
+                g_repr->setAttribute("style", nullptr);
 
                 // add the group to the parent
                 parent->appendChild(g_repr);
@@ -1427,23 +1396,11 @@ sp_item_path_outline(SPItem *item, SPDesktop *desktop, bool legacy)
                         gchar *str = sp_svg_write_path( pathv );
                         fill->setAttribute("d", str);
                         g_free(str);
-
-                        if (mask)
-                            fill->setAttribute("mask", mask);
-                        if (clip_path)
-                            fill->setAttribute("clip-path", clip_path);
                     }
                 }
-                // restore title, description, id, transform
-                g_repr->setAttribute("id", id);
+                // restore transform
                 SPItem *newitem = (SPItem *) doc->getObjectByRepr(g_repr);
                 newitem->doWriteTransform(transform);
-                if (title) {
-                	newitem->setTitle(title);
-                }
-                if (desc) {
-                	newitem->setDesc(desc);
-                }
                 SPShape *shape = SP_SHAPE(item);
 
                 Geom::PathVector const & pathv = curve->get_pathvector();
@@ -1526,12 +1483,6 @@ sp_item_path_outline(SPItem *item, SPDesktop *desktop, bool legacy)
                                                                  markers, xml_doc, doc, desktop, legacy);
                         }
                     }
-                    if (!legacy) {
-                        if (mask)
-                            markers->setAttribute("mask", mask);
-                        if (clip_path)
-                            markers->setAttribute("clip-path", clip_path);
-                    }
                 }
 
                 gchar const *paint_order = sp_repr_css_property(ncss, "paint-order", nullptr);
@@ -1665,15 +1616,6 @@ sp_item_path_outline(SPItem *item, SPDesktop *desktop, bool legacy)
             }
         }
 
-        if (title) {
-            g_free(title);
-            title = nullptr;
-        }
-        if (desc) {
-            g_free(desc);
-            desc = nullptr;
-        }
-
         delete res;
         delete orig;
     }
@@ -1796,9 +1738,6 @@ void sp_selected_path_create_offset_object(SPDesktop *desktop, int expand, bool
 
     item->doWriteTransform(Geom::identity());
 
-    //XML Tree being used directly here while it shouldn't be...
-    gchar *style = g_strdup(item->getRepr()->attribute("style"));
-
     // remember the position of the item
     gint pos = item->getRepr()->position();
 
@@ -1820,7 +1759,6 @@ void sp_selected_path_create_offset_object(SPDesktop *desktop, int expand, bool
     Path *orig = Path_for_item(item, true, false);
     if (orig == nullptr)
     {
-        g_free(style);
         curve->unref();
         return;
     }
@@ -1873,13 +1811,20 @@ void sp_selected_path_create_offset_object(SPDesktop *desktop, int expand, bool
 
         delete res;
         delete orig;
-        g_free(style);
         return;
     }
 
     {
         Inkscape::XML::Document *xml_doc = desktop->doc()->getReprDoc();
         Inkscape::XML::Node *repr = xml_doc->createElement("svg:path");
+
+        if (!updating) {
+            Inkscape::copy_object_properties(repr, item->getRepr());
+        } else {
+            gchar const *style = item->getRepr()->attribute("style");
+            repr->setAttribute("style", style);
+        }
+
         repr->setAttribute("sodipodi:type", "inkscape:offset");
         sp_repr_set_svg_double(repr, "inkscape:radius", ( expand > 0
                                                           ? o_width
@@ -1902,10 +1847,10 @@ void sp_selected_path_create_offset_object(SPDesktop *desktop, int expand, bool
             g_free((void *) uri);
         } else {
             repr->setAttribute("inkscape:href", nullptr);
+            // delete original
+            item->deleteObject(false);
         }
 
-        repr->setAttribute("style", style);
-
         // add the new repr to the parent
         parent->appendChild(repr);
 
@@ -1915,10 +1860,7 @@ void sp_selected_path_create_offset_object(SPDesktop *desktop, int expand, bool
         SPItem *nitem = reinterpret_cast<SPItem *>(desktop->getDocument()->getObjectByRepr(repr));
 
         if ( !updating ) {
-            // delete original, apply the transform to the offset
-            const char *n_id = item->getRepr()->attribute("id");
-            item->deleteObject(false);
-            repr->setAttribute("id", n_id);
+            // apply the transform to the offset
             nitem->doWriteTransform(transform);
         }
 
@@ -1939,8 +1881,6 @@ void sp_selected_path_create_offset_object(SPDesktop *desktop, int expand, bool
 
     delete res;
     delete orig;
-
-    g_free(style);
 }
 
 
@@ -1989,8 +1929,6 @@ sp_selected_path_do_offset(SPDesktop *desktop, bool expand, double prefOffset)
 
         item->doWriteTransform(Geom::identity());
 
-        gchar *style = g_strdup(item->getRepr()->attribute("style"));
-
         float o_width = 0;
         float o_miter = 0;
         JoinType o_join = join_straight;
@@ -2021,7 +1959,6 @@ sp_selected_path_do_offset(SPDesktop *desktop, bool expand, double prefOffset)
 
         Path *orig = Path_for_item(item, false);
         if (orig == nullptr) {
-            g_free(style);
             curve->unref();
             continue;
         }
@@ -2104,18 +2041,21 @@ sp_selected_path_do_offset(SPDesktop *desktop, bool expand, double prefOffset)
         gint pos = item->getRepr()->position();
         // remember parent
         Inkscape::XML::Node *parent = item->getRepr()->parent();
-        // remember id
-        char const *id = item->getRepr()->attribute("id");
 
         selection->remove(item);
-        item->deleteObject(false);
+
+        Inkscape::XML::Node *repr = nullptr;
 
         if (res->descr_cmd.size() > 1) { // if there's 0 or 1 node left, drop this path altogether
             Inkscape::XML::Document *xml_doc = desktop->doc()->getReprDoc();
-            Inkscape::XML::Node *repr = xml_doc->createElement("svg:path");
+            repr = xml_doc->createElement("svg:path");
 
-            repr->setAttribute("style", style);
+            Inkscape::copy_object_properties(repr, item->getRepr());
+        }
 
+        item->deleteObject(false);
+
+        if (repr) {
             gchar *str = res->svg_dump_path();
             repr->setAttribute("d", str);
             g_free(str);
@@ -2131,8 +2071,6 @@ sp_selected_path_do_offset(SPDesktop *desktop, bool expand, double prefOffset)
             // reapply the transform
             newitem->doWriteTransform(transform);
 
-            repr->setAttribute("id", id);
-
             selection->add(repr);
 
             Inkscape::GC::release(repr);
@@ -2201,30 +2139,18 @@ sp_selected_path_simplify_item(SPDesktop *desktop,
     */
     item->doWriteTransform(Geom::identity());
 
-    gchar *style = g_strdup(item->getRepr()->attribute("style"));
-    gchar *mask = g_strdup(item->getRepr()->attribute("mask"));
-    gchar *clip_path = g_strdup(item->getRepr()->attribute("clip-path"));
-
     // remember the position of the item
     gint pos = item->getRepr()->position();
     // remember parent
     Inkscape::XML::Node *parent = item->getRepr()->parent();
-    // remember id
-    char const *id = item->getRepr()->attribute("id");
     // remember path effect
     char const *patheffect = item->getRepr()->attribute("inkscape:path-effect");
-    // remember title
-    gchar *title = item->title();
-    // remember description
-    gchar *desc = item->desc();
     
     //If a group was selected, to not change the selection list
     if (modifySelection) {
         selection->remove(item);
     }
 
-    item->deleteObject(false);
-
     if ( justCoalesce ) {
         orig->Coalesce(threshold * size);
     } else {
@@ -2235,19 +2161,10 @@ sp_selected_path_simplify_item(SPDesktop *desktop,
     Inkscape::XML::Document *xml_doc = desktop->doc()->getReprDoc();
     Inkscape::XML::Node *repr = xml_doc->createElement("svg:path");
 
-    // restore style, mask and clip-path
-    repr->setAttribute("style", style);
-    g_free(style);
+    // restore attributes
+    Inkscape::copy_object_properties(repr, item->getRepr());
 
-    if ( mask ) {
-        repr->setAttribute("mask", mask);
-        g_free(mask);
-    }
-
-    if ( clip_path ) {
-        repr->setAttribute("clip-path", clip_path);
-        g_free(clip_path);
-    }
+    item->deleteObject(false);
 
     // restore path effect
     repr->setAttribute("inkscape:path-effect", patheffect);
@@ -2260,9 +2177,6 @@ sp_selected_path_simplify_item(SPDesktop *desktop,
         repr->setAttribute("d", str);
     g_free(str);
 
-    // restore id
-    repr->setAttribute("id", id);
-
     // add the new repr to the parent
     parent->appendChild(repr);
 
@@ -2274,16 +2188,6 @@ sp_selected_path_simplify_item(SPDesktop *desktop,
     // reapply the transform
     newitem->doWriteTransform(transform);
 
-    // restore title & description
-    if (title) {
-    	newitem->setTitle(title);
-        g_free(title);
-    }
-    if (desc) {
-    	newitem->setDesc(desc);
-        g_free(desc);
-    }
-    
     //If we are not in a selected group
     if (modifySelection)
         selection->add(repr);