Error with datepicker for at least one month

jsavell commented 9 months ago

The datepicker is having trouble with November 2024:

30-days-hath-november

andyp-uk commented 9 months ago

I cannot replicate this bug on https://github.com/coral-erm/coral/tree/ERM-63-modern-jquery-update-with-php8

jsavell commented 9 months ago

I'm able to recreate the bug on that branch:

calendar-bug-on-updated-branch

The comments from the bug fix in the source js says it only manifests for certain server locale/daylight saving configurations:

                // addDays(1) fails in some locales due to daylight savings. See issue 39.
                //currentDate.addDays(1);
                // set the time to midday to avoid any weird timezone issues??

(Sorry in advance for the strange line number references that follow. It looks like Github can't handle both viewing a 'large diff' and linking into specific lines in that diff.)

Looking in the history, the original introduction of the plugin file came through the management module and was the more up to date Kelvin Luck source that includes the bug fix (Line 141 of 'management/js/plugins/jquery.datePicker.js'): https://github.com/coral-erm/coral/commit/01b3474bafc1091e560ae3f545920df2314e4fe4#diff-8f1a01dd8201733947ec454503b7d8ccc847464751c64a352e3685978fb94b17R141

Then, that file was updated to support i18n: https://github.com/biblibre/Coral/commit/ac4c50aa066562d0f01c66e8a0011ccf91145b84#diff-8f1a01dd8201733947ec454503b7d8ccc847464751c64a352e3685978fb94b17

After that, the file was moved from the management module to a shared directory and renamed to reflect the previous i18n modification: https://github.com/coral-erm/coral/pull/196/files#diff-b0b99dfbd73b4bbf76e50dabfe3a9692460680460ed3ad17b1442dae1071abc9

Later, when completing a jquery upgrade, the file was switched to use a forked version for compatibility with jquery 1.8. Unfortunately, that version was based on an older, unpatched version of the Kelvin Luck source (Line 141 of 'js/plugins/jquery.datePicker-patched-for-i18n.js'), which appears to have introduced this bug into Coral for the first time: https://github.com/coral-erm/coral/commit/35ec64ba49f051cb4a53b27350aeacb81f225976#diff-b0b99dfbd73b4bbf76e50dabfe3a9692460680460ed3ad17b1442dae1071abc9L141

Line 2 of 'js/plugins/jquery.datePicker-patched-for-i18n.js' demonstrates this pretty clearly, with the header copyright year dropping from 2008 to 2007: https://github.com/coral-erm/coral/pull/620/files#diff-b0b99dfbd73b4bbf76e50dabfe3a9692460680460ed3ad17b1442dae1071abc9L2

So, after looking at this more deeply, I think the complete solution would be to combine the original, up to date 2008 version of the datepicker with both the fork's introduction of jquery 1.8 (or later version) and the i18n key replacements.

andyp-uk commented 9 months ago

The Steering Committee is hoping to wrap up work on the upcoming release in the next week. If you're able to create a PR to merge to https://github.com/coral-erm/coral/tree/ERM-63-modern-jquery-update-with-php8 we can smoke test this and include it with the next release.

jsavell commented 8 months ago

I've opened an alternative PR into the ERM-63 branch that fixes the original bug and addresses these followup concerns.

I'm going to close the original PR into the development branch.

coral-erm / coral

Error with datepicker for at least one month #747