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

Explicitly set YYYY-MM-dd format for Zend_Date constructor #147

Open
wants to merge 1 commit into
base: 2.1.x
Choose a base branch
from

Conversation

fnogatz
Copy link

@fnogatz fnogatz commented Feb 6, 2023

In the Adminhtml's Renderer.php, the call of $this->date->date()->format('Y-m-d') correctly returns the string 2023-02-06 for today, February 6th of 2023. However, in installations with a different locale (like de_DE), this is wrongly interpreted as June 2nd of 2023 by the Zend_Date() constructor. For every date that might possibly be parsed both ways, this results in the problems described in #91 and #124: the set opening hours are incorrectly displayed at the very right of the range bar. If you look closely, you will notice that this problem was reported to be "magically gone" only after the 12th day of a month has passed, since this takes away the ambiguity in the date parsing process.

This PR explicitly states the date format of the given string, so it is parsed correctly independently on the set locale. It is more straightforward than my original proposal from three years ago in #91 (comment).

@pravaliterabricoman
Copy link

pravaliterabricoman commented Feb 8, 2023

Hello ! I discovered this bug yesterday, and was about to send an mail about it 😉 What a coincidence !

From my part, i analyzed that it came from the locale sent in the HTTP headers Accept-Language, by the browser.

If no format is given to Zend_Date, it will try to guess it, and it does it through the priority given in Accept-Language: everything was fine when fr_FR was firt, but if it was not in, or after en_US, i had the same bugs !

It's a very dangerous bug, because if you edit the retailer for another thing than the opening hours, and you don't notice they are not ok, you could possibly corrupt the data on save ! And if you have a shipping method using those hours... 😢

Thx a lot for this fix !

@pravaliterabricoman
Copy link

@fnogatz Hello ! Any news on when it would be merged and deployed ?

Thx 😄

@fnogatz
Copy link
Author

fnogatz commented Feb 17, 2023

No idea when the Smile folks will look into this, sorry 😇

In the meantime, you could apply this patch via composer patches.

@fnogatz
Copy link
Author

fnogatz commented Mar 7, 2023

Maybe @PrigentMatthieu or @Coosos can help out getting this merged?

@Coosos
Copy link
Contributor

Coosos commented Mar 14, 2023

I sent a message to my colleague :)

@fnogatz
Copy link
Author

fnogatz commented Mar 27, 2023

Thank you, @Coosos. Did you get any response?

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.

3 participants