plone / Products.CMFPlone

The core of the Plone content management system
https://plone.org
GNU General Public License v2.0
254 stars 191 forks source link

"last modified" dates format: not the same in "View" and in "Contents" of a folder #2953

Closed dbitouze closed 4 years ago

dbitouze commented 5 years ago

BUG/PROBLEM REPORT (OR OTHER COMMON ISSUE)

What I did:

I installed a fresh Plone 5.2 (python3) instance and started to create a new Plone site in French, with "last modified" dates displayed. Then I added content and looked at the content of a folder either from (one of) its view(s) or from its "Contents" (/folder_contents).

What I expect to happen:

I expected the "last modified" dates format to be the same in the two cases, following the French rules (dd/mm/yyyy hh:mm).

What actually happened:

The "last modified" dates format are:

hence not the same.

What version of Plone/Addons I am using:

vincentfretin commented 5 years ago

Can you please give us a screenshot? I'm currently seeing "Par admin — Dernière modification 8 hours ago" in French (another issue) Capture d’écran de 2019-10-13 17-24-44 Capture d’écran de 2019-10-13 17-25-19

dbitouze commented 5 years ago

Cf.:

image

versus:

image

(BTW, another issue in the translations "external", "internal", etc.)

dbitouze commented 5 years ago

Quite strange! Sometimes the "Contents" case is OK:

image

and sometimes not:

image

AFAICS, it is OK if another view (other than the standard one) has been previously selected...

vincentfretin commented 4 years ago

For the date in the Last modified column, #2074 may be related.

vincentfretin commented 4 years ago

Ah this is actually a regression in Plone 5.2.0? In the buildout.coredev 5.1 it actually works correctly. Capture d’écran de 2020-06-20 14-49-10

What changed?

vincentfretin commented 4 years ago

For the Last modified column, the regression is in plone-editor-tools bundle between mockup branch 2.7.x and master. (I tried to use mockup 2.7.x in Plone 5.2 and this fixed the issue)

For the "3 minutes ago" in toolbar and "by line", I don't know.

dbitouze commented 4 years ago

And what about" external", "internal", etc.?

vincentfretin commented 4 years ago

The review state not translated is still an issue. I made a long list of the issues here https://github.com/plone/Products.CMFPlone/issues/2188#issuecomment-646535054 See the list of known i18n issues here https://github.com/plone/Products.CMFPlone/issues/3123

vincentfretin commented 4 years ago

The "3 minutes ago" is using the pat-moment pattern from mockup, but switching mockup to 2.7.x doesn't resolve the issue, so the regression may be a change in an other package?

If someone want to dig in this regression, I switched mockup like this:

cd src/mockup
git checkout 2.7.x
make bootstrap

and put plone-editor-tools bundle in develop mode (see https://github.com/plone/mockup/pull/985)

To go back to master:

git checkout package-lock.json
git checkout master
rm -rf node_modules
make bootstrap
vincentfretin commented 4 years ago

OMG no, this is not related to branch 2.7.x vs master at all. I'm on mockup master, if I put the plone-editor-tools js bundle in develop mode, the issue for the Last modified column is fixed. This is the same thing as #3124 . Is the bundle not up to date? How do we rebuild that for the next release?

vincentfretin commented 4 years ago

Compared to branch 2.7.x, mockup master now lazy load the moment locale, I put a console.log in moment pattern mockup/patterns/moment/pattern.js, I see it will do require(['moment-url/fr']);, but I still get the "3 minutes ago" in English, not in French. @davilima6 you worked on this, do you know what's going on? Did it really work with non English locale at one point?

vincentfretin commented 4 years ago

The require is a requirejs thing. How and where it is supposed to load an additional javascript file? I see in mockup/js/config.js that moment-url is actually an alias to node_modules/moment/locale. I see in PR https://github.com/plone/plone.staticresources/pull/10 that the moment/locale folder was committed in plone/staticresources/static/components/moment/locale @agitator can you help?

vincentfretin commented 4 years ago

Actually I can see in Chrome network tab that http://0.0.0.0:8080/Plone/++plone++static/components/moment/locale/fr.js is downloaded. But it's still not in French.

vincentfretin commented 4 years ago

@davilima6 This is an issue with requirejs and the require line it seems. If I put 'moment-url/fr' in the define, I see the French translation.

vincentfretin commented 4 years ago

@davilima6 I think the require is somewhat asynchrone and the moment date are rendered before the fr.js is fully loaded? When you first show folder contents, it downloads the fr.js but in the toolbar you see "20 hours ago" in English. Then if you click on folder contents to show 30 items per page, it will somewhat update the moment pattern in the toolbar and you can see in French "il y a 20 heures".

davilima6 commented 4 years ago

Hi, @vincentfretin. Thanks for the investigation. I'll try to reproduce the issue and the path you took. Your last message might mean that we lack reloading the pattern after the require (possibly in a delayed manner). I think pat-registry has a scan function for that.

vincentfretin commented 4 years ago

Hi @davilima6 You seems to know better this stuff than me. I'll leave it to you. :pray: Thanks.

vincentfretin commented 4 years ago

@davilima6 I did the fix in https://github.com/plone/mockup/pull/987

davilima6 commented 4 years ago

Looks good. Thanks!