Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change date format in Last-Modifed Header #20905

Merged
merged 3 commits into from
Apr 30, 2022
Merged

Conversation

DimionX
Copy link
Contributor

@DimionX DimionX commented Jun 27, 2018

Pull Request for Issue #20905.

Summary of Changes

  1. Turn off translating Last-Modified header
  2. Append " GMT" to date format

Testing Instructions

  1. Set Russian Language
  2. Allow cache and set modified date:
$app = JFactory::getApplication();
$doc = JFactory::getDocument();
$app->allowCache(true);
$doc->setModifiedDate('Wed, 17 Aug 2005 00:00:00 GMT');

Expected result

Last-Modified: Wed, 17 Aug 2005 00:00:00 GMT

Actual result

Last-Modified: Ср, 17 авг 2005 00:00:00

1. No need to translate Last-Modified header
2. Append " GMT" to date format
@brianteeman
Copy link
Contributor

There was a reason you were asked to complete various sections when you submitted this. Please do so.

@DimionX
Copy link
Contributor Author

DimionX commented Jun 27, 2018

done

@ghost
Copy link

ghost commented Jun 28, 2018

@DimionX can you please Info which Issue is solved by your PR? You wrote "Pull Request for Issue #20905" which is this PR.

@DimionX
Copy link
Contributor Author

DimionX commented Jun 28, 2018

Pull request solves an Issue of incorrect sending of response header (Last-Modified)

@brianteeman
Copy link
Contributor

It is correct to add GMT as it is part of the rfc1123 specification. As it is always GMT and not a timezone I believe this is more correct than @mbabker suggestion and follows the recommendation at PHP.net

http://php.net/manual/en/function.gmdate.php

@mbabker
Copy link
Contributor

mbabker commented Jun 28, 2018

Honestly either way works. The format() method is already being told to format the Date instance's timestamp in GMT, so it will always output a GMT time. Just a personal nitpick about arbitrarily printing "GMT" on its own outside the format() method versus including it in the format() method's output to begin with. The only way it would be problematic is if the timezone identifiers couldn't reliably output "GMT", in which case it's 100% right as is.

@ghost ghost added the J3 Issue label Apr 5, 2019
@ghost ghost removed the J3 Issue label Apr 19, 2019
@laoneo
Copy link
Member

laoneo commented Mar 25, 2022

Sorry that there were no responses for such a long time. Can you add the suggestion from Michael and update the test instructions so we can properly test it. In the meantime I'm closing the pr, when ready, please reopen again. Thanks for your help making Joomla better.

@laoneo laoneo closed this Mar 25, 2022
@laoneo
Copy link
Member

laoneo commented Mar 25, 2022

I apologize, my impression was that this pr is not complete. Reopening again.

@laoneo laoneo reopened this Mar 25, 2022
@laoneo
Copy link
Member

laoneo commented Mar 25, 2022

I have tested this item ✅ successfully on e840c86


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20905.

@richard67
Copy link
Member

I have tested this item ✅ successfully on e840c86

I've added the code snipped from the testing instructions (except of the first line because that was already there) to the index.php of the site template and inspected the response header in the browser's developer tools.

Without this PR applied it is "last-modified: Thu, 01 Jan 1970 00:00:00 GMT".

With this PR applied it is "last-modified: Wed, 17 Aug 2005 00:00:00 GMT".

I used Russian site language for the test.

So it seems the browser can't understand the Russian day and month names and so can't understand the time stamp and so sets it to zero = begin of epoch.

With the patch of this PR applied I get the expected value.

With the patch applied but without the code snippet from the testing instructions, everything works as well as before regarding the last-modified header.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20905.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20905.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 26, 2022
@zero-24 zero-24 added this to the Joomla 3.10.9 milestone Apr 30, 2022
@zero-24
Copy link
Contributor

zero-24 commented Apr 30, 2022

Will merge this here now. Thanks 🥳

@zero-24 zero-24 merged commit 4131fca into joomla:3.10-dev Apr 30, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants