developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.
Opened 12 years ago
Closed 12 years ago
#781 closed bug (fixed)
Bug de validation d'un formulaire quand il y a un tag <radiobuttons> dans le formulaire
Reported by: | geekbay | Owned by: | Julien |
---|---|---|---|
Priority: | normal | Milestone: | jelix 1.1 |
Component: | jelix:forms | Version: | trunk |
Severity: | normal | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Documentation needed: | no | |
Hosting Provider: | Php version: |
Description
Si dans myform.form.xml on a un tag <radiobuttons> par exemple
<?xml version="1.0" encoding="utf-8"?> <form xmlns="http://jelix.org/ns/forms/1.1"> <radiobuttons ref="couleur"> <label>Couleur de votre voiture:</label> <item value="r">Rouge</item> <item value="b">Bleue</item> <item value="v">Verte</item> <item value="j">Jaune</item> </radiobuttons> <input ref="nom" required="true" maxlength="255"> <label>nom</label> </input> <submit ref="_submit"> <label>Valider</label> </submit> </form>
alors il n'y a plus de validation javascript due a l'erreur suivante:
"").replace is not a function |
Fichier Source : http://127.0.0.1/jelix/www/jquery/jquery.js Ligne : 23
Attachments (6)
Change History (22)
Changed 12 years ago by geekbay
comment:1 follow-ups: ↓ 2 ↓ 3 Changed 12 years ago by bibo
- Milestone set to jelix 1.1
- review set to review?
- Version changed from 1.0RC1 to trunk
thanks geekBay for your report and first patch. I tend to think the problem is generic to "list values" controls.
My patch fixes this in the two forms builder (jquery and light) and is focused on getValue prototype. getValue should always return an empty string if nothing has been edited/selected/checked even on list controls.
Asking for review.
comment:2 in reply to: ↑ 1 Changed 12 years ago by geekbay
yes it's a solution too since the bug was in the Jquery trim function that can't trim since in the case of a radio the argument was an array. So now if the getValue return always a string, trim() will work.
Replying to bibo:
thanks geekBay for your report and first patch. I tend to think the problem is generic to "list values" controls.
My patch fixes this in the two forms builder (jquery and light) and is focused on getValue prototype. getValue should always return an empty string if nothing has been edited/selected/checked even on list controls.
Asking for review.
comment:3 in reply to: ↑ 1 Changed 12 years ago by Julien
Replying to bibo:
Asking for review.
Hello,
doesn't seem to work, as it still returns an array, that can't be trimmed.
made another patch, that always returns a string. Tested with a real world case in an app.
Changed 12 years ago by Julien
comment:4 Changed 12 years ago by bibo
Effectively, Julien, my patch is buggy and still raises a jQuery.trim() error...
But the meaning of getValue is to return value(s) of form elements and as so, an array of values if there is multiple selection. And it should return a default failure value being it or false, if no value is retrieved. That failure value should be the same for all elements.
Here i see 2 paths :
1) do the trimming in getValue
2) add another condition before jQuery.trim : if (!val.join && jQuery.trim(...
I would lean toward 1, thoughts guys ?
comment:5 Changed 12 years ago by Julien
I think there's another way : adding a getValue() method in the prototype of every jFormsControl object.
That way, each type of control has it's own getValue method, it will be easier to maintain and to handle qite complex controls I think.
Look at what I did for jFormsJQControlDatetime or jFormsJQControlDate for example.
comment:6 Changed 12 years ago by bibo
That would be the cleaner solution. Also to use with But much more work tough !
comment:7 Changed 12 years ago by bibo
"Also to use with" should be trimmed to "" of course :)
comment:8 Changed 12 years ago by Julien
- Owner set to Julien
- Status changed from new to assigned
I think I can do it today.
I take the ticket.
comment:9 Changed 12 years ago by Julien
I think I found a quicker way: returning null when the array is empty (that really mean "there is no value").
Also added a check for "false" value (for unchecked required checkboxes), as we now use strict comparison operator, and
false === '' // will return false
Changed 12 years ago by Julien
comment:10 follow-up: ↓ 11 Changed 12 years ago by bibo
Julien, I see you have taken my proposal #1. ie. do the trimming in getValues. I really feel it is the good way to go.
Is this not possible to always return false on failure in getValues ? I mean : return false at the end and return field values if :
- trimmed input/textarea values != ''
- radiobuttons, checboxes, select : one item is selected at min
comment:11 in reply to: ↑ 10 Changed 12 years ago by Julien
Replying to bibo:
Julien, I see you have taken my proposal #1. ie. do the trimming in getValues. I really feel it is the good way to go.
yup, and it's quicker than writing a getValue method for each control type.
Is this not possible to always return false on failure in getValues ?
I would prefer returning null when there's no value:
- textboxes: trimmed value is an empty string
- checkbox: unchecked (means no value instead of false)
- select/checkboxes/radios: nothing selected
- default returned value at the end of getValue
thus, we'll only have to check against the null value
Is this what you meant ?
Changed 12 years ago by Julien
comment:12 Changed 12 years ago by Julien
last patch: we have to keep returning false for unchecked checkboxes...
so we now fire an error when control is required and value is null or false.
comment:13 Changed 12 years ago by laurentj
- Component changed from jelix to jelix:forms
Since this not the first regression on jforms.js, we really need some unit tests on this jforms.js script NOW.
So we must have, in testapp, a page which show ALL possible controls, and which includes a script which set/unset some values on each controls, and verify jForms methods/properties each time it does this modifications on the forms. Of course, this page will be display "manually", but a contributor who modify jforms.js will verify it this test page is ok.
If you want to use http://fireunit.org/ or any other js libraries, no problem, but we must do it. We cannot continue to have this kind of regression.
comment:14 Changed 12 years ago by laurentj
I'm going to add tests this next days.
comment:15 Changed 12 years ago by laurentj
- review changed from review? to review+
Committed some new unit tests + this patch. trunk + 1.1.x branch. r1235
comment:16 Changed 12 years ago by laurentj
- Resolution set to fixed
- Status changed from assigned to closed
Proposition de patch