View Issue Details

IDProjectCategoryView StatusLast Update
0016323ScribusGeneralpublic2021-03-23 06:25
Reporterachalk Assigned Tojghali  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version1.4.8 
Fixed in Version1.5.7.svn 
Summary0016323: Starting page not stored
DescriptionTo reproduce: Set starting page to "Left" and "Apply".
Next document you create (after a restart) of Scribus has the option set back to "Right".
Steps To ReproduceTo reproduce: Set starting page to "Left" and "Apply".
Next document you create (after a restart) of Scribus has the option set back to "Right".
Tagsoptions, start page
PatchYes

Activities

ale

2020-11-09 07:27

manager   ~0048351

this still happens in 1.5.6svn...

ale

2020-11-10 14:44

manager   ~0048373

the code is checking for a PageNames tag around the Set.
but there is only a PageNames inside of the Set.

the whole thing feels way over complicated -- and this made tracking the error way too hard -- but, well, here is a patch.
page-layout-left.diff (3,029 bytes)   
diff --git a/scribus/prefsmanager.cpp b/scribus/prefsmanager.cpp
index 383704b04..6299dfb70 100644
--- a/scribus/prefsmanager.cpp
+++ b/scribus/prefsmanager.cpp
@@ -2351,40 +2351,34 @@ bool PrefsManager::ReadPref(const QString& ho)
 		if (dc.tagName()=="PageSets")
 		{
 			QDomNode PGS = DOC.firstChild();
-			if  (!PGS.namedItem("PageNames").isNull())
+			appPrefs.pageSets.clear();
+			while (!PGS.isNull())
 			{
-				appPrefs.pageSets.clear();
-				while (!PGS.isNull())
+				QDomElement PgsAttr = PGS.toElement();
+				if(PgsAttr.tagName() == "Set")
 				{
-					QDomElement PgsAttr = PGS.toElement();
-					if(PgsAttr.tagName() == "Set")
+					struct PageSet pageS;
+					pageS.Name = PgsAttr.attribute("Name");
+					pageS.FirstPage = PgsAttr.attribute("FirstPage", "0").toInt();
+					pageS.Rows = PgsAttr.attribute("Rows", "1").toInt();
+					pageS.Columns = PgsAttr.attribute("Columns", "1").toInt();
+					pageS.pageNames.clear();
+					QDomNode PGSN = PGS.firstChild();
+					while (!PGSN.isNull())
 					{
-						struct PageSet pageS;
-						pageS.Name = PgsAttr.attribute("Name");
-						pageS.FirstPage = PgsAttr.attribute("FirstPage", "0").toInt();
-						pageS.Rows = PgsAttr.attribute("Rows", "1").toInt();
-						pageS.Columns = PgsAttr.attribute("Columns", "1").toInt();
-//						pageS.GapHorizontal = PgsAttr.attribute("GapHorizontal", "0").toDouble();
-//						pageS.GapVertical = PgsAttr.attribute("GapVertical", "0").toDouble();
-//						pageS.GapBelow = PgsAttr.attribute("GapBelow", "0").toDouble();
-						pageS.pageNames.clear();
-						QDomNode PGSN = PGS.firstChild();
-						while (!PGSN.isNull())
-						{
-							QDomElement PgsAttrN = PGSN.toElement();
-							if(PgsAttrN.tagName() == "PageNames")
-								pageS.pageNames.append(PgsAttrN.attribute("Name"));
-							PGSN = PGSN.nextSibling();
-						}
-						appPrefs.pageSets.append(pageS);
-						if ((appPrefs.pageSets.count() == appPrefs.docSetupPrefs.pagePositioning) && ((appPrefs.displayPrefs.pageGapHorizontal < 0) && (appPrefs.displayPrefs.pageGapVertical < 0)))
-						{
-							appPrefs.displayPrefs.pageGapHorizontal = ScCLocale::toDoubleC(PgsAttr.attribute("GapHorizontal"), 0.0);
-							appPrefs.displayPrefs.pageGapVertical   = ScCLocale::toDoubleC(PgsAttr.attribute("GapBelow"), 40.0);
-						}
+						QDomElement PgsAttrN = PGSN.toElement();
+						if(PgsAttrN.tagName() == "PageNames")
+							pageS.pageNames.append(PgsAttrN.attribute("Name"));
+						PGSN = PGSN.nextSibling();
+					}
+					appPrefs.pageSets.append(pageS);
+					if ((appPrefs.pageSets.count() == appPrefs.docSetupPrefs.pagePositioning) && ((appPrefs.displayPrefs.pageGapHorizontal < 0) && (appPrefs.displayPrefs.pageGapVertical < 0)))
+					{
+						appPrefs.displayPrefs.pageGapHorizontal = ScCLocale::toDoubleC(PgsAttr.attribute("GapHorizontal"), 0.0);
+						appPrefs.displayPrefs.pageGapVertical   = ScCLocale::toDoubleC(PgsAttr.attribute("GapBelow"), 40.0);
 					}
-					PGS = PGS.nextSibling();
 				}
+				PGS = PGS.nextSibling();
 			}
 		}
 
page-layout-left.diff (3,029 bytes)   

jghali

2020-11-11 13:12

administrator   ~0048378

Last edited: 2020-11-11 13:17

Qt's QDomNode::namedItem() is buggy but it is here to avoid ending with an empty page set list. So it is necessary to add a few more safety checks to avoid potential crashes on startup.

Issue History

Date Modified Username Field Change
2020-11-08 16:22 achalk New Issue
2020-11-08 16:22 achalk Tag Attached: options
2020-11-08 16:22 achalk Tag Attached: start page
2020-11-09 07:27 ale Note Added: 0048351
2020-11-10 14:44 ale File Added: page-layout-left.diff
2020-11-10 14:44 ale Note Added: 0048373
2020-11-10 14:44 ale Summary Starting page not stored => [PATCH] Starting page not stored
2020-11-10 14:44 ale Patch No => Yes
2020-11-11 13:12 jghali Note Added: 0048378
2020-11-11 13:17 jghali Note Edited: 0048378
2020-11-11 13:21 jghali Assigned To => jghali
2020-11-11 13:21 jghali Status new => resolved
2020-11-11 13:21 jghali Resolution open => fixed
2020-11-11 13:21 jghali Fixed in Version => 1.5.7.svn
2020-11-11 13:21 jghali Summary [PATCH] Starting page not stored => Starting page not stored
2021-03-23 06:25 cbradney Status resolved => closed