Merge pull request #7769 from elpaso/bugfix-19695-qlocale-permissive-…

…input

Bugfix 19695 qlocale permissive input

python/core/auto_generated/qgis.sip.in

@@ -162,6 +162,21 @@ numbers of digits between thousand separators
 .. versionadded:: 2.9
 %End
 
+qlonglong qgsPermissiveToLongLong( QString string, bool &ok );
+%Docstring
+Converts a string to an qlonglong in a permissive way, e.g., allowing for incorrect
+numbers of digits between thousand separators
+
+:param string: string to convert
+:param ok: will be set to true if conversion was successful
+
+:return: string converted to int if possible
+
+.. seealso:: :py:func:`permissiveToInt`
+
+.. versionadded:: 3.4
+%End
+
 bool qgsVariantLessThan( const QVariant &lhs, const QVariant &rhs );
 %Docstring
 Compares two QVariant values and returns whether the first is less than the second.

src/core/qgis.cpp

@@ -108,6 +108,13 @@ int qgsPermissiveToInt( QString string, bool &ok )
   return QLocale().toInt( string, &ok );
 }
 
+qlonglong qgsPermissiveToLongLong( QString string, bool &ok )
+{
+  //remove any thousands separators
+  string.remove( QLocale().groupSeparator() );
+  return QLocale().toLongLong( string, &ok );
+}
+
 void *qgsMalloc( size_t size )
 {
   if ( size == 0 || long( size ) < 0 )

src/core/qgis.h

@@ -409,6 +409,17 @@ CORE_EXPORT double qgsPermissiveToDouble( QString string, bool &ok );
  */
 CORE_EXPORT int qgsPermissiveToInt( QString string, bool &ok );
 
+/**
+ * Converts a string to an qlonglong in a permissive way, e.g., allowing for incorrect
+ * numbers of digits between thousand separators
+ * \param string string to convert
+ * \param ok will be set to true if conversion was successful
+ * \returns string converted to int if possible
+ * \see permissiveToInt
+ * \since QGIS 3.4
+ */
+CORE_EXPORT qlonglong qgsPermissiveToLongLong( QString string, bool &ok );
+
 /**
  * Compares two QVariant values and returns whether the first is less than the second.
  * Useful for sorting lists of variants, correctly handling sorting of the various

src/core/qgsfield.cpp

@@ -285,8 +285,8 @@ bool QgsField::convertCompatible( QVariant &v ) const
     if ( !tmp.convert( d->type ) )
     {
       // This might be a string with thousand separator: use locale to convert
-      bool ok;
-      double d = QLocale().toDouble( v.toString(), &ok );
+      bool ok = false;
+      double d = qgsPermissiveToDouble( v.toString(), ok );
       if ( ok )
       {
         v = QVariant( d );
@@ -295,7 +295,7 @@ bool QgsField::convertCompatible( QVariant &v ) const
       // For not 'dot' locales, we also want to accept '.'
       if ( QLocale().decimalPoint() != '.' )
       {
-        d = QLocale( QLocale::English ).toDouble( v.toString(), &ok );
+        d = QLocale( QLocale::C ).toDouble( v.toString(), &ok );
         if ( ok )
         {
           v = QVariant( d );
@@ -313,7 +313,7 @@ bool QgsField::convertCompatible( QVariant &v ) const
     {
       // This might be a string with thousand separator: use locale to convert
       bool ok;
-      int i = QLocale().toInt( v.toString(), &ok );
+      int i = qgsPermissiveToInt( v.toString(), ok );
       if ( ok )
       {
         v = QVariant( i );
@@ -330,7 +330,7 @@ bool QgsField::convertCompatible( QVariant &v ) const
     {
       // This might be a string with thousand separator: use locale to convert
       bool ok;
-      qlonglong l = QLocale().toLongLong( v.toString(), &ok );
+      qlonglong l = qgsPermissiveToLongLong( v.toString(), ok );
       if ( ok )
       {
         v = QVariant( l );

src/gui/editorwidgets/qgstexteditwrapper.cpp

@@ -221,6 +221,15 @@ void QgsTextEditWrapper::setWidgetValue( const QVariant &val )
     v = field().displayString( val );
   }
 
+  // For numbers, remove the group separator that might cause validation errors
+  // when the user is editing the field value.
+  // We are checking for editable layer because in the form field context we do not
+  // want to strip the separator unless the layer is editable
+  if ( layer() && layer()->isEditable() && ! QLocale().groupSeparator().isNull() && field().isNumeric() )
+  {
+    v = v.remove( QLocale().groupSeparator() );
+  }
+
   if ( mTextEdit )
   {
     if ( val != value() )

src/gui/qgsattributeform.cpp

@@ -1463,6 +1463,12 @@ void QgsAttributeForm::init()
   connect( mLayer, &QgsVectorLayer::editingStarted, this, &QgsAttributeForm::synchronizeEnabledState );
   connect( mLayer, &QgsVectorLayer::editingStopped, this, &QgsAttributeForm::synchronizeEnabledState );
 
+  // This triggers a refresh of the form widget and gives a chance to re-format the
+  // value to those widgets that have a different representation when in edit mode
+  connect( mLayer, &QgsVectorLayer::editingStarted, this, [ = ] { setFeature( feature() ); } );
+  connect( mLayer, &QgsVectorLayer::editingStopped, this,  [ = ] { setFeature( feature() ); } );
+
+
   Q_FOREACH ( QgsAttributeFormInterface *iface, mInterfaces )
   {
     iface->initForm();

src/gui/symbology/qgsgraduatedsymbolrendererwidget.cpp

@@ -1013,8 +1013,13 @@ void QgsGraduatedSymbolRendererWidget::changeRange( int rangeIdx )
 
   if ( dialog.exec() == QDialog::Accepted )
   {
-    double lowerValue = dialog.lowerValue().toDouble();
-    double upperValue = dialog.upperValue().toDouble();
+    bool ok = false;
+    double lowerValue = qgsPermissiveToDouble( dialog.lowerValue(), ok );
+    if ( ! ok )
+      lowerValue = 0.0;
+    double upperValue = qgsPermissiveToDouble( dialog.upperValue(), ok );
+    if ( ! ok )
+      upperValue = 0.0;
     mRenderer->updateRangeUpperValue( rangeIdx, upperValue );
     mRenderer->updateRangeLowerValue( rangeIdx, lowerValue );
 
@@ -1138,9 +1143,15 @@ QList<QgsSymbol *> QgsGraduatedSymbolRendererWidget::selectedSymbols()
       {
         continue;
       }
-
-      double lowerBound = list.at( 0 ).toDouble();
-      double upperBound = list.at( 2 ).toDouble();
+      // Not strictly necessary because the range should have been sanitized already
+      // after user input, but being permissive never hurts
+      bool ok = false;
+      double lowerBound = qgsPermissiveToDouble( list.at( 0 ), ok );
+      if ( ! ok )
+        lowerBound = 0.0;
+      double upperBound = qgsPermissiveToDouble( list.at( 2 ), ok );
+      if ( ! ok )
+        upperBound = 0.0;
       QgsSymbol *s = findSymbolForRange( lowerBound, upperBound, ranges );
       if ( s )
       {

tests/src/core/testqgis.cpp

@@ -38,6 +38,7 @@ class TestQgis : public QObject
 
     void permissiveToDouble();
     void permissiveToInt();
+    void permissiveToLongLong();
     void doubleToString();
     void signalBlocker();
     void qVariantCompare_data();
@@ -127,6 +128,31 @@ void TestQgis::permissiveToInt()
   QCOMPARE( result, 1000 );
 }
 
+void TestQgis::permissiveToLongLong()
+{
+  //good inputs
+  bool ok = false;
+  qlonglong result = qgsPermissiveToLongLong( QStringLiteral( "1000" ), ok );
+  QVERIFY( ok );
+  QCOMPARE( result, 1000 );
+  ok = false;
+  result = qgsPermissiveToLongLong( QStringLiteral( "1%01000" ).arg( QLocale().groupSeparator() ), ok );
+  QVERIFY( ok );
+  QCOMPARE( result, 1000 );
+
+  //bad input
+  ok = false;
+  ( void ) qgsPermissiveToLongLong( QStringLiteral( "a" ), ok );
+  QVERIFY( !ok );
+
+  //messy input (invalid thousand separator position), should still be converted
+  ok = false;
+  result = qgsPermissiveToLongLong( QStringLiteral( "10%0100" ).arg( QLocale().groupSeparator() ), ok );
+  QVERIFY( ok );
+  QCOMPARE( result, 1000 );
+
+}
+
 void TestQgis::doubleToString()
 {
   QCOMPARE( qgsDoubleToString( 5.6783212, 5 ), QString( "5.67832" ) );

tests/src/core/testqgsfield.cpp

@@ -607,6 +607,12 @@ void TestQgsField::convertCompatible()
   QCOMPARE( stringDouble.type(), QVariant::Double );
   QCOMPARE( stringDouble, QVariant( 1223456.012345 ) );
 
+  // Test that wrongly formatted decimal separator are also accepted
+  QLocale::setDefault( QLocale::German );
+  stringDouble = QVariant( "12.23.456,012345" );
+  QVERIFY( doubleField.convertCompatible( stringDouble ) );
+  QCOMPARE( stringDouble.type(), QVariant::Double );
+  QCOMPARE( stringDouble, QVariant( 1223456.012345 ) );
 
 }