Summary | monthdayyear form element not saved in DB |
Queue | Horde Framework Packages |
Type | Bug |
State | Resolved |
Priority | 1. Low |
Owners | jan (at) horde (dot) org |
Requester | birnbacs (at) gmail (dot) com |
Created | 10/08/2015 (3561 days ago) |
Due | |
Updated | 02/11/2016 (3435 days ago) |
Assigned | 10/11/2015 (3558 days ago) |
Resolved | 02/11/2016 (3435 days ago) |
Github Issue Link | |
Github Pull Request | |
Milestone | |
Patch | Yes |
State ⇒ Resolved
commit 75e9b5e3065dae60273c7920a83c804de3a64afc
Author: Jan Schneider <jan@horde.org>
Date: Thu Jan 14 14:52:15 2016 +0100
[jan] Allow any empty format specifiers for the monthdayyear
field (
Bug #14130).framework/Form/package.xml | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
http://github.com/horde/horde/commit/75e9b5e3065dae60273c7920a83c804de3a64afc
commit 9f65a05d470ee9e2ca797ce26f09acb4dcef9b5c
Author: Jan Schneider <jan@horde.org>
Date: Thu Jan 14 14:51:08 2016 +0100
Loosen the the test for the date format (
Bug #14130).Theoretically it might have been intended to explicitly pass an
empty string (as opposed to a null value) as the date format. But this
doesn't make sense because that would generate a form field that never
has a result value. Beside that we already *do* catch an empty value,
even if it's a strict null check.
framework/Form/lib/Horde/Form/Type.php | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
http://github.com/horde/horde/commit/9f65a05d470ee9e2ca797ce26f09acb4dcef9b5c
should explicitly check for this using empty(). This makes the
code's intent much cleaner. Setting the equality check as you did
can hide other issues (such as in this case where some code is
passing a value that is seemingly incorrect) or make them harder to
track down.
An additional check for is_string() would probably be overkill.
erroneous parameters. It seems legitimate to not accept null as a
format string as the null format never makes sense. The sames´goes
for formats like an empty array, 0 and false.
should explicitly check for this using empty(). This makes the code's
intent much cleaner. Setting the equality check as you did can hide
other issues (such as in this case where some code is passing a value
that is seemingly incorrect) or make them harder to track down.
erroneous parameters. It seems legitimate to not accept null as a
format string as the null format never makes sense. The sames´goes for
formats like an empty array, 0 and false.
More errors may wait up the call stack but this also seems to be one.
equivalence of value, not type. So a value of e.g., false or 0 will
be considered equivalent to null in this case. Probably what is
happening is there is some code further up the chain that is passing
a false-ish value to the format_in parameter instead of null.
State ⇒ Feedback
This isn't really a correct fix. Changing == checks only for
equivalence of value, not type. So a value of e.g., false or 0 will be
considered equivalent to null in this case. Probably what is happening
is there is some code further up the chain that is passing a false-ish
value to the format_in parameter instead of null.
Priority ⇒ 2. Medium
Type ⇒ Bug
Summary ⇒ monthdayyear form element not saved in DB
Due ⇒ 10/30/2015
Queue ⇒ Horde Framework Packages
Milestone ⇒
Patch ⇒ Yes
State ⇒ Unconfirmed
monthdayyear but it wouldn't work, i.e. no matter what I entered the
values were not written into the backend.
I tracked down the bug to the Form module of pear. In Form/Type.php
the monthdayyear object has a method called _validateAndFormat(). It
appears that the comparison in line 3021 of $this->_format_in with
null is using the wrong operator (===). I changed it to == and the
parameter gets saved to (and retrieved from) the database.
Here is the method in question (fixed version):
/**
* Validate/format a date submission.
*/
function _validateAndFormat($value, &$var)
{
/* If any component is empty consider it a bad date and return the
* default. */
if ($this->emptyDateArray($value) == 1) {
return $var->getDefault();
} else {
$date = $this->getDateOb($value);
if ($this->_format_in == null) { // line 3021, changed
comparison operator from === to ==
return $date->timestamp();
} else {
return $date->strftime($this->_format_in);
}
}
}