View Issue Details

IDProjectCategoryView StatusLast Update
15405Bug reports[All Projects] Expression Managerpublic2019-11-19 16:16
Reporterollehar Assigned To 
PrioritynoneSeveritymajor 
Status newResolutionopen 
Product Version3.18.0 
Target Version3.18.xFixed in Version 
Summary15405: Numerical vs alphabetical order in EM, <, >, vs strcmp etc
Description

Two problems:

"2" > "18" behaves differently in JS and PHP

Second problem:

The intuition behind "A2" > "A18" is wrong

Default answer code is A1. Press plus, and it become A10, A11, etc. Condition designer uses <, >, etc, but comparing "A2" < "A18" is probably not what user wants (instead "A02" < "A18" OR compare question order).

Steps To Reproduce

User had survey which broke. Mandatory question was hidden on client (browser) but "shown" on server, leading to mandatory violation BUT without any possibility to actually fill in the question. The origin of the problem was "2" > "18" (age compare).

Additional Information

Two commits (that were reverted):

0dee716e8bcc01eceff9c85d9de1a5b5a815c647

efa8a1012897af32632737c92e2d98f433cf6ba8

TagsNo tags attached.
Complete LimeSurvey version number (& build)latest master
I will donate to the project if issue is resolvedNo
Browser-
Database & DB-Version-
Server OS (if known)-
Webserver software & version (if known)-
PHP Version-

Relationships

related to 15501 new relevanceStatus : different evaluation in JS and PHP 

Activities

DenisChenu

DenisChenu

2019-10-15 11:38

developer   ~54044

Last edited: 2019-10-15 11:38

View 2 revisions

"2" > "18" is not really different (bvecause you can 't have it in JS)

With Q00 as single choice with number Q00 as 5 : Q00 < "10" is different in PHP and JS …
Seems i made an error in https://github.com/LimeSurvey/LimeSurvey/blob/93455a7c64c78793375acb4e2eed6963c6cc6334/application/helpers/expressions/em_core_helper.php#L327

Maybe must be STRING for both ?

'A2' > 'A18' : no issue in my point of view (no diff in JS and PHP) … but send a warning is better.

DenisChenu

DenisChenu

2019-10-15 11:42

developer  

limesurvey_survey_jsphpIssueCompare.lss (18,722 bytes)
DenisChenu

DenisChenu

2019-10-15 11:43

developer   ~54045

@ollehar : in my opinion : must separate the issue

  1. PHP vs JS compare must be equal ( Q00 < "18") : 1st issue
  2. intuition/ default answer code

PS : since 2.05 : my advice when compare number is always do intval(Q00) before ;)

ollehar

ollehar

2019-10-15 14:34

administrator   ~54046

But we agree: Warning (or error) when doing "18", e.g. [0-9]* in quote?

DenisChenu

DenisChenu

2019-10-15 14:37

developer   ~54047

Yes : my opinion :

  1. We must fix JS vs PHP (or if not able to fix : send an error) for Q00 < "18"
  2. Send a warning about usage of "[0-9]" and "" (adding "" to force string) and '[0-9]' and ''

The A2 > A10 is another issue :)

ollehar

ollehar

2019-10-15 14:38

administrator   ~54048

Related: Lots of people (except Denis) agree answer codes should be numeric as default, not alphanumeric.

DenisChenu

DenisChenu

2019-10-15 14:41

developer   ~54049

My real opinion : empty default answer code (for the 1st)
If have number : seond can be number +1

Same for subquestion code
Same for question code

:D

DenisChenu

DenisChenu

2019-10-17 08:01

developer   ~54067

@ollehar : can we list the real part wher it's broken : i mean compare is different between PHP and JS ?

ollehar

ollehar

2019-10-17 10:27

administrator   ~54072

"2" > "18" is different in PHP and JS.

DenisChenu

DenisChenu

2019-10-17 11:35

developer   ~54078

But : did you have a Survey for this.

I know it's different, but since we have an hacked compare with PHP : maybe we can try to dix it .
No ?

DenisChenu

DenisChenu

2019-10-17 11:36

developer   ~54079

I think i do an error :
If both can be number AND and are string : sometimes it‘s OK sometimes not.

For example (in JS)

  1. Q00 < "18" => compare as number ( WORD vs FORCED STRING)
  2. Q00 + "" < "18" => compare as string ( FORCED STRING vs FORCED STRING)
ollehar

ollehar

2019-10-17 11:38

administrator   ~54081

Please, don't try to hack EM more - it's already too complex. The best solution is a warning message, IMO.

DenisChenu

DenisChenu

2019-10-17 11:41

developer   ~54082

Not for this part : since code are already updated since some year now …
https://github.com/LimeSurvey/LimeSurvey/blob/033b995ee5698109120694479a391b3e2ec59ad5/application/helpers/expressions/em_core_helper.php#L353-L362

The fix can be use && here : https://github.com/LimeSurvey/LimeSurvey/blob/033b995ee5698109120694479a391b3e2ec59ad5/application/helpers/expressions/em_core_helper.php#L328

ollehar

ollehar

2019-10-17 11:47

administrator   ~54083

Again, I disagree. Writing < "18" is always a conceptual error from the users side.

DenisChenu

DenisChenu

2019-10-17 12:45

developer   ~54087

Yes, it can be. But the biggest issue : JS must be same than PHP.

Since it's hard ( impossible ?) to fix 2 &lt; '18' and '2' &lt; '18' in JS :

  1. PHP must have the same behaviour :)
  2. A warning must be shown on admin part each time a user have "[0-9 ]" or '[0-9 ]' inside equation :)
ollehar

ollehar

2019-10-17 15:10

administrator   ~54088

  1. Don't know how to fix
  2. Yes, for this BUT also for answer codes BECAUSE "A2" < "A18", so still unintuitive behvaiour. Question is: What is users intention? Anyway, let's discuss next team meeting? With Carsten.

Issue History

Date Modified Username Field Change
2019-10-15 11:32 ollehar New Issue
2019-10-15 11:34 ollehar Steps to Reproduce Updated View Revisions
2019-10-15 11:37 ollehar Additional Information Updated View Revisions
2019-10-15 11:38 DenisChenu Note Added: 54044
2019-10-15 11:38 DenisChenu Note Edited: 54044 View Revisions
2019-10-15 11:41 ollehar Description Updated View Revisions
2019-10-15 11:42 DenisChenu File Added: limesurvey_survey_jsphpIssueCompare.lss
2019-10-15 11:43 DenisChenu Note Added: 54045
2019-10-15 14:34 ollehar Note Added: 54046
2019-10-15 14:37 DenisChenu Note Added: 54047
2019-10-15 14:38 ollehar Note Added: 54048
2019-10-15 14:41 DenisChenu Note Added: 54049
2019-10-17 08:01 DenisChenu Note Added: 54067
2019-10-17 10:27 ollehar Note Added: 54072
2019-10-17 11:35 DenisChenu Note Added: 54078
2019-10-17 11:36 DenisChenu Note Added: 54079
2019-10-17 11:38 ollehar Note Added: 54081
2019-10-17 11:41 DenisChenu Note Added: 54082
2019-10-17 11:47 ollehar Note Added: 54083
2019-10-17 12:45 DenisChenu Note Added: 54087
2019-10-17 15:10 ollehar Note Added: 54088
2019-10-31 10:20 DenisChenu Relationship added related to 15501