View Issue Details

IDProjectCategoryView StatusLast Update
0017418ScribusGeneralpublic2025-02-25 08:36
Reportertropion Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Summary0017418: It is possible to create objects with non-unique names by copying groups.
DescriptionIt is possible to create objects with non-unique names by copying groups.

This is relevant in combination with 0017417 which would make scribus rely on unique names for PageItems.
Steps To ReproduceCreate to Objects.
Change the name of one of them.
Group the objects.

Copy and paste the group.
Look at the names of the elements of the copy.

The autogenerated name is changed to a unique name, the name you changed now exists twice.
TagsNo tags attached.
Patch

Activities

tropion

2025-02-13 17:10

reporter  

keep_names_unique_copying_groups.diff (1,700 bytes)   
diff --git a/scribus/plugins/fileloader/scribus170format/scribus170format.cpp b/scribus/plugins/fileloader/scribus170format/scribus170format.cpp
index b19e858ec..b8dff1e9f 100644
--- a/scribus/plugins/fileloader/scribus170format/scribus170format.cpp
+++ b/scribus/plugins/fileloader/scribus170format/scribus170format.cpp
@@ -4504,9 +4504,7 @@ bool Scribus170Format::readObject(ScribusDoc* doc, ScXmlStreamReader& reader, co
 			if (newItem->isGroup())
 			{
 			//	bool success = true;
-				QList<PageItem*>* docItems = doc->Items;
-				QList<PageItem*> groupItems;
-				doc->Items = &groupItems;
+				qsizetype docItemCount{ doc->Items->size() };
 				ItemInfo itemInfo;
 				// #12313: set the 'loadPage' parameter to true in order to
 				// avoid the change of page mode and the doc item lists switch
@@ -4516,16 +4514,17 @@ bool Scribus170Format::readObject(ScribusDoc* doc, ScXmlStreamReader& reader, co
 				readObjectParams2.itemKind = (itemKind == PageItem::PatternItem) ? PageItem::PatternItem : PageItem::StandardItem;
 				readObjectParams2.loadingPage = true;
 				readObject(doc, reader, readObjectParams2, itemInfo);
-				for (int as = 0; as < groupItems.count(); ++as)
+				
+				for (int as = docItemCount; as < doc->Items->count(); ++as)
 				{
-					PageItem* currItem = groupItems.at(as);
+					PageItem* currItem = doc->Items->at(as);
+					doc->Items->removeAt(as);
 					newItem->groupItemList.append(currItem);
 					currItem->Parent = newItem;
 					currItem->m_layerID = newItem->m_layerID;
 					currItem->OwnPage = newItem->OwnPage;
 					currItem->OnMasterPage = newItem->OnMasterPage;
 				}
-				doc->Items = docItems;
 			}
 		}
 		if (tName == QLatin1String("MARK"))

ale

2025-02-22 11:49

manager   ~0052101

itemId shall indeed be unique.

but I don't really understand what your code does.

in such cases, i think that it would be appropriate to add a comment at the beginning of the branch:

             if (newItem->isGroup())
             {
                 /*
                  * For groups, read the number of groups, do this do this
                  * ...
                  * and finally do that
                  */
                qsizetype docItemCount{ doc->Items->size() };
                ...

tropion

2025-02-25 08:36

reporter   ~0052121

Scribus170Format::readObject() reads an object from the ScXmlStreamReader and calls Scribus170Format::pasteItem() to put it into doc->Items.
pasteItem() tries to give the object an unique name using PageItem::setItemName() which uses PageItem::generateUniqueCopyName() which uses ScribusDoc::itemNameExists() to look through doc->Items if the name exists anywhere.

This failed in this case, because doc->Items was temporarily set to the local QList<PageItem*> groupItems.

The patch changes this so that doc->Items not changed, readObject() pastes the object into doc->Items and can correctly generateUniqueCopyName(). The object is then moved from doc->Items to newItem->groupItemList.

The loop in which the new object is moved to newItem->groupItemList seems unnecessary to me. I did not find a way to get more than one object from readObject(), but I do not yet understand readObject() enough to be certain.

I just noticed that exactly the same code exists in Scribus150Format.

Issue History

Date Modified Username Field Change
2025-02-13 17:10 tropion New Issue
2025-02-13 17:10 tropion File Added: keep_names_unique_copying_groups.diff
2025-02-22 10:39 ale Project Infrastructure => Scribus
2025-02-22 11:49 ale Note Added: 0052101
2025-02-25 08:36 tropion Note Added: 0052121