View Issue Details

This bug affects 1 person(s).
 6
IDProjectCategoryView StatusLast Update
05792Bug reportsSurvey takingpublic2012-03-14 21:08
ReporterDenisChenu Assigned ToTMSWhite  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version1.92RC3 
Target Version1.92RC5Fixed in Version1.92RC5 
Summary05792: Problem with comma separator for EM_validation
Description

If you choose comma separator in survey option, the all EM calculation with numer are bad.

Steps To Reproduce

Add a survey with Comma seperator
Add a numeric question with some minimal value,

Test the survey with a float value ( 1,7 for example)
Bad

Same think for Multi numeric and maybe other

Additional Information

In 1.91,

We use this javascript function:
.split('{$sSeperator}').join('.') in php

There are 2 solution:
Get a var in javascript for seperator, and in each test do this function
In inline script (done in php) do this function before launch EM validation function.

Attention : need a .join('{$sSeperator}').split('.') to show the result ( or after put the value).

TagsNo tags attached.
Attached Files
Bug heat6
Complete LimeSurvey version number (& build)12425
I will donate to the project if issue is resolvedNo
Browsernot relevant
Database type & versionnot relevant
Server OS (if known)not relevant
Webserver software & version (if known)not relevant
PHP Versionnot relevant

Relationships

related to 05524 closedTMSWhite matrix (text) + additionl option only numbers not working correct 
related to 05490 closedmdekker Spss export error in decimal values 

Users monitoring this issue

There are no users monitoring this issue.

Activities

TMSWhite

TMSWhite

2012-02-09 17:03

reporter   ~17334

OK, this is going to be tricky.

For all of the advanced question options that EM now controls (e.g. min/max value, min/max sum), there is no way to have them support comma as the radix separator (decimal point). Comma is a reserved character in programming languages (e.g. to generate lists), so EM would have no way of telling whether you were trying to enter a floating point number or a comma needed for a function.

For example, say your minimum value was (using a period as the separator), {min(0.5,q1)}. If you tried it with a comma separator, it would be this: {min(0,5,q1))} - which would be treated as using 0 as the minimum, not one half.

Sorry, but there is no way around this, so, users will need to use a decimal point as separator when entering min/max values and sums in advanced question options.

However, it should be possible to:
(1) Pre-process all entered numbers input with comma to convert to decimal first (prior to doing validations - so that the validations pass), and
(2) Post-process numerical values for text display to convert from the period to comma separator (e.g. for the "Please enter a number between X and Y", and when re-displaying the value in a numerical input field).

Whether we should do that via a str_replace() function, or via a masking function (jQuery masks - http://archive.plugins.jquery.com/project/meioMask) is another matter.

In all of this, I presume that the database inserts need to use the period separator, so the internal representation would always be with a period.

DenisChenu

DenisChenu

2012-02-09 17:45

developer   ~17337

Tom,

You can have a look at 1.91 :
.split('{$sSeperator}').join('.')
And
.split('.').join('{$sSeperator}')

In 1.92, the SESSION variable are put with dot too.
Maybe some problem with {INSERTANS:...} but it's not important.

DenisChenu

DenisChenu

2012-02-09 18:10

developer   ~17338

Tom,

If you're OK, i get the bug, but when it's on LM i think it's important to speak with you :)

Denis

TMSWhite

TMSWhite

2012-02-10 18:02

reporter   ~17362

Attached is a survey that tests all of the question types the support numeric input. It is currently set to use a period as the radix separator. Switching to using comma will let us test accuracy of any fixes done here.

TMSWhite

TMSWhite

2012-02-10 18:55

reporter   ~17366

This fix will also require better validating numbers. Currently, LS uses onkeypress="return goodchars('-0123456789,')". However, you can enter number like '-12-3.4.56-7-' and still have that pass the first validation step.

When only numbers are allowed, we should use EM and regexMatch('/^-?[0-9].[0-9]$/') to validate numbers (and the comparable one for countries that use a comma as the radix separator.

This is too much work for the 1.92 RC4 release.

DenisChenu

DenisChenu

2012-02-10 19:13

developer   ~17367

Hello Tom,

Sur for number like '-12-3.4.56-7-' :)

For testing if it's an numeric, no regexp:
return parseFloat(data)==data;
( but after replace the ,).

( maybe done at .blur() ?)

There are other possibility for numeric :
http://stackoverflow.com/questions/18082/validate-numbers-in-javascript-isnumeric
But with numeric question of LS, i think parsefloat is good.

TMSWhite

TMSWhite

2012-02-10 19:23

reporter   ~17368

True, parseFloat() would work adequately for testing whether numeric, so regex would not be needed for this.

However, would it make more sense to forcibly change the input values, or let people enter what they want, and then use CSS styling to show if the entered value was not truly numeric?

DenisChenu

DenisChenu

2012-02-11 00:37

developer   ~17370

<q>However, would it make more sense to forcibly change the input values, or let people enter what they want, and then use CSS styling to show if the entered value was not truly numeric? </q> Don't know.

I already use some script to show number like this : 10 000,00 ( in english 10,000.00), calculate some other input, show it with good seperetor and put it in good value in database.

To see, but not for 1.92RC4.

TMSWhite

TMSWhite

2012-02-23 08:15

reporter   ~17575

Note to self on strategy:

(1) replace noop_checkconditions() with fixnum_checkconditions() - use it only for input fields that only accept numbers

(2) have fixnum_checkconditions()
(a) call join('.',split(',',value)), if needed
(b) call parseFloat() to determine whether a true number
(c) ensure that the $('#java'+sgqa) field uses a decimal point as the radix separator
(d) update the input field $('#answer'+sgqa) with the parseFloat() value (to fix bad inputs like -1.2.3), after replacing period with comma (if needed)
(e) call checkconditions to do the proper validations

(3) Update displays of totals and remaining values to use commas if needed

(4) Add .onlynum class to inputs that only accept numbers so that LEMsetTabIndexes() can add fixnum_checkconditions() as the tab function to call for those fields

(5) Need way to display numeric values from {INSERTANS:xxx}, and for validation messages (e.g. "Sum must be between X and Y"). This only affects tailoring, and should only affect tailoring of values coming from .onlynum variables.
(a) add .onlynum attribute to LEMvarNameAttr? If so, then LEMval() could do period to comma conversion, BUT don't want to do that for math processing. So, could consider using it in EM::GetJavaScriptFunctionForReplacement() function, which sets the innerHTML. Would only want to do that if final output is a floating point number with a period as the radix separator.
(b) create LEMfixnum() function that does period to comma conversion in tailoring, if needed
(c) create LEMradix variable on each page. Have LEMfixnum() use that to determine whether to convert period to radix
(d) have GetJavaScriptFunctionForReplacement() wrap replacement in LEMfixnum() instead of htmlspecialchars; have LEMfixnum call htmlspecialchars if not a number.

(6) Create validation type for numbers? This would color-code if not numeric vs. forcing the value to be numeric - that is probably overkill.

TMSWhite

TMSWhite

2012-02-24 01:42

reporter   ~17598

Fixed with GitHub patch https://github.com/LimeSurvey/LimeSurvey/commit/1101ba6d064f7898191276dec03ce4263997c338

c_schmitz

c_schmitz

2012-02-26 14:21

administrator   ~17644

1.92RC5 release

Related Changesets

LimeSurvey: Yii 9ed3c434

2012-03-01 17:01

TMSWhite


Details Diff
Fixed issue 05792: Problem with comma separator for EM_validation
Dev Comma as radix separator now works for all question types that can require numbers, except for array multi text using the option numbers_only (which is the topic of bug http://bugs.limesurvey.org/view.php?id=5524)
Affected Issues
05792
mod - application/helpers/SurveyRuntimeHelper.php Diff File
mod - application/helpers/expressions/em_core_helper.php Diff File
mod - application/helpers/expressions/em_manager_helper.php Diff File
mod - application/helpers/qanda_helper.php Diff File
mod - scripts/admin/expressions/em_javascript.js Diff File

Issue History

Date Modified Username Field Change
2012-02-09 15:37 DenisChenu New Issue
2012-02-09 17:03 TMSWhite Note Added: 17334
2012-02-09 17:45 DenisChenu Note Added: 17337
2012-02-09 18:10 DenisChenu Note Added: 17338
2012-02-10 18:02 TMSWhite File Added: limesurvey_survey_96772.lss
2012-02-10 18:02 TMSWhite Note Added: 17362
2012-02-10 18:55 TMSWhite Note Added: 17366
2012-02-10 19:13 DenisChenu Note Added: 17367
2012-02-10 19:23 TMSWhite Note Added: 17368
2012-02-10 21:15 TMSWhite Relationship added related to 05524
2012-02-10 21:15 TMSWhite Assigned To => TMSWhite
2012-02-10 21:15 TMSWhite Status new => assigned
2012-02-11 00:37 DenisChenu Note Added: 17370
2012-02-17 18:44 TMSWhite Target Version => 1.92RC5
2012-02-23 08:15 TMSWhite Note Added: 17575
2012-02-24 01:42 TMSWhite Note Added: 17598
2012-02-24 01:42 TMSWhite Status assigned => resolved
2012-02-24 01:42 TMSWhite Fixed in Version => 1.92RC5
2012-02-24 01:42 TMSWhite Resolution open => fixed
2012-02-26 14:21 c_schmitz Note Added: 17644
2012-02-26 14:21 c_schmitz Status resolved => closed
2012-03-09 16:29 TMSWhite Relationship added related to 05490
2012-03-14 21:08 TMSWhite Changeset attached => Import 2012-03-09 13:30:34 Yii 9ed3c434