View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0017418 | Scribus | General | public | 2025-02-13 17:10 | 2025-02-25 08:36 |
Reporter | tropion | Assigned To | |||
Priority | normal | Severity | minor | Reproducibility | always |
Status | new | Resolution | open | ||
Summary | 0017418: It is possible to create objects with non-unique names by copying groups. | ||||
Description | It 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 Reproduce | Create 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. | ||||
Tags | No tags attached. | ||||
Patch | |||||
|
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")) |
|
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() }; ... |
|
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. |