Incomplete string escaping in formats for calendar.

[Shrikant Dhayje]

At
The Given Code

390| handleCalendarCallback: function(num) {
391|            let format = get_format('DATE_INPUT_FORMATS')[0];
392|            // the format needs to be escaped a little
393|            format = format.replace('\\', '\\\\')
394|                .replace('\r', '\\r')
395|                .replace('\n', '\\n')
396|                .replace('\t', '\\t')

This replaces only the first occurrence of all the escape characters not all, and as per the the function is unable to completely accurate replacement of string with escape characters.

This replaces only the first occurrence of "", "/r", "/n", "/t" and "'",
So We Should Use regex for replacing all occurrence.

Used regex for easy updating.

This issue "Has patch" ​https://github.com/django/django/pull/15460

[Shrikant Dhayje]

"Has patch" ​​https://github.com/django/django/pull/15460

[Mariusz Felisiak]

Seems reasonable. Can you add a regression test? Do you have an example of the format with multiple occurrence of the same char that should be escaped?

[Fabian Jarrett]

Replying to Shrikant Dhayje:

"Has patch" ​​https://github.com/django/django/pull/15460

Hey, are you still working on this ticket? The PR was closed in April due to inactivity. Maybe someone else can take it.

[Fabian Jarrett]

Replying to Mariusz Felisiak:

Seems reasonable. Can you add a regression test? Do you have an example of the format with multiple occurrence of the same char that should be escaped?

I've looked and can't find any DATE_INPUT_FORMATS that contain escape characters in any of the languages. If they are not used should we just remove the replace?

[Sander Beekhuis]

Hi all, just chiming in without much django contribution experience

One can, in theory, set any DATE_INPUT_FORMATS in ones settings.py

I set the following

USE_L10N = False
DATE_INPUT_FORMATS = ["%Y-'''--%m-\t-%d"]

in my settings.py. It allows me to test the code paths discussed in this patch. If I use admin date picker to pick dates they are formatted using the logic in this JavaScript file.

However even with a single ' or \t in the format the behavior does not seem work in a very sensible way. Improving them would take more then the proposed patch. So dropping these replaces might make the most sense.

When I get time to work on this issue again I can try write to write a JS tests for this in the future. If that would be nice?

[Sander Beekhuis]

I also only now see this has been worked on in ​https://github.com/django/django/pull/15761/files#diff-c28a20f3291264125c6c8083e1c44075bf4e75e37bddd26a43c20ddc0a5fe837 as well.

Don't put to much weight on my statments. :)

[Mariusz Felisiak]

Replying to Sander Beekhuis:

Hi all, just chiming in without much django contribution experience

One can, in theory, set any DATE_INPUT_FORMATS in ones settings.py

I set the following

USE_L10N = False
DATE_INPUT_FORMATS = ["%Y-'''--%m-\t-%d"]

in my settings.py. It allows me to test the code paths discussed in this patch. If I use admin date picker to pick dates they are formatted using the logic in this JavaScript file.

However even with a single ' or \t in the format the behavior does not seem work in a very sensible way. Improving them would take more then the proposed patch. So dropping these replaces might make the most sense.

When I get time to work on this issue again I can try write to write a JS tests for this in the future. If that would be nice?

Ah yes, we should drop replacements. It was added in fa0653cd1d791a8bce835e8992cbeab6fd70d0e7 where we created a callback function by concatenating strings. It's unnecessary and buggy since d638cdc42acec608c1967f44af6be32a477c239f.