User tests: Successful: Unsuccessful:
Pull Request for Issue #13869.
Added a JFormRule in com_fields to make sure default value is validated against the field type.
Displays a validation message.
Does not validate the default value against the field type.
No.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_fields |
This works for the core form fields. But when a custom field like the gallery one is used, then the rule doesn't get found. But first let's get this one in and I will add then support for none core fields.
I have tested this item
Thanks for the tests. RTC please !
I couldn't test this yet but I think rhe approach doesn't work. For example the calendar allows a special case "NOW" for the defaulr value, which eesults in the current date/ime displayed by default. This PR will break that if I read thw code right.
There likely will be other such cases.
Imho, it's the responsibilty of each formfield to make sure the default value is allowed if needed. Custom Fields mostly directly extend from those library classes and don't implement own logic.
Now had the chance to test this and looking at the code closer.
Actually calendar isn't affected because there is no JFormRuleCalendar. The whole code only works if the typename of the field matches the rulename, which for core is only the case for color
and ùrl` as far as I see. So this doesn't make much sense to me.
For once the validation shouldn't be tied to the name of the field type and if we implement it in the component it should be more generic than only working for two fields.
I still think validation should be done in the formfield class itself and only if needed. And personally I think in most cases it's not needed at all. After all we currently don't sanitise the default values at all when set in the XML. It's the exact same code which "breaks" here when the default value is set wrong in the XML or in the custom field.
Labels |
Added:
?
|
I have just fixed two CS issues.
@franz-wohlkoenig i'm working on another PR that i'll submit soon.
Thanks @marrouchi so this PR can be closed?
Okay
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-02-22 15:57:35 |
Closed_By | ⇒ | marrouchi |
Thanks @marrouchi
I have tested this item✅ successfully on da3e463
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13888.