Merge branch '646-fix-settingpanel'

* 646-fix-settingpanel:
  ctkSettingsPanel: Ensure "settingChanged" signal is emitted only if needed.
  ctkSettingsPanel: Fix changedSettings() for empty QStringList
  ctkSettingsPanel: Add setter/getter for PropertyType::PreviousValue
  ctkSettingsPanelTest1: Check settings after reloading from file
  ctkSettingsPanelTest1: Improve readability splitting test in smaller functions
  ctkSettingsPanelTest1: Explicitly compare QVariant
  Extend ctkTesting(Utilities|Macro) to support QVariant
  ctkSettingsPanelTest1: Test return value of "changedSettings()"
  ctkSettingsPanelTest1: Add "QStringList" test case
  Extend ctkTesting(Utilities|Macro) to support QStringList

Libs/Core/Testing/Cpp/ctkCoreTestingMacrosTest.cpp

@@ -60,6 +60,12 @@ int TestCheckStdStringDifferentFailure();
 int TestCheckQStringDifferentSuccess();
 int TestCheckQStringDifferentFailure();
 
+int TestCheckQStringListSuccess();
+int TestCheckQStringListFailure();
+
+int TestCheckQVariantSuccess();
+int TestCheckQVariantFailure();
+
 //----------------------------------------------------------------------------
 #define TestMacro(MACRO_NAME) \
   if (Test##MACRO_NAME##Success() != EXIT_SUCCESS) \
@@ -86,6 +92,8 @@ int ctkCoreTestingMacrosTest(int , char * [])
   TestMacro(CheckQString)
   TestMacro(CheckStdStringDifferent)
   TestMacro(CheckQStringDifferent)
+  TestMacro(CheckQStringList)
+  TestMacro(CheckQVariant)
   return EXIT_SUCCESS;
 }
 
@@ -335,3 +343,45 @@ int TestCheckQStringDifferentFailure()
   return EXIT_SUCCESS;
 }
 
+//----------------------------------------------------------------------------
+// Test CHECK_QSTRINGLIST
+
+//----------------------------------------------------------------------------
+int TestCheckQStringListSuccess()
+{
+  QStringList actual = QStringList() << "a" << "b" << "c";
+  QStringList expected = actual;
+  CHECK_QSTRINGLIST(actual, expected);
+  return EXIT_SUCCESS;
+}
+
+//----------------------------------------------------------------------------
+int TestCheckQStringListFailure()
+{
+  QStringList actual = QStringList() << "a" << "b" << "c";
+  QStringList expected = QStringList() << "a" << "x" << "c";
+  CHECK_QSTRINGLIST(actual, expected);
+  return EXIT_SUCCESS;
+}
+
+//----------------------------------------------------------------------------
+// Test CHECK_QVARIANT
+
+//----------------------------------------------------------------------------
+int TestCheckQVariantSuccess()
+{
+  QVariant actual = QVariant(4);
+  QVariant expected = actual;
+  CHECK_QVARIANT(actual, expected);
+  return EXIT_SUCCESS;
+}
+
+//----------------------------------------------------------------------------
+int TestCheckQVariantFailure()
+{
+  QVariant actual = QVariant(4);
+  QVariant expected = QVariant(2);
+  CHECK_QVARIANT(actual, expected);
+  return EXIT_SUCCESS;
+}
+

Libs/Core/Testing/Cpp/ctkCoreTestingUtilitiesTest.cpp

@@ -29,6 +29,8 @@ bool TestCheckNotNull();
 bool TestCheckNull();
 bool TestCheckPointer();
 bool TestCheckString();
+bool TestCheckStringList();
+bool TestCheckVariant();
 
 //----------------------------------------------------------------------------
 int ctkCoreTestingUtilitiesTest(int , char * [])
@@ -39,6 +41,8 @@ int ctkCoreTestingUtilitiesTest(int , char * [])
   res = res && TestCheckNull();
   res = res && TestCheckPointer();
   res = res && TestCheckString();
+  res = res && TestCheckStringList();
+  res = res && TestCheckVariant();
   return res ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
@@ -110,3 +114,35 @@ bool TestCheckString()
   return true;
 }
 
+//----------------------------------------------------------------------------
+bool TestCheckStringList()
+{
+  QStringList abc = QStringList() << "a" << "b" << "c";
+  QStringList axc = QStringList() << "a" << "x" << "c";
+  QStringList abcd = QStringList() << "a" << "b" << "c" << "d";
+  if (!CheckStringList(__LINE__, "TestCheckStringList", QStringList(), QStringList())
+      ||!CheckStringList(__LINE__, "TestCheckStringList", abc, abc)
+      || CheckStringList(__LINE__, "TestCheckStringList Expected Failure", abc, axc)
+      || CheckStringList(__LINE__, "TestCheckStringList Expected Failure", abc, abcd))
+    {
+    qWarning() << "Line " << __LINE__ << " - TestCheckString failed";
+    return false;
+    }
+  return true;
+}
+
+//----------------------------------------------------------------------------
+bool TestCheckVariant()
+{
+  QVariant foo = QVariant(4);
+  QVariant bar = QVariant(2);
+  if (!CheckVariant(__LINE__, "TestCheckVariant", QVariant(), QVariant())
+      ||!CheckVariant(__LINE__, "TestCheckVariant", foo, foo)
+      || CheckVariant(__LINE__, "TestCheckVariant Expected Failure", foo, bar))
+    {
+    qWarning() << "Line " << __LINE__ << " - TestCheckVariant failed";
+    return false;
+    }
+  return true;
+}
+

Libs/Core/ctkCoreTestingMacros.h

@@ -183,5 +183,27 @@
     } \
   }
 
+/// Verifies if actual QStringList is the same as expected.
+#define CHECK_QSTRINGLIST(actual, expected) \
+  { \
+  QStringList a = (actual); \
+  QStringList e = (expected); \
+  if (!ctkCoreTestingUtilities::CheckStringList(__LINE__,#actual " != " #expected, a, e)) \
+    { \
+    return EXIT_FAILURE; \
+    } \
+  }
+
+/// Verifies if actual QVariant is the same as expected.
+#define CHECK_QVARIANT(actual, expected) \
+  { \
+  QVariant a = (actual); \
+  QVariant e = (expected); \
+  if (!ctkCoreTestingUtilities::CheckVariant(__LINE__,#actual " != " #expected, a, e)) \
+    { \
+    return EXIT_FAILURE; \
+    } \
+  }
+
 #endif
 

Libs/Core/ctkCoreTestingUtilities.cpp

@@ -71,5 +71,19 @@ bool CheckString(int line, const QString& description,
   return true;
 }
 
+//----------------------------------------------------------------------------
+bool CheckStringList(int line, const QString& description,
+                     const QStringList& current, const QStringList& expected)
+{
+  return CheckList<QString>(line, description, current, expected, "CheckStringList");
+}
+
+//----------------------------------------------------------------------------
+bool CheckVariant(int line, const QString& description,
+                  const QVariant& current, const QVariant& expected)
+{
+  return Check<QVariant>(line, description, current, expected, "CheckVariant");
+}
+
 } // namespace ctkCoreTestingUtilities
 

Libs/Core/ctkCoreTestingUtilities.h

@@ -26,6 +26,8 @@
 
 // Qt includes
 #include <QString>
+#include <QStringList>
+#include <QVariant>
 
 /// This module provides functions to facilitate writing tests.
 ///
@@ -69,6 +71,14 @@ CTK_CORE_EXPORT
 bool CheckString(int line, const QString& description,
                  const char* current, const char* expected, bool errorIfDifferent = true );
 
+CTK_CORE_EXPORT
+bool CheckStringList(int line, const QString& description,
+                     const QStringList& current, const QStringList& expected);
+
+CTK_CORE_EXPORT
+bool CheckVariant(int line, const QString& description,
+                  const QVariant& current, const QVariant& expected);
+
 } // namespace ctkCoreTestingUtilities
 
 #include "ctkCoreTestingUtilities.tpp"

Libs/Core/ctkCoreTestingUtilities.tpp

@@ -57,5 +57,38 @@ bool Check(int line, const QString& description,
   return true;
 }
 
+//----------------------------------------------------------------------------
+template<typename TYPE>
+bool CheckList(int line, const QString& description,
+               const QList<TYPE>& current, const QList<TYPE>& expected,
+               const QString& testName)
+{
+  QString msg;
+  if (current.count() != expected.count())
+    {
+    qWarning() << "\nLine " << line << " - " << description
+               << " : " << testName << " failed"
+               << "\nCompared lists have different sizes."
+               << "\n\tcurrent size :" << current.count()
+               << "\n\texpected size:" << expected.count()
+               << "\n\tcurrent:" << current
+               << "\n\texpected:" << expected;
+    return false;
+    }
+  for (int idx = 0; idx < current.count(); ++idx)
+    {
+    if (current.at(idx) != expected.at(idx))
+      {
+      qWarning() << "\nLine " << line << " - " << description
+                 << " : " << testName << " failed"
+                 << "\nCompared lists differ at index " << idx
+                 << "\n\tcurrent[" << idx << "] :" << current.at(idx)
+                 << "\n\texpected[" << idx << "]:" << expected.at(idx);
+      return false;
+      }
+    }
+  return true;
+}
+
 } // namespace ctkCoreTestingUtilities
 

Libs/Widgets/Testing/Cpp/ctkSettingsPanelTest.cpp

@@ -219,6 +219,28 @@ void ctkSettingsPanelTester::testChangeProperty_data()
   QTest::newRow("label RequireRestart unchanged panel") << QString("label") << ctkSettingsPanel::SettingOptions(ctkSettingsPanel::OptionRequireRestart) << 1 << false << true << 0 << QStringList();
 }
 
+namespace
+{
+//-----------------------------------------------------------------------------
+class ctkSettingsPanelForTest : public ctkSettingsPanel
+{
+public:
+  QVariant myDefaultPropertyValue(const QString& key) const
+    {
+    return this->defaultPropertyValue(key);
+    }
+  QVariant myPreviousPropertyValue(const QString& key) const
+    {
+    return this->previousPropertyValue(key);
+    }
+  QVariant myPropertyValue(const QString& key) const
+    {
+    return this->propertyValue(key);
+    }
+};
+
+} // end of anonymous namespace
+
 //-----------------------------------------------------------------------------
 void ctkSettingsPanelTester::testEmptyQStringList()
 {
@@ -230,12 +252,21 @@ void ctkSettingsPanelTester::testEmptyQStringList()
   }
 
   // Regression: Reading empty QStringList property from settings should be handled properly
+  // See issue #646
 
-  ctkSettingsPanel settingsPanel;
+  ctkSettingsPanelForTest settingsPanel;
   ctkSettingsPanelTest2Helper * list = new ctkSettingsPanelTest2Helper(&settingsPanel);
   settingsPanel.registerProperty("list", list, "list", SIGNAL(listChanged()));
   QSettings settings2(QSettings::IniFormat, QSettings::UserScope, "Common ToolKit", "CTK");
   settingsPanel.setSettings(&settings2);
+
+  QVariant listVal = settings2.value("list");
+  QCOMPARE(listVal.isValid(), false); // See issue #646
+  QCOMPARE(listVal, QVariant()); // See issue #646
+  QCOMPARE(listVal.toStringList(), QStringList());
+  QCOMPARE(settingsPanel.myPreviousPropertyValue("list").toStringList(), QStringList());
+  QCOMPARE(settingsPanel.myDefaultPropertyValue("list").toStringList(), QStringList());
+  QCOMPARE(settingsPanel.myPropertyValue("list").toStringList(), QStringList());
 }
 
 //-----------------------------------------------------------------------------

Libs/Widgets/Testing/Cpp/ctkSettingsPanelTest1.cpp

@@ -23,6 +23,7 @@
 #include <QCheckBox>
 #include <QLineEdit>
 #include <QSettings>
+#include <QSignalSpy>
 #include <QTimer>
 #include <QVariant>
 
@@ -30,6 +31,7 @@
 #include "ctkBooleanMapper.h"
 #include "ctkCoreTestingMacros.h"
 #include "ctkSettingsPanel.h"
+#include "ctkSettingsPanelTest2Helper.h"
 
 // STD includes
 #include <cstdlib>
@@ -57,31 +59,59 @@ public:
 
 } // end of anonymous namespace
 
+
+//-----------------------------------------------------------------------------
+int TestBoolean(ctkSettingsPanelForTest& settingsPanel);
+int TestString(ctkSettingsPanelForTest& settingsPanel);
+int TestBooleanMapper(ctkSettingsPanelForTest& settingsPanel);
+int TestStringList(ctkSettingsPanelForTest& settingsPanel);
+
 //-----------------------------------------------------------------------------
 int ctkSettingsPanelTest1(int argc, char * argv [] )
 {
   QApplication app(argc, argv);
-  
-  QSettings settings(QSettings::IniFormat, QSettings::UserScope, "Common ToolKit", "CTK");
-  settings.clear();
+
+  {
+    QSettings settings(QSettings::IniFormat, QSettings::UserScope, "Common ToolKit", "CTK");
+    settings.clear();
+  }
 
   ctkSettingsPanelForTest settingsPanel;
+
+  CHECK_EXIT_SUCCESS(TestBoolean(settingsPanel));
+  CHECK_EXIT_SUCCESS(TestString(settingsPanel));
+  CHECK_EXIT_SUCCESS(TestBooleanMapper(settingsPanel));
+  CHECK_EXIT_SUCCESS(TestStringList(settingsPanel));
+
+  settingsPanel.show();
+
+  if (argc < 2 || QString(argv[1]) != "-I" )
+    {
+    QTimer::singleShot(200, &app, SLOT(quit()));
+    }
+
+  return app.exec();
+}
+
+//-----------------------------------------------------------------------------
+int TestBoolean(ctkSettingsPanelForTest& settingsPanel)
+{
+  QSettings settings(QSettings::IniFormat, QSettings::UserScope, "Common ToolKit", "CTK");
   settingsPanel.setSettings(&settings);
 
-  //
-  // QCheckBox
-  //
   QCheckBox* box = new QCheckBox(&settingsPanel);
 
   settingsPanel.registerProperty("key 1", box, "checked",
                                   SIGNAL(toggled(bool)));
-  
+
   // Check settings value after a property is registered
   QVariant boxVal = settings.value("key 1");
   CHECK_BOOL(boxVal.isValid(), true);
+  CHECK_QVARIANT(boxVal, QVariant(false));
   CHECK_BOOL(boxVal.toBool(), false);
   CHECK_BOOL(settingsPanel.myPreviousPropertyValue("key 1").toBool(), false);
   CHECK_BOOL(settingsPanel.myPropertyValue("key 1").toBool(), false);
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
 
   // Update value using the object/widget API
   box->setChecked(true);
@@ -89,13 +119,46 @@ int ctkSettingsPanelTest1(int argc, char * argv [] )
   // Check settings value after it has been updated using object/widget API
   boxVal = settings.value("key 1");
   CHECK_BOOL(boxVal.isValid(), true);
+  CHECK_QVARIANT(boxVal, QVariant(true));
   CHECK_BOOL(boxVal.toBool(), true);
   CHECK_BOOL(settingsPanel.myPreviousPropertyValue("key 1").toBool(), false);
   CHECK_BOOL(settingsPanel.myPropertyValue("key 1").toBool(), true);
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList() << "key 1");
+
+  // Check settings value after applySettings() has been called
+  settingsPanel.applySettings();
+  boxVal = settings.value("key 1");
+  CHECK_BOOL(boxVal.isValid(), true);
+  CHECK_QVARIANT(boxVal, QVariant(true));
+  CHECK_BOOL(boxVal.toBool(), true);
+  CHECK_BOOL(settingsPanel.myPreviousPropertyValue("key 1").toBool(), true);
+  CHECK_BOOL(settingsPanel.myDefaultPropertyValue("key 1").toBool(), false);
+  CHECK_BOOL(settingsPanel.myPropertyValue("key 1").toBool(), true);
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
+
+  // Check settings value after saving settings to disk
+  settings.sync();
+  QSettings settings2(QSettings::IniFormat, QSettings::UserScope, "Common ToolKit", "CTK");
+  settingsPanel.setSettings(&settings2);
+
+  boxVal = settings2.value("key 1");
+  CHECK_BOOL(boxVal.isValid(), true);
+  CHECK_QVARIANT(boxVal, QVariant(true));
+  CHECK_BOOL(boxVal.toBool(), true);
+  CHECK_BOOL(settingsPanel.myPreviousPropertyValue("key 1").toBool(), true);
+  CHECK_BOOL(settingsPanel.myDefaultPropertyValue("key 1").toBool(), false);
+  CHECK_BOOL(settingsPanel.myPropertyValue("key 1").toBool(), true);
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
+
+  return EXIT_SUCCESS;
+}
+
+//-----------------------------------------------------------------------------
+int TestString(ctkSettingsPanelForTest& settingsPanel)
+{
+  QSettings settings(QSettings::IniFormat, QSettings::UserScope, "Common ToolKit", "CTK");
+  settingsPanel.setSettings(&settings);
 
-  //
-  // QLineEdit
-  //
   QLineEdit* lineEdit = new QLineEdit("default", &settingsPanel);
   settingsPanel.registerProperty("key 2", lineEdit, "text",
                                   SIGNAL(textChanged(QString)));
@@ -103,10 +166,12 @@ int ctkSettingsPanelTest1(int argc, char * argv [] )
   // Check value after a property is registered
   QVariant lineEditVal = settings.value("key 2");
   CHECK_BOOL(lineEditVal.isValid(), true);
+  CHECK_QVARIANT(lineEditVal, QVariant("default"));
   CHECK_QSTRING(lineEditVal.toString(), QString("default"));
   CHECK_QSTRING(settingsPanel.myPreviousPropertyValue("key 2").toString(), QString("default"));
   CHECK_QSTRING(settingsPanel.myDefaultPropertyValue("key 2").toString(), QString("default"));
   CHECK_QSTRING(settingsPanel.myPropertyValue("key 2").toString(), QString("default"));
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
 
   // Update value using the object/widget API
   lineEdit->setText("first edit");
@@ -114,19 +179,23 @@ int ctkSettingsPanelTest1(int argc, char * argv [] )
   // Check settings value after it has been updated using object/widget API
   lineEditVal = settings.value("key 2");
   CHECK_BOOL(lineEditVal.isValid(), true);
+  CHECK_QVARIANT(lineEditVal, QVariant("first edit"));
   CHECK_QSTRING(lineEditVal.toString(), QString("first edit"));
   CHECK_QSTRING(settingsPanel.myPreviousPropertyValue("key 2").toString(), QString("default"));
   CHECK_QSTRING(settingsPanel.myDefaultPropertyValue("key 2").toString(), QString("default"));
   CHECK_QSTRING(settingsPanel.myPropertyValue("key 2").toString(), QString("first edit"));
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList() << "key 2");
 
   // Check settings value after applySettings() has been called
   settingsPanel.applySettings();
   lineEditVal = settings.value("key 2");
   CHECK_BOOL(lineEditVal.isValid(), true);
+  CHECK_QVARIANT(lineEditVal, QVariant("first edit"));
   CHECK_QSTRING(lineEditVal.toString(), QString("first edit"));
   CHECK_QSTRING(settingsPanel.myPreviousPropertyValue("key 2").toString(), QString("first edit"));
   CHECK_QSTRING(settingsPanel.myDefaultPropertyValue("key 2").toString(), QString("default"));
   CHECK_QSTRING(settingsPanel.myPropertyValue("key 2").toString(), QString("first edit"));
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
 
   // Update value using the object/widget API
   lineEdit->setText("second edit");
@@ -134,35 +203,62 @@ int ctkSettingsPanelTest1(int argc, char * argv [] )
   // Check settings value after it has been updated using object/widget API
   lineEditVal = settings.value("key 2");
   CHECK_BOOL(lineEditVal.isValid(), true);
+  CHECK_QVARIANT(lineEditVal, QVariant("second edit"));
   CHECK_QSTRING(lineEditVal.toString(), QString("second edit"));
   CHECK_QSTRING(settingsPanel.myPreviousPropertyValue("key 2").toString(), QString("first edit"));
   CHECK_QSTRING(settingsPanel.myDefaultPropertyValue("key 2").toString(), QString("default"));
   CHECK_QSTRING(settingsPanel.myPropertyValue("key 2").toString(), QString("second edit"));
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList() << "key 2");
 
   // Check settings value after applySettings() has been called
   settingsPanel.applySettings();
   lineEditVal = settings.value("key 2");
   CHECK_BOOL(lineEditVal.isValid(), true);
+  CHECK_QVARIANT(lineEditVal, QVariant("second edit"));
+  CHECK_QSTRING(lineEditVal.toString(), QString("second edit"));
+  CHECK_QSTRING(settingsPanel.myPreviousPropertyValue("key 2").toString(), QString("second edit"));
+  CHECK_QSTRING(settingsPanel.myDefaultPropertyValue("key 2").toString(), QString("default"));
+  CHECK_QSTRING(settingsPanel.myPropertyValue("key 2").toString(), QString("second edit"));
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
+
+  // Check settings value after saving settings to disk
+  settings.sync();
+  QSettings settings2(QSettings::IniFormat, QSettings::UserScope, "Common ToolKit", "CTK");
+  settingsPanel.setSettings(&settings2);
+
+  lineEditVal = settings2.value("key 2");
+  CHECK_BOOL(lineEditVal.isValid(), true);
+  CHECK_QVARIANT(lineEditVal, QVariant("second edit"));
   CHECK_QSTRING(lineEditVal.toString(), QString("second edit"));
   CHECK_QSTRING(settingsPanel.myPreviousPropertyValue("key 2").toString(), QString("second edit"));
   CHECK_QSTRING(settingsPanel.myDefaultPropertyValue("key 2").toString(), QString("default"));
   CHECK_QSTRING(settingsPanel.myPropertyValue("key 2").toString(), QString("second edit"));
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
+
+  return EXIT_SUCCESS;
+}
+
+//-----------------------------------------------------------------------------
+int TestBooleanMapper(ctkSettingsPanelForTest& settingsPanel)
+{
+  QSettings settings(QSettings::IniFormat, QSettings::UserScope, "Common ToolKit", "CTK");
+  settingsPanel.setSettings(&settings);
+
+  QCheckBox* box = new QCheckBox(&settingsPanel);
 
-  //
-  // QCheckBox + ctkBooleanMapper
-  //
-  box->setChecked(false);
   settingsPanel.registerProperty("key complement",
                                  new ctkBooleanMapper(box, "checked", SIGNAL(toggled(bool))),
                                  "complement",
                                   SIGNAL(complementChanged(bool)));
 
   // Check settings value after a property is registered
-  boxVal = settings.value("key complement");
+  QVariant boxVal = settings.value("key complement");
   CHECK_BOOL(boxVal.isValid(), true);
+  CHECK_QVARIANT(boxVal, QVariant(true));
   CHECK_BOOL(boxVal.toBool(), true);
   CHECK_BOOL(settingsPanel.myPreviousPropertyValue("key complement").toBool(), true);
   CHECK_BOOL(settingsPanel.myPropertyValue("key complement").toBool(), true);
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
 
   // Update value using the object/widget API
   box->setChecked(true);
@@ -170,17 +266,178 @@ int ctkSettingsPanelTest1(int argc, char * argv [] )
   // Check settings value after it has been updated using object/widget API
   boxVal = settings.value("key complement");
   CHECK_BOOL(boxVal.isValid(), true);
+  CHECK_QVARIANT(boxVal, QVariant(false));
   CHECK_BOOL(boxVal.toBool(), false);
   CHECK_BOOL(settingsPanel.myPreviousPropertyValue("key complement").toBool(), true);
   CHECK_BOOL(settingsPanel.myPropertyValue("key complement").toBool(), false);
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList() << "key complement");
 
-  settingsPanel.show();
+  // Check settings value after applySettings() has been called
+  settingsPanel.applySettings();
+  boxVal = settings.value("key complement");
+  CHECK_BOOL(boxVal.isValid(), true);
+  CHECK_QVARIANT(boxVal, QVariant(false));
+  CHECK_BOOL(boxVal.toBool(), false);
+  CHECK_BOOL(settingsPanel.myPreviousPropertyValue("key complement").toBool(), false);
+  CHECK_BOOL(settingsPanel.myDefaultPropertyValue("key complement").toBool(), true);
+  CHECK_BOOL(settingsPanel.myPropertyValue("key complement").toBool(), false);
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
 
-  if (argc < 2 || QString(argv[1]) != "-I" )
-    {
-    QTimer::singleShot(200, &app, SLOT(quit()));
-    }
+  // Check settings value after saving settings to disk
+  settings.sync();
+  QSettings settings2(QSettings::IniFormat, QSettings::UserScope, "Common ToolKit", "CTK");
+  settingsPanel.setSettings(&settings2);
 
-  return app.exec();
+  boxVal = settings2.value("key complement");
+  CHECK_BOOL(boxVal.isValid(), true);
+  CHECK_QVARIANT(boxVal, QVariant(false));
+  CHECK_BOOL(boxVal.toBool(), false);
+  CHECK_BOOL(settingsPanel.myPreviousPropertyValue("key complement").toBool(), false);
+  CHECK_BOOL(settingsPanel.myDefaultPropertyValue("key complement").toBool(), true);
+  CHECK_BOOL(settingsPanel.myPropertyValue("key complement").toBool(), false);
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
+
+  return EXIT_SUCCESS;
+}
+
+//-----------------------------------------------------------------------------
+int TestStringList(ctkSettingsPanelForTest& settingsPanel)
+{
+  qRegisterMetaType<QVariant>("QVariant");
+  QSignalSpy spy(&settingsPanel, SIGNAL(settingChanged(QString,QVariant)));
+
+  QSettings settings(QSettings::IniFormat, QSettings::UserScope, "Common ToolKit", "CTK");
+  settingsPanel.setSettings(&settings);
+
+  ctkSettingsPanelTest2Helper* list = new ctkSettingsPanelTest2Helper(&settingsPanel);
+  settingsPanel.registerProperty("key list", list, "list",
+                                 SIGNAL(listChanged()));
+
+  // Check value after a property is registered
+  QVariant listVal = settings.value("key list");
+  CHECK_BOOL(listVal.isValid(), true);
+  CHECK_QVARIANT(listVal, QVariant(QStringList()));
+  CHECK_QSTRINGLIST(listVal.toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPreviousPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myDefaultPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
+  CHECK_INT(spy.count(), 0);
+
+  // Reset spy
+  spy.clear();
+
+  // Update value using the object/widget API: Add one item
+  list->setList(QStringList() << "first item");
+
+  // Check settings value after it has been updated using object/widget API
+  listVal = settings.value("key list");
+  CHECK_BOOL(listVal.isValid(), true);
+  CHECK_QVARIANT(listVal, QVariant(QStringList() << "first item"));
+  CHECK_QSTRINGLIST(listVal.toStringList(), QStringList() << "first item");
+  CHECK_QSTRINGLIST(settingsPanel.myPreviousPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myDefaultPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPropertyValue("key list").toStringList(), QStringList() << "first item");
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList() << "key list");
+  CHECK_INT(spy.count(), 1);
+
+  // Reset spy
+  spy.clear();
+
+  // Check settings value after applySettings() has been called
+  settingsPanel.applySettings();
+  listVal = settings.value("key list");
+  CHECK_BOOL(listVal.isValid(), true);
+  CHECK_QVARIANT(listVal, QVariant(QStringList() << "first item"));
+  CHECK_QSTRINGLIST(listVal.toStringList(), QStringList() << "first item");
+  CHECK_QSTRINGLIST(settingsPanel.myPreviousPropertyValue("key list").toStringList(), QStringList() << "first item");
+  CHECK_QSTRINGLIST(settingsPanel.myDefaultPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPropertyValue("key list").toStringList(), QStringList() << "first item");
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
+  CHECK_INT(spy.count(), 0);
+
+  // Reset spy
+  spy.clear();
+
+  // Update value using the object/widget API: Add one other item
+  list->setList(QStringList() << "first item" << "second item");
+
+  // Check settings value after it has been updated using object/widget API
+  listVal = settings.value("key list");
+  CHECK_BOOL(listVal.isValid(), true);
+  CHECK_QVARIANT(listVal, QVariant(QStringList() << "first item" << "second item"));
+  CHECK_QSTRINGLIST(listVal.toStringList(), QStringList() << "first item" << "second item");
+  CHECK_QSTRINGLIST(settingsPanel.myPreviousPropertyValue("key list").toStringList(), QStringList() << "first item");
+  CHECK_QSTRINGLIST(settingsPanel.myDefaultPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPropertyValue("key list").toStringList(), QStringList() << "first item" << "second item");
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList() << "key list");
+  CHECK_INT(spy.count(), 1);
+
+  // Reset spy
+  spy.clear();
+
+  // Check settings value after applySettings() has been called
+  settingsPanel.applySettings();
+  listVal = settings.value("key list");
+  CHECK_BOOL(listVal.isValid(), true);
+  CHECK_QVARIANT(listVal, QVariant(QStringList() << "first item" << "second item"));
+  CHECK_QSTRINGLIST(listVal.toStringList(), QStringList() << "first item" << "second item");
+  CHECK_QSTRINGLIST(settingsPanel.myPreviousPropertyValue("key list").toStringList(), QStringList() << "first item" << "second item");
+  CHECK_QSTRINGLIST(settingsPanel.myDefaultPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPropertyValue("key list").toStringList(), QStringList() << "first item" << "second item");
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
+  CHECK_INT(spy.count(), 0);
+
+  // Reset spy
+  spy.clear();
+
+  // Update value using the object/widget API: Remove items
+  list->setList(QStringList());
+
+  // Check settings value after it has been updated using object/widget API
+  listVal = settings.value("key list");
+  CHECK_BOOL(listVal.isValid(), true);
+  CHECK_QVARIANT(listVal, QVariant(QStringList()));
+  CHECK_QSTRINGLIST(listVal.toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPreviousPropertyValue("key list").toStringList(), QStringList() << "first item" << "second item");
+  CHECK_QSTRINGLIST(settingsPanel.myDefaultPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList() << "key list");
+  CHECK_INT(spy.count(), 1);
+
+  // Reset spy
+  spy.clear();
+
+  // Check settings value after applySettings() has been called
+  settingsPanel.applySettings();
+  listVal = settings.value("key list");
+  CHECK_BOOL(listVal.isValid(), true);
+  CHECK_QVARIANT(listVal, QVariant(QStringList()));
+  CHECK_QSTRINGLIST(listVal.toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPreviousPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myDefaultPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList());
+  CHECK_INT(spy.count(), 0);
+
+  // Reset spy
+  spy.clear();
+
+  // Check settings value after saving settings to disk
+  settings.sync();
+  QSettings settings2(QSettings::IniFormat, QSettings::UserScope, "Common ToolKit", "CTK");
+  settingsPanel.setSettings(&settings2);
+
+  listVal = settings2.value("key list");
+  CHECK_BOOL(listVal.isValid(), false); // Issue #646
+  CHECK_QVARIANT(listVal, QVariant()); // Issue #646
+  CHECK_QSTRINGLIST(listVal.toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPreviousPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myDefaultPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.myPropertyValue("key list").toStringList(), QStringList());
+  CHECK_QSTRINGLIST(settingsPanel.changedSettings(), QStringList()); // Issue #646
+  CHECK_INT(spy.count(), 0);
+
+  return EXIT_SUCCESS;
 }
 

Libs/Widgets/ctkSettingsPanel.cpp

@@ -36,10 +36,10 @@ namespace
 // --------------------------------------------------------------------------
 struct PropertyType
 {
+  typedef PropertyType Self;
   PropertyType();
   QObject* Object;
   QString  Property;
-  QVariant PreviousValue;
   QVariant DefaultValue;
   QString  Label;
   QSettings* Settings;
@@ -48,7 +48,17 @@ struct PropertyType
   QVariant value()const;
   bool setValue(const QVariant& value);
 
+  QVariant previousValue()const;
+  void setPreviousValue(const QVariant& value);
+
   QMetaProperty metaProperty();
+
+  /// Workaround https://bugreports.qt.io/browse/QTBUG-19823
+  static QVariant fixEmptyStringListVariant(const QVariant& value, const char* _typename);
+
+private:
+
+  QVariant PreviousValue;
 };
 
 // --------------------------------------------------------------------------
@@ -78,11 +88,8 @@ bool PropertyType::setValue(const QVariant& val)
     return false;
     }
   QVariant value(val);
-  // HACK - See http://bugreports.qt.nokia.com/browse/QTBUG-19823
-  if (qstrcmp(this->metaProperty().typeName(), "QStringList") == 0 && !value.isValid())
-    {
-    value = QVariant(QStringList());
-    }
+  value = Self::fixEmptyStringListVariant(value,
+                                          this->metaProperty().typeName());
   bool success = true;
   if (this->Object->property(this->Property.toLatin1()) != value)
     {
@@ -93,6 +100,32 @@ bool PropertyType::setValue(const QVariant& val)
 }
 
 // --------------------------------------------------------------------------
+QVariant PropertyType::previousValue()const
+{
+  return this->PreviousValue;
+}
+
+// --------------------------------------------------------------------------
+void PropertyType::setPreviousValue(const QVariant& val)
+{
+  this->PreviousValue =
+      Self::fixEmptyStringListVariant(val, this->metaProperty().typeName());
+}
+
+// --------------------------------------------------------------------------
+QVariant PropertyType::fixEmptyStringListVariant(const QVariant& value,
+                                                 const char* _typename)
+{
+  QVariant fixedValue = value;
+  // HACK - See https://bugreports.qt.io/browse/QTBUG-19823
+  if (qstrcmp(_typename, "QStringList") == 0 && !value.isValid())
+    {
+    fixedValue = QVariant(QStringList());
+    }
+  return fixedValue;
+}
+
+// --------------------------------------------------------------------------
 QMetaProperty PropertyType::metaProperty()
 {
   Q_ASSERT(this->Object);
@@ -211,7 +244,7 @@ void ctkSettingsPanel::reloadSettings()
       PropertyType& prop = d->Properties[key];
       // Update object registered using registerProperty()
       prop.setValue(value);
-      prop.PreviousValue = value;
+      prop.setPreviousValue(value);
       }
     else
       {
@@ -241,6 +274,8 @@ void ctkSettingsPanel::setSetting(const QString& key, const QVariant& newVal)
     return;
     }
   QVariant oldVal = settings->value(key);
+  oldVal = PropertyType::fixEmptyStringListVariant(
+        oldVal, d->Properties[key].metaProperty().typeName());
   settings->setValue(key, newVal);
   d->Properties[key].setValue(newVal);
   if (settings->status() != QSettings::NoError)
@@ -268,7 +303,8 @@ void ctkSettingsPanel::registerProperty(const QString& key,
   PropertyType prop;
   prop.Object = object;
   prop.Property = property;
-  prop.DefaultValue = prop.PreviousValue = prop.value();
+  prop.setPreviousValue(prop.value());
+  prop.DefaultValue = prop.previousValue();
   prop.Label = label;
   prop.Options = options;
   if (d->Settings != settings)
@@ -281,7 +317,7 @@ void ctkSettingsPanel::registerProperty(const QString& key,
     {
     QVariant val = propSettings->value(key);
     prop.setValue(val);
-    prop.PreviousValue = val;
+    prop.setPreviousValue(val);
     }
     
   d->Properties[key] = prop;
@@ -329,7 +365,7 @@ QVariant ctkSettingsPanel::previousPropertyValue(const QString& key) const
     {
     return QVariant();
     }
-  return d->Properties.value(key).PreviousValue;
+  return d->Properties.value(key).previousValue();
 }
 
 // --------------------------------------------------------------------------
@@ -351,7 +387,7 @@ QStringList ctkSettingsPanel::changedSettings()const
   foreach(const QString& key, d->Properties.keys())
     {
     const PropertyType& prop = d->Properties[key];
-    if (prop.PreviousValue != prop.value())
+    if (prop.previousValue() != prop.value())
       {
       settingsKeys << key;
       }
@@ -381,7 +417,7 @@ void ctkSettingsPanel::applySettings()
   foreach(const QString& key, d->Properties.keys())
     {
     PropertyType& prop = d->Properties[key];
-    prop.PreviousValue = prop.value();
+    prop.setPreviousValue(prop.value());
     }
 }
 
@@ -391,7 +427,7 @@ void ctkSettingsPanel::resetSettings()
   Q_D(ctkSettingsPanel);
   foreach(const QString& key, d->Properties.keys())
     {
-    this->setSetting(key, d->Properties[key].PreviousValue);
+    this->setSetting(key, d->Properties[key].previousValue());
     }
 }