developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.
#751 closed bug (fixed)
jForms: javascript check of secret control's confirmation does not work
Reported by: | Julien | 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
It didn't work because generated JS used jFormsJQControlString instead of jFormsJQControlSecret.
Now Javascript uses a generic confirm control, that may be used for other controls than <secret> (maybe we could introduce <confirm> for <input> controls in the future, to confirm an email for example)
Also updated unit tests.
NB : in the future, I think we'll need the ability to set errRequired, errInvalid and help messages for the confirmation control (currently they are inherited from the "parent" control when specified). But this will be another ticket.
Attachments (1)
Change History (10)
Changed 12 years ago by Julien
comment:1 Changed 12 years ago by Julien
- Status changed from new to assigned
comment:2 Changed 12 years ago by laurentj
- review changed from review? to review-
comment:3 Changed 12 years ago by Julien
I'm not sure this could be simplier, because with the old way in JS check of secret control with confirmation, the alert messages were not good:
if(this.confirmField){ var val2 = jfrm.getValue(jfrm.frmElt.elements[this.confirmField.name]); if(val != val2){ jfrm.tForm.errorDecorator.addError(this.confirmField, 2); // this line adds an error message equal to "this.confirmField.errInvalid", which is empty. return false; /* returning false will add a error message equal to this.errInvalid, which is not really good, because it's the confirmation which is invalid, not the "master" control itself. */ } }
So we have the following messages in the alert box:
- [* ] (blank message because this.confirmField.errInvalid is empty)
- [* the field "password" is invalid] (instead of: the "confirm password" field is invalid)
That's why I made all theses changes, and made the confirmation control a standard control, which is checked like all the others.
I'm not sure I can have a better approach to solve this bug, so please consider taking a look again at the patch.
NB: this patch only solves the bug, there is not implementation for the confirmation on <input> controls, nor grammar enhancement for the <confirm> tag (to specify proper alert messages, etc...). That will be another ticket, as I said.
comment:4 Changed 12 years ago by Julien
- review changed from review- to review?
comment:5 Changed 12 years ago by laurentj
After re-reading the patch, I'm ok for the principle. But I think we can simplify the code :
+ this._masterControl = jForms.tForm.getControl(name.replace(/_confirm$/,''));
Perhaps this following code is enough ?
+ this._masterControl = name.replace(/_confirm$/,'');
Do we really need the object of the control, since we need only its name ?
comment:6 Changed 12 years ago by Julien
you're right, we only need the name, will change it when committing if everything else is OK
comment:8 Changed 12 years ago by Julien
- Resolution set to fixed
- Status changed from assigned to closed
committed in the trunk, r1188
will backport it to 1.0.x branch too
comment:9 Changed 12 years ago by Julien
finally, it works right in 1.0.x
I don't think you have to change all this things to fix the bug. We are in a "features freeze" state (yes , I have to inform all developers of that :-)), so only bug fix are accepted for 1.1.
Please propose a patch to fix only the bug, and create a ticket to add the support of the confirm element on input elements.
I don't want to break something just before the RC1...