amsul / pickadate.js

The mobile-friendly, responsive, and lightweight jQuery date & time input picker.
http://amsul.ca/pickadate.js
MIT License
7.7k stars 1.02k forks source link

Submitted date of "201-06-02" when using formatSubmit: 'yyyy-mm-dd' 🤔 #1177

Open quinncomendant opened 5 years ago

quinncomendant commented 5 years ago

We use pickadate.js version 3.5.6 on a restaurant website for receiving reservations. It has been working correctly for months, but yesterday we received a reservation with the following date:

201-06-02

The date the user selected (formatted in their locale by pickadate):

giovedì 20 giugno 2019

The correct date submitted should have been 2019-06-20.

I'm not able to reproduce this issue, so I'm hoping someone reads this and has some idea what's going on. Perhaps it's related to #1089?

Here's the HTML:

<form …>
    <input type="text" name="date" class="reserve-date">
    <input type="hidden" name="date_for_display">
    <!-- etc … -->
</form>

<script type="text/javascript" src="js/pickadate/lib/compressed/picker.js"></script>
<script type="text/javascript" src="js/pickadate/lib/compressed/picker.date.js"></script>
<script type="text/javascript" src="js/pickadate/lib/compressed/picker.time.js"></script>
<script type="text/javascript" src="js/pickadate/lib/compressed/legacy.js"></script>

Here's the JS:

var $pickadate = $('.reserve-date').pickadate({
    formatSubmit: 'yyyy-mm-dd',
    hiddenName: true,
    disable: [
        1,
        { from: -120, to: d },
        { from: [2019,0,1], to: [2019,2,21] },
        { from: [2020,0,7], to: [2020,2,19] }
    ],
    onSet: function (c) {
        $('input[name="date_for_display"]').val(this.get());
    }
});
quinncomendant commented 5 years ago

Update: I have been able to reproduce this issue. It occurs under the following conditions:

Which means the initial state of this input contains:

    <input type="text" name="date" class="reserve-date" value="giovedì 20 giugno 2019">

When the page loads with the Italian localized date like this, the general sequence of events is:

  1. <input name="date"> value updates to 0201-06-02 (because we're using formatSubmit)
  2. The onSet function runs.
  3. <input name="date_for_display"> value is set to giovedì 20 giugno 2019.

So it seems pickadate is unable to correctly parse the italian locale format of giovedì 20 giugno 2019 when converting the date into the formatSubmit. This may be a bug in pickadate, or else pickadate does not support this locale, or I've implemented pickadate incorrectly.

The solution in this case is to define a format option (I'm using d mmm yyyy so the date is changed to a parsable format, and never use dates like giovedì 20 giugno 2019.

DanielRuf commented 5 years ago

Hi @quinncomendant,

Can you provide a small reproducible testcase eg on codepen?

The options at https://github.com/amsul/pickadate.js/blob/master/lib/translations/it_IT.js seem to be ok at a first glance.

quinncomendant commented 5 years ago

Hey @DanielRuf, I've added a Codepen: https://codepen.io/strangecode/pen/vqmJbp

I simplified the PoC code, so the bug occurs still with as little code as:

<input type="text" name="date" value="giovedì 20 giugno 2019">
var $pickadate = $('input[name="date"]').pickadate({
    hiddenName: true,
});

Although I kept formatSubmit: 'yyyy-mm-dd', in there too so you can see the same incorrect date that I reported above.

DanielRuf commented 5 years ago

Well, unicode in regular expressions in JavaScript is a big problem.

https://codepen.io/DanielRuf/pen/BgRwKb?editors=1010

See https://gist.github.com/DanielRuf/30fe0fd89223b16dd718adc159c3ab8e/revisions for the needed changes for at least some languages.

See https://stackoverflow.com/a/22075070/753676

https://stackoverflow.com/a/150078/753676 is not the right approach.

Also \u does not really work as expected:

"giovedì".match( /\w+/u )
["gioved", index: 0, input: "giovedì", groups: undefined]
"giovedì".match( /[^\x00-\x7F]+|\w+/ )
["gioved", index: 0, input: "giovedì", groups: undefined]
"giovedì".match( /[^\x00-\x7F]+|\w+/u )
["gioved", index: 0, input: "giovedì", groups: undefined]

Probably longer and more complete solution: https://stackoverflow.com/a/48902765/753676

DanielRuf commented 5 years ago

https://github.com/mathiasbynens/regexpu https://mothereff.in/regexpu#input=var+regex+%3D+/%5Cp%7BL%7D/u%3B&unicodePropertyEscape=1

@amsul what do you think? Which solution should we use? Such regular expressions have probably a high impact on the performance.

DanielRuf commented 5 years ago

An alternative solution would be to use the short version, see https://codepen.io/DanielRuf/pen/JQNOLV. Or use a Date object.

quinncomendant commented 5 years ago

Oh, cool you found the problem: unicode in regex. I've encountered this before. 🤕

An alternative solution would be to use the short version, see https://codepen.io/DanielRuf/pen/JQNOLV. Or use a Date object.

Yeah, that's how I solve the problem for us. I'm using a very short version:

format: 'd mmm yyyy'
amsul commented 4 years ago

@DanielRuf yeah those transpiled expressions seem like they'd have a significant impact on perf. Wouldn't the regexp in https://github.com/amsul/pickadate.js/pull/821 be sufficient?

DanielRuf commented 4 years ago
"giovedì".match(/[^\x00-\x7F]+|[a-zA-Z0-9_\u0080-\u00FF]+/ )[ 0 ]
"giovedì"

But as others already mentioned in the threads, this may not cover all languages. So we should really test this to ensure that we cover the current languages and that nothing breaks or is still broken.

DanielRuf commented 4 years ago

Keep in mind that this is a transpiled reular expression which supports all unicode characters / languages. See also https://esdiscuss.org/topic/inclusion-of-unicode-character-set-constants-in-regexp-p-l.