Skip to content
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

Plural translations don't seem to work #20453

Open
matiasdelellis opened this issue Apr 13, 2020 · 11 comments
Open

Plural translations don't seem to work #20453

matiasdelellis opened this issue Apr 13, 2020 · 11 comments

Comments

@matiasdelellis
Copy link

@matiasdelellis matiasdelellis commented Apr 13, 2020

Steps to reproduce

  1. Inside the server, set the language to a translation, for example, Spanish Argentina
  2. Find some plural translations. For example the summary of the current folder.
  3. Just check if the plural works.

Expected behaviour

In summary expect:

6 carpetas y 5 archivos 8,5 GB

Actual behaviour

In summary i see:

6 carpeta y 5 archivo 8,5 GB

imagen
😞

Note that the translation is programmed correctly:

// Substitute old content with new translations
$dirInfo.html(n('files', '%n folder', '%n folders', this.summary.totalDirs));
$fileInfo.html(n('files', '%n file', '%n files', this.summary.totalFiles));
$hiddenInfo.html(' (' + n('files', 'including %n hidden', 'including %n hidden', this.summary.totalHidden) + ')');

..and also is translated.

"_%n folder_::_%n folders_" : ["%n carpeta","%n carpetas"],
"_%n file_::_%n files_" : ["%n archivo","%n archivos"],

But use the singular, with the number greater than 1.

Server configuration detail

Operating system: Linux 5.5.10-200.fc31.x86_64 #1 SMP Wed Mar 18 14:21:38 UTC 2020 x86_64

Webserver: Apache/2.4.43 (Fedora) OpenSSL/1.1.1d (fpm-fcgi)

Database: mysql 10.3.22

PHP version:

7.3.16
Modules loaded: Core, date, libxml, openssl, pcre, zlib, filter, hash, Reflection, SPL, session, standard, cgi-fcgi, bz2, calendar, ctype, curl, dom, mbstring, fileinfo, ftp, gd, gettext, iconv, imap, intl, json, ldap, exif, mysqlnd, PDO, Phar, posix, shmop, SimpleXML, sockets, sqlite3, sysvmsg, sysvsem, sysvshm, tokenizer, xml, xmlwriter, xsl, mcrypt, mysqli, pdo_mysql, pdo_sqlite, wddx, xmlreader, apcu, igbinary, imagick, msgpack, pcov, pdlib, smbclient, zip, redis, libsmbclient, Zend OPcache

Nextcloud version: 18.0.2 - 18.0.2.2

Updated from an older Nextcloud/ownCloud or fresh install:

Where did you install Nextcloud from:

Signing status
List of activated apps

Configuration (config/config.php)

Are you using external storage, if yes which one:

Are you using encryption:

Are you using an external user-backend, if yes which one:

Client configuration

Browser: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:74.0) Gecko/20100101 Firefox/74.0

Operating system:

Logs

Web server error log
Insert your web server log here 
Nextcloud log
Insert your Nextcloud log here
Browser log

Insert your browser log here, this could for example include:

a) The javascript console log
b) The network log
c) ...
@kesselb
Copy link
Contributor

@kesselb kesselb commented Apr 13, 2020

Good finding 👍

image

It seems to work with es.

image

That seems to be the issue. The function assumes that the language is es_AR but es-AR is passed. I'm not sure why the language is es-AR or where this value is coming from ;)

cc @gary-kim @nextcloud/javascript

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Apr 13, 2020

We updatet the lang list recently iirc?
Maybe before they made sure to convert everything to _ and not - ?
Or the other way around :p
Anyway, simple fix I guess?

@kesselb
Copy link
Contributor

@kesselb kesselb commented Apr 13, 2020

image

It's correct over there 🤷‍♂️

@kesselb
Copy link
Contributor

@kesselb kesselb commented Apr 13, 2020

e5be299 That's the commit / code changing es_AR to es-AR. It's like that since Nextcloud 14 so changing the _getPlural function should do it.

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Apr 13, 2020

my locale update pr #17222

@matiasdelellis
Copy link
Author

@matiasdelellis matiasdelellis commented Apr 14, 2020

Well, I guess is difficult to discuss about translations to Transifex, but seem that they use underscores to separate counties from translations, which seems wrong. I don't know what would be the right thing ..

@MorrisJobke
Copy link
Member

@MorrisJobke MorrisJobke commented Apr 15, 2020

@skjnldsv There was a reason why it is - instead of _ but I can't find it right now. For the transformation it might also help to just specify it here: https://github.com/nextcloud/server/blob/master/.tx/config#L3 ?

@MorrisJobke
Copy link
Member

@MorrisJobke MorrisJobke commented Apr 15, 2020

@skjnldsv There was a reason why it is - instead of _ but I can't find it right now. For the transformation it might also help to just specify it here: https://github.com/nextcloud/server/blob/master/.tx/config#L3 ?

Keep in mind that this then needs to be updated in every single app out there. So fixing the JS part to look for the transifex version of the string might be the easier solution.

@matiasdelellis

This comment was marked as outdated.

@kesselb
Copy link
Contributor

@kesselb kesselb commented Apr 15, 2020

I'm confused now. Are we still talking about the same thing?

#8217 changed es_AR to es-AR. The reason is that https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/lang expects the language tag as defined at https://www.ietf.org/rfc/bcp/bcp47.txt.

What needs to done is to check the usages of OC.getLanguage() and other places using the lang attribute and make sure the code expects - instead of _. Fixing _getPlural might be a start.

@welaq

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.