User tests: Successful: Unsuccessful:
ISSUE : JHtml::calendar ignores $format when readonly attrib set
Did you had a look at the other PR? I would prefer a consistent solution for Joomla 2.5 and 3.x.
Also this one would only solve it for readonly, but would still ignore the format for editable fields?
@Bakual Yes, I did have a look. The first issue that you have mentioned was due to the hard coded format 'Y-m-d H:i:s' for readonly fields and have fixed that by replacing it with a method call to linux_to_php_date($format).
I could not reproduce the second issue you had mentioned (readonly and disabled attributes ignored).
Format attribute is respected for non readonly attributes as that was already being passed in the js initialisation of the calendar. Please test with field
Since i am really a Joomla novice, I am unable to see exactly where this solution fails for Joomla 3.x. Can you please explain that part to me?
@Bakual Yes, I did have a look. The first issue that you have mentioned was due to the hard coded format 'Y-m-d H:i:s' for readonly fields and have fixed that by replacing it with a method call to linux_to_php_date($format). Format attribute is respected for non readonly attributes as that was already being passed in the js initialisation of the calendar. Please test with field < field type="calendar" format="%d-%m-%Y" / >
I successfully reproduced the second bug in 3.2.1. But this does not exist in 2.5 and hence is not included here.
Format attribute is respected for non readonly attributes as that was already being passed in the js initialisation of the calendar.
I can't confirm that. See screenshot, left part is the article form and right part is the modified article.xml form definition where you see the format is set to the date only (without time).
As you see, the editable Created Date
field is not respecting the format neither. It's only respected in the JavaScript calendar itself. That is when you select a date from the popup window, it will show correct in the field. However if you load the article form the next time, it will show the time part again.
I see. Since the calendar respects the format after it has loaded, I believe the bug may not lie with the calendar field but in the way in which the stored date is first being put into the calendar field.
If we look into com_content, I think we should be able to find some line where the stored timestamp is being formatted using a hard coded format thus ignoring our format for that particular instance. Do you think you know where to look for that?
@Bakual Digging a little into com_content, I find that the calendar fields in question are being echoed on line #120 of /administrator/components/com_content/views/article/tmpl/edit.php which is
echo $this->form->getInput('created');
I am still unable to locate the definition of this function. I feel this is where the bug would be. Can you tell me where i can locate the definition?
Not the definition of the function actually, but where the initial values for this field are initialised. I think that is where calendar will be assigned the value stored in the db.
It's all in those two files you already found: libraries/joomla/form/fields/calendar.php
is the formfield which is called by JForm to display the field. The formfield then uses JHtml::_('calendar', ...)
in libraries/joomla/html/html.php
to actually show the calendar input.
The $value
itself comes straight from the database, is adjusted to the timezone as needed by the formfield and then passed to JHtml. JHtml itself just outputs the given value without applying any formats for the editable field.
Again, it's the same issue I fixed in the other PR for 3.x. I don't think converting the $format
back and forth is the solution for this issue, that's why I came up with the solution proposed in the PR where the format is simply applied using strftime.
Just looking at the code:
Now you're formatting the value twice for readonly fields, once in the formfield and once in JHtml.
And the editable fields still don't work if JHtml::calendar is used directly (not using the formfield).
That's why I used
$inputvalue = strftime($format, strtotime($value));
and use $inputvalue
then for the <input>
field.
There's a bit more code around it which deal with timezones to make sure the value isn't adjusted based on tz.
I get what you are saying. Let me fix that.
Still the same issues present I wrote 20 minutes ago and still don't understand why you don't want to use strftime()
directly.
@Bakual On line 982 of html.php, a format string is to be passed to date. Suppose we pass 'Y-m-d H:i:s' and it returns '2014-02-01 08:08:08'. To use strftime, you are first using strtotime on this value. if you pass this to strtotime, there is an ambiguity bet ween Y-m-d H:i:s and Y-d-m H:i:s as any one of 01 or 02 may be the month and then strtotime will apply its own method to resolve this. This may result in incorrect conversions.
Hence, i suggest that we translate the format string itself using our custom method.
Can you please confirm if your format string is not being ignored now so then we can move on to what you are saying about time zones.
Using formats like Y-d-m will likely fail anyway as soon as you save the value. Because during save PHP date will guess the format itself. If you want to solve that, you need a lot more code.
Also when you are using strftime, you don't need the self::_('date', ...)
part anymore, because $inputvalue
is already formatted correct.
As you can see in my PR, I don't use it there. I also don't need the if-else part to return different inputs when the field is readable or not. I just add the style and attributes as needed. Otherwise the output is the same.
@Bakual You can see this is one of my first pull requests so I tried not changing a lot of code to prevent unwarranted repercussions.
So I did it by adding a method and altering small chunks of code while your PR does it by alteration to a bigger chunk of code.
As you said, outputs are the same. So this does solve the issue for you, right?
You can see this is one of my first pull requests so I tried not changing a lot of code to prevent unwarranted repercussions.
I didn't want to discourage you. I very appreciate your help. I just wanted to point you to a better and already tested solution.
So I did it by adding a method and altering small chunks of code while your PR does it by alteration to a bigger chunk of code.
Actually, you ended up with 85 additions and 4 deletions, while mine has 49 additions and 39 deletions. I'd say the change is about the same size
Also my PR deals with an additional issue about readonly attributes which I believe isn't present in 2.5.
As you said, outputs are the same. So this does solve the issue for you, right?
I meant the output for readonly and editable fields is the same in my PR.
And no, the issue isn't solved
@Bakual Fixed the date to be formatted for readonly and editable even when calendar is directly used. However, I could not avoid formatting the date twice when a filter is set so I went through your code. If i am not wrong then there too the date is formatted twice in case a formfield is used and a filter is set (once while fixing the tz in calendar.php and then in html.php using strftime) so I let that remain.
I still stuck to my implementation because I feel I understand that better. Also, just to clarify, I've spent about 45 lines for a single array definition just to make it more readable so the additions are actually just half the number.
Still, please let me know if the PR is OK or if there is anything at all that I need to take care of further. I hope to take away from here more than just a bug fix. Thanks a lot for your patience.
If i am not wrong then there too the date is formatted twice in case a formfield is used and a filter is set (once while fixing the tz in calendar.php and then in html.php using strftime) so I let that remain.
The formfield only calculates the TZ difference based on the filter parameter (user or server timezone). It doesn't apply a format here.
Also, just to clarify, I've spent about 45 lines for a single array definition just to make it more readable so the additions are actually just half the number.
Don't want to be picky, but many of the changed lines in my PR are only because I removed an if-else. Most of the code is untouched elsewhere but shows up as changed because the whole block was moved around slightly. Plus there is an additional issue and related test.
It's really quite a small change once you understand what I did. It looks more complicate than it is.
Still, please let me know if the PR is OK or if there is anything at all that I need to take care of further.
We just need to make sure the solution is the same in the end in both PRs. I don't want a new JHtml method in J!2.5 which isn't present in J!3.x. The codebase should only be different when there are reasons for it. And for this I don't see any reason to have two different solutions. So either mine can be backported to 2.5 or yours will end up in 3.x as well. Otherwise it's just harder to fix something without actually gaining anything.
Personally I prefer my solution for obvious subjective reasons.
If you want, you can send me an email to bakual@bakual.ch so we could disucss it a bit better.
Closed as per the comment on the tracker
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-08-08 14:20:36 |
Please have a look at what is done in #2531. As I understand it, it's the same issue but for Joomla 3.2. You likely need a similar approach.
Simply removing the
%
from the format will not work as the format is set to%Y-%m-%d %H:%M:%S
and you'd end up withY-m-d H:M:S
instead of the correctY-m-d H:i:s
Also depending on extension and language, you would face other inconsistencies between the two date formats as well.