From 710e1ca0e2443882e2a1d5294bc9c5d757bef950 Mon Sep 17 00:00:00 2001
From: Max Gaukler <development@maxgaukler.de>
Date: Sat, 4 Jan 2025 19:37:47 +0100
Subject: [PATCH] Fix Export batch dialog crash / misbehavior with multiple
 documents open

Fixes the issue that switching between (nonempty) documents causes the export-batch dialog to show too more and more objects with every switch. The internal state became inconsistent in one edge case that was not triggered before.

As a lucky coincidence, it also fixes a crash when closing the export dialog (not sure why):

Fixes https://gitlab.com/inkscape/inbox/-/issues/11496
---
 src/ui/dialog/export-batch.cpp | 32 ++++++++++++++++++++++++++++++--
 src/ui/dialog/export-batch.h   |  3 +++
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/ui/dialog/export-batch.cpp b/src/ui/dialog/export-batch.cpp
index bfa7732423..d6417937bf 100644
--- a/src/ui/dialog/export-batch.cpp
+++ b/src/ui/dialog/export-batch.cpp
@@ -102,6 +102,8 @@ void BatchItem::update_label()
     set_tooltip_text(label);
 }
 
+/// @brief Set "Export selected only"
+/// @param isolate true if only selected items should be shown in export
 void BatchItem::setIsolateItem(bool isolate)
 {
     if (_isolate_item != isolate) {
@@ -279,10 +281,23 @@ void BatchItem::setDrawing(std::shared_ptr<PreviewDrawing> drawing)
 
 /**
  * Add and remove batch items and their previews carefully and insert new ones into the container FlowBox
+ *
+ * @param items List of batch-items (e.g. layers) in the batch dialog. Is updated by this function.
+ * @param objects New list of objects (e.g. layers). Taken as input.
+ * @param container GUI widget containing the selection of batch items. Is updated by this function.
+ * @param isolate_items true if only the given (selected) items should be shown in export
  */
 void BatchItem::syncItems(BatchItems &items, std::map<std::string, SPObject*> const &objects, Gtk::FlowBox &container, std::shared_ptr<PreviewDrawing> preview, bool isolate_items)
 {
-    // Remove any items not in objects
+    // Pre- and post-condition of this function is that
+    // `items` and `container.children` have the same content.
+    // (They have different types and contain slightly different information,
+    // but there is still a 1:1 correspondence.)
+    //
+    // We update `items` so that it matches `objects`.
+    // Any necessary change to `items` is also applied to `container.children`.
+
+    // a) Remove any items not in objects
     for (auto it = items.begin(); it != items.end();) {
         if (!objects.contains(it->first)) {
             container.remove(*it->second);
@@ -293,10 +308,11 @@ void BatchItem::syncItems(BatchItems &items, std::map<std::string, SPObject*> co
         }
     }
 
+    // b) Add any objects not in items
+
     // A special container for pages allows them to be sorted correctly
     std::set<SPPage *, SPPage::PageIndexOrder> pages;
 
-    // Add any objects not in items
     for (auto &[id, obj] : objects) {
         if (auto page = cast<SPPage>(obj)) {
             if (!items[id] || items[id]->getPage() != page)
@@ -310,6 +326,11 @@ void BatchItem::syncItems(BatchItems &items, std::map<std::string, SPObject*> co
         if (items[id] && items[id]->getItem() == item)
             continue;
 
+        if (items[id]) {
+            // Remove existing item with same id
+            // (can occur when switching between document tabs)
+            container.remove(*items[id]);
+        }
         // Add new item to the end of list
         items[id] = std::make_unique<BatchItem>(item, isolate_items, preview);
         container.insert(*items[id], -1);
@@ -318,11 +339,18 @@ void BatchItem::syncItems(BatchItems &items, std::map<std::string, SPObject*> co
 
     for (auto &page : pages) {
         if (auto id = page->getId()) {
+            if (items[id]) {
+                container.remove(*items[id]);
+            }
             items[id] = std::make_unique<BatchItem>(page, preview);
             container.insert(*items[id], -1);
             items[id]->set_selected(true);
         }
     }
+
+    // Check postconditions
+    g_assert(items.size() == objects.size());
+    g_assert(container.get_children().size() == items.size());
 }
 
 BatchExport::BatchExport(BaseObjectType * const cobject, Glib::RefPtr<Gtk::Builder> const &builder)
diff --git a/src/ui/dialog/export-batch.h b/src/ui/dialog/export-batch.h
index 59427c1d06..dfaa7a0608 100644
--- a/src/ui/dialog/export-batch.h
+++ b/src/ui/dialog/export-batch.h
@@ -82,6 +82,9 @@ public:
     void on_mode_changed(Gtk::SelectionMode mode);
     void set_selected(bool selected);
     void update_selected();
+
+    /// @brief Get "Export selected only" setting
+    /// @returns true if only selected items are shown in export
     bool isolateItem() const { return _isolate_item; }
     void setIsolateItem(bool isolate);
 
-- 
GitLab