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

Fix parsing for months/weekdays with weird characters #3078

Closed
wants to merge 2 commits into
base: develop
from

Conversation

Projects
None yet
5 participants
@ichernev
Contributor

ichernev commented Mar 31, 2016

This is a better implementation for #2782 (fixing #2801). It is a continuation of #2869 (which had to be shipped faster, without all the intended improvements).

  • implement for weekdays
  • add tests for weekdays
@ichernev

This comment has been minimized.

Contributor

ichernev commented Apr 4, 2016

This is ready to go. The amount of locales that were fixed with this PR is just immense. I wonder how we didn't get more complaints earlier. @mj1856 @icambron quick review?

@icambron

This comment has been minimized.

Member

icambron commented Apr 4, 2016

Wow, awesome work. It makes sense to me, but I was very cursory.

@maggiepint

This comment has been minimized.

Member

maggiepint commented Apr 4, 2016

Those handleStrictParse functions are a classic example of why Javascript needs a real null coalescing operator. They're as good as they can be without the language having that though.

@ichernev

This comment has been minimized.

Contributor

ichernev commented Apr 12, 2016

@maggiepint yeah, all I was missing was goto to make it complete :D

@@ -0,0 +1,18 @@
var indexOf;
if (Array.prototype.indexOf) {

This comment has been minimized.

@socketpair

socketpair Apr 12, 2016

Contributor

Es6 must provide it, please dont reinvent wheel.

if (strict) {
if (format === 'dddd') {
ii = indexOf.call(this._weekdaysParse, llc);
mixedPieces[i] = regexEscape(mixedPieces[i]);
}
this._weekdaysRegex = new RegExp('^(' + mixedPieces.join('|') + ')', 'i');

This comment has been minimized.

@socketpair

socketpair Apr 12, 2016

Contributor

Should each piece be escaped ? Note, there is RegExp.escape function in es6

@ichernev

This comment has been minimized.

Contributor

ichernev commented Apr 16, 2016

@socketpair we're not using es6 features, only the module system.

@ichernev

This comment has been minimized.

Contributor

ichernev commented Apr 16, 2016

Merged in 41fdb58

@ichernev ichernev closed this Apr 16, 2016

ichernev added a commit that referenced this pull request Apr 16, 2016

Merge pull request #3078 from ichernev:fix-word-parsing
Fix parsing for months/weekdays with weird characters

beledouxdenis added a commit to odoo/odoo that referenced this pull request Jul 5, 2016

[FIX] web: parsing of date with abbreviated month/day
When using the abbreviated month in the language
date format (`%b`),
e.g. `%d %b %Y`
in some languages, the parsing of the date
from this format to the database format (YYYY-mm-dd)
failed because of a dot `.` added from time to time
to the end of the abbreviated name
e.g., in French, `5 juil. 2016`

This is a bug of `moment.js` < 2.13.0,
handling badly this dot.
This is solved from `moment.js` 2.13.0,
thanks to the below revision
moment/moment@41fdb58
moment/moment#3078

Unfortunately, we cannot update this library to its latest
release in stable release of Odoo (e.g. 9.0),
as this is seen as an unstable change
(if some API changes occured in the given library)

Instead, if the parsing of the date fails with the
strict mode (meaning the date must respect the format
exactly), we perform a second pass without the strict mode,
so this dot will be ignored,
and the date can be correctly parsed.

The issue can be reproduced by loading the French language,
and setting `%d %b %Y` as date format, and then performing
an advanced search on a date field. The value of the date
in the advanced search will be empty without this revision
(for the months having more than 4 chars,
other than `mars`, `mai`, `juin`, `aout`).

Fixes #11854
opw-682534
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment