Skip to content
Browse files

[bugfix][attrtable] Convert comma to dot for floating point input

This fixes an unreported bug that without detecting an
invalid input when using a comma as a decimal separator
silently converts the entered value to NULL.

Since locale support in QGIS is in its early stages
we convert commas to dots within the validator,
this is common practice in almost all web applications
where you can enter a comma instead of a dot and
the conversion appears while you digit.

This comes with brand new tests for QgsFieldValidator.

Bonus: small fix in sipify.
  • Loading branch information
elpaso committed Jan 31, 2018
1 parent cbd0ece commit cc1625c83ec50c698a6cf79e7da617efd164b287
@@ -19,7 +19,7 @@ class QgsFieldValidator : QValidator
QgsFieldValidator( QObject *parent, const QgsField &field, const QString &defaultValue, const QString &dateFormat = "yyyy-MM-dd" );

virtual State validate( QString &s /Constrained/, int &i /In,Out/ ) const;
virtual State validate( QString &s /Constrained,In,Out/, int &i /In,Out/ ) const;

virtual void fixup( QString &s /Constrained/ ) const;

@@ -84,7 +84,6 @@ QgsFieldValidator::QgsFieldValidator( QObject *parent, const QgsField &field, co
mValidator = nullptr;

QgsSettings settings;
mNullValue = QgsApplication::nullRepresentation();

@@ -113,6 +112,12 @@ QValidator::State QgsFieldValidator::validate( QString &s, int &i ) const
// delegate to the child validator if any
if ( mValidator )
// force to use the '.' as a decimal point or in case we are using QDoubleValidator
// we can get a valid number with a comma depending on current locale
// ... but this will fail subsequently when converted from string to double and
// becomes a NULL!
if ( mField.type() == QVariant::Double )
s = s.replace( ',', '.' );
QValidator::State result = mValidator->validate( s, i );
return result;
@@ -38,7 +38,7 @@ class GUI_EXPORT QgsFieldValidator : public QValidator
QgsFieldValidator( QObject *parent, const QgsField &field, const QString &defaultValue, const QString &dateFormat = "yyyy-MM-dd" );
~QgsFieldValidator() override;

State validate( QString &s SIP_CONSTRAINED, int &i SIP_INOUT ) const override;
State validate( QString &s SIP_CONSTRAINED SIP_INOUT, int &i SIP_INOUT ) const override;
void fixup( QString &s SIP_CONSTRAINED ) const override;

QString dateFormat() const { return mDateFormat; }
@@ -196,6 +196,7 @@ ADD_PYTHON_TEST(PyQgsAuthManagerProxy

@@ -0,0 +1,105 @@
# -*- coding: utf-8 -*-
"""QGIS Unit tests for QgsFieldValidator.
.. note:: This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.
__author__ = 'Alessandro Pasotti'
__date__ = '31/01/2018'
__copyright__ = 'Copyright 2018, The QGIS Project'
# This will get replaced with a git SHA1 when you do a git archive
__revision__ = '$Format:%H$'

import qgis # NOQA

from qgis.PyQt.QtCore import QVariant
from qgis.PyQt.QtGui import QValidator
from qgis.core import QgsVectorLayer
from qgis.gui import QgsFieldValidator
from qgis.testing import start_app, unittest
from utilities import unitTestDataPath

TEST_DATA_DIR = unitTestDataPath()


class TestQgsFieldValidator(unittest.TestCase):

def setUp(self):
"""Run before each test."""
testPath = TEST_DATA_DIR + '/' + 'bug_17878.gpkg|layername=bug_17878'
self.vl = QgsVectorLayer(testPath, "test_data", "ogr")
assert self.vl.isValid()

def tearDown(self):
"""Run after each test."""

def test_validator(self):
# Test the double
Expected results from validate
QValidator::Invalid 0 The string is clearly invalid.
QValidator::Intermediate 1 The string is a plausible intermediate value.
QValidator::Acceptable 2 The string is acceptable as a final result; i.e. it is valid.

double_field = self.vl.fields()[self.vl.fields().indexFromName('double_field')]
self.assertEqual(double_field.precision(), 0) # this is what the provider reports :(
self.assertEqual(double_field.length(), 0) # not set
self.assertEqual(double_field.type(), QVariant.Double)

validator = QgsFieldValidator(None, double_field, '0.0', '')

def _test(value, expected):
ret = validator.validate(value, 0)
self.assertEqual(ret[0], expected, value)
if value:
self.assertEqual(validator.validate('-' + value, 0)[0], expected, '-' + value)
# Check the decimal comma separator has been properly transformed
if expected != QValidator.Invalid:
self.assertEqual(ret[1], value.replace(',', '.'))

# Valid
_test('0.1234', QValidator.Acceptable)
_test('0,1234', QValidator.Acceptable)
_test('12345.1234e+123', QValidator.Acceptable)
_test('12345.1234e-123', QValidator.Acceptable)
_test('12345,1234e+123', QValidator.Acceptable)
_test('12345,1234e-123', QValidator.Acceptable)
_test('', QValidator.Acceptable)

# Out of range
_test('12345.1234e+823', QValidator.Intermediate)
_test('12345.1234e-823', QValidator.Intermediate)
_test('12345,1234e+823', QValidator.Intermediate)
_test('12345,1234e-823', QValidator.Intermediate)

# Invalid
_test('12345-1234', QValidator.Invalid)
_test('onetwothree', QValidator.Invalid)

int_field = self.vl.fields()[self.vl.fields().indexFromName('int_field')]
self.assertEqual(int_field.precision(), 0) # this is what the provider reports :(
self.assertEqual(int_field.length(), 0) # not set
self.assertEqual(int_field.type(), QVariant.Int)

validator = QgsFieldValidator(None, int_field, '0', '')

# Valid
_test('0', QValidator.Acceptable)
_test('1234', QValidator.Acceptable)
_test('', QValidator.Acceptable)

# Invalid
_test('12345-1234', QValidator.Invalid)
_test('12345.1234', QValidator.Invalid)
_test('onetwothree', QValidator.Invalid)

if __name__ == '__main__':
Binary file not shown.

0 comments on commit cc1625c

Please sign in to comment.
You can’t perform that action at this time.