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

[Bug]: "Delete file" and "Delete folder" context menu items not localised on v27.1.7 #43947

Closed
4 of 8 tasks
ostasevych opened this issue Mar 1, 2024 · 20 comments · Fixed by #44526
Closed
4 of 8 tasks
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 27-feedback bug

Comments

@ostasevych
Copy link

ostasevych commented Mar 1, 2024

⚠️ This issue respects the following points: ⚠️

Bug description

Files context menu items Delete file and Delete folder appear to be not localised after the last update from 27.1.6 to 27.1.7.

Steps to reproduce

  1. Click context menu and observe the issue.
    image

Expected behavior

Should be localised.

Installation method

None

Nextcloud Server version

26

Operating system

None

PHP engine version

None

Web server

None

Database engine version

None

Is this bug present after an update or on a fresh install?

None

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

No response

List of activated Apps

No response

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

in uk.js I observe the change of the format from the last version:
in 27.1.6:

    "Delete file" : "Вилучити файл",
    "Delete folder" : "Вилучити каталог",

in 27.1.7:

    "_Delete file_::_Delete files_" : ["Вилучити файл","Вилучити файли","Вилучити файли","Вилучити файли"],
    "_Delete folder_::_Delete folders_" : ["Вилучити каталог","Вилучити каталоги","Вилучити каталоги","Вилучити каталоги"],

Is this something related to?

@ostasevych ostasevych added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Mar 1, 2024
@solracsf solracsf changed the title [Bug]: Delete file and Delete folder context menu items appeared to be not localised after the update to 27.1.7 [Bug]: "Delete file" and "Delete folder" context menu items not localised on v27.1.7 Mar 2, 2024
@susnux
Copy link
Contributor

susnux commented Mar 8, 2024

My guess here: We take translations from master / stable28 were that string is pluralized due to batch actions being possible.
But we should update changes like that to older stable branches.

Maybe @skjnldsv knows more about the CI that updates the translations?

@skjnldsv
Copy link
Member

skjnldsv commented Mar 9, 2024

Is this actually translated on transifex?

@ostasevych
Copy link
Author

Is this actually translated on transifex?

Sure :)
https://app.transifex.com/nextcloud/nextcloud/translate/#uk/files/483799062?q=text%3A'delete+file'

@susnux
Copy link
Contributor

susnux commented Mar 9, 2024

As said my guess it that we update translations from current master -> so the string is now pluralized and thus the frontend translate function can not translate it anymore.

@skjnldsv
Copy link
Member

skjnldsv commented Mar 9, 2024

I don't remember how it works.
if that was the case, it would mean every non-master releases would have translations missing for years no?
I mean, we change a lot of apps and various strings...

@ostasevych
Copy link
Author

ostasevych commented Mar 9, 2024

"Delete file::Delete files" : ["Вилучити файл","Вилучити файли","Вилучити файли","Вилучити файли"],
"Delete folder::Delete folders" : ["Вилучити каталог","Вилучити каталоги","Вилучити каталоги","Вилучити каталоги"],

Is this formatting supported in 27.1.x?

@skjnldsv
Copy link
Member

@blizzz @nickvergessen do you know more?

@nickvergessen
Copy link
Member

Strings from all versions we translate are combined.

But the string looks a bit weird anyway as the number is not included?
Do you have a link to the respective source code?

@skjnldsv
Copy link
Member

skjnldsv commented Mar 10, 2024

28 and 29

return n('files', 'Disconnect storage', 'Disconnect storages', nodes.length)

return n('files', 'Delete file', 'Delete files', nodes.length)

27

var deleteTitle = (type && type === 'file')
? t('files', 'Delete file')
: t('files', 'Delete folder')

@susnux
Copy link
Contributor

susnux commented Mar 18, 2024

So yes this is a problem of combining the strings, if it was singular previously and then the same string is provided as plural option we can not use it anymore in the frontend. (Because the translation key is merged into a plural version and thus we can not detect it anymore).

Two fixes possible:

  1. Also change 27 and 26 to the plural version (easy)
  2. Adjust our translation logic to somehow figure out the plural key and try that one as well. (harder)

@nickvergessen
Copy link
Member

That being said, the strings in

return n('files', 'Disconnect storage', 'Disconnect storages', nodes.length)
are "undistinct" plurals.

So this could be changed to a "non-pluraled" string as they do not need to follow a dedicated plural rule as you basically want to say "Delete many folders" so something like this would work:

	if (isAllFolders(nodes)) {
		if (nodes.length === 1) {
			return t('files', 'Delete folder')
		}
		// We don't really want to count and show the number, so using generic plural instead of a pluraled-string
		return t('files',  'Delete folders')
	}

@nickvergessen
Copy link
Member

That being said, I think it's a bug from the tooling that it can not handle translations of the same string that is a plural

@skjnldsv
Copy link
Member

so something like this would work:

	if (isAllFolders(nodes)) {
		if (nodes.length === 1) {
			return t('files', 'Delete folder')
		}
		// We don't really want to count and show the number, so using generic plural instead of a pluraled-string
		return t('files',  'Delete folders')
	}

Far less clean though 🙃

@nickvergessen
Copy link
Member

Far less clean though 🙃

But what you do (translating a plural without a number in it) is confusing for translators of languages that have 6 plural forms. How should they guess that they need to put in the "undistinct" in all of them.

@camilasan
Copy link
Member

2. Adjust our translation logic to somehow figure out the plural key and try that one as well.

What is the plural key?

@nickvergessen
Copy link
Member

the English one is _singular_::_plural_

"_%n folder_::_%n folders_" : ["%n Ordner","%n Ordner"],

but this is really not the way to go. Just don't use plurals for this, unless you have the %n in it.

@ostasevych
Copy link
Author

Hi! How is that solved? In the recent 27.x upgrade I cannot find the updated localisation :(

@nickvergessen
Copy link
Member

It was fixed right after the release and backporting is still to be done next week

@ElSi-DVT
Copy link

ElSi-DVT commented Apr 2, 2024

Problem still exists in 27.1.8

@nickvergessen
Copy link
Member

yes, it will be solved with 27.1.9

or you replace your language files with the new ones from https://github.com/nextcloud/server/tree/stable27/apps/files/l10n

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 27-feedback bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants