ManageSieve: make sieve_formattime() parse all time-formatters PHP does. #9655
Conversation
|
Thank you very much for your contribution! Your solution is solid, but please don't include the drive-by improvements into the same PR (or at least not into the same commit). It makes it unnecessary hard to comprehend, which changes are related. Could you correct that? |
bennet0496
force-pushed
the
master
branch
2 times, most recently
from
1a72010
to
2352247
Compare
|
I removed the drive-by changes to the 12h format. Do you want the fix for that here as another commit or do you want another PR for that? Also I removed the Unix timestamp, because looking over it, it doesn't quite makes sense here, because of the implicitly included date. |
|
Please open a new PR for the 12h-changes. Thank you! |
|
Great, thanks! Now all that's missing from my point of view is an entry to the Changelog. Would you mind adding one? (Feel free to decline!) @alecpl Any thoughts on this? |
|
Is the entry ok like this? If not I will revert it :) |
|
Perfect, thank you! 👍 |
|
@alecpl |
|
@alecpl I'll merge this after one more week without a comment. /remind me to merge in one week |
|
@pabzm set a reminder for 11/28/2024 |
|
@bennet0496 Can you rebase to resolve the conflict, please? |
…me() that PHP understands
|
👋 @pabzm, merge |
|
@bennet0496 Thanks again for your contribution! 🎊 |
Roundcube's
$config['time_format']understands all PHP time and especially timezone format specifiers listed in the PHP docs; however,sieve_formattime()that used this value viarcmail.env.time_formatdoes not.Therefore if the config value e.g. is
$config['time_format'] = "H:i T"to include the abbreviated timezone name (which works else where, like the message list), then forms usingsieve_formattime(), the OOO form, will generate times like "6:00 T" and so onThis causes the actual time be set to "Tango Time", i.e. UTC-7
Which for users outside of UTC-7 is unexpected behavior.
Therefore I changed
sieve_formattime()to understand any formatter that may reasonably (or unreasonably, like microseconds or Unix Time. But never say never) be configured by an admin or user. For the timezone values, I opted to pass these values from PHP to JS viaset_env()as in JS these values are extraordinarily hard to reliably to obtain without additional libraries.I also changed some thing at the beginning of
sieve_formattime()that looked odd to me. Like per-declaring all variables. And actually used the value thathwould have been in thegandhcases. But I'm also happy reverting these changes if they are undesired.