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

[4.0] Fix language strings causing false plurals in crowdin #35681

Conversation

richard67
Copy link
Member

@richard67 richard67 commented Sep 27, 2021

Pull Request for infograf768/j4localise#30 (comment) .

Summary of Changes

Language string constants ending with "_ONE" seem to trigger some kind of plural string generation for translations on Crodwin.

See infograf768/j4localise#30 (comment) .

To avoid this, we should not use language string constants ending with "_ONE" if they are not really plural strings.

Work in Progress (WiP): I have to complete testing instructions.

As soon as ready, I'll remove draft status from this PR.

Testing Instructions

Hint: It needs to run "npm ci" or "npm run build:js" after having applied the PR.

  1. Code review: Check if there are language strings in the CMS core having a key ending with "_ONE".
  2. In content history modal of some content having some history (i.e. more than 1 version), use the "Preview" button without having selected any version.
    Hint: You can use that button even if it visually appears to be disabled. But that's another issue to be solved with another PR.
  3. In the same modal, use the "Compare" button with only one version selected.
  4. Create a module of type "Tags - Similar" and check the available options in the "Match Type" drop-down on the first Tab "Module".

Actual result BEFORE applying this Pull Request

  1. Strings "COM_CONTENTHISTORY_BUTTON_SELECT_ONE" and "MOD_TAGS_SIMILAR_FIELD_ONE" found.

Screen Shot 2021-09-24 at 07 18 06

Screen Shot 2021-09-24 at 07 18 24

  1. Options "All", "Any" and "Half" are available.

Expected result AFTER applying this Pull Request

  1. None.

  2. Same as without this PR.

  3. Same as without this PR.

  4. Same as without this PR.

Documentation Changes Required

None.

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Sep 27, 2021
@brianteeman
Copy link
Contributor

You might need to check MOD_TAGS_SIMILAR_FIELD_ONE

@richard67
Copy link
Member Author

You might need to check MOD_TAGS_SIMILAR_FIELD_ONE

@brianteeman Yes, that's the one I wanna check. Do you think I should do them all in this PR, or should I better make separate PRs?

@brianteeman
Copy link
Contributor

if you are fixing the same problem then you can do them in the same pr. its a small enough a change

@richard67
Copy link
Member Author

@brianteeman Are you familiar with CrowdIn? I'm not. Do you know if it would be ok to change "MOD_TAGS_SIMILAR_FIELD_ONE" to "MOD_TAGS_SIMILAR_FIELD_ANY", or do you have an idea why it hasn't been named like that from the beginning on? Does ending with "_ANY" cause similar issues on CrowdIn? Or can we use that?

@brianteeman
Copy link
Contributor

Answering no and no idea to each question - sorry

@richard67
Copy link
Member Author

@brianteeman Other question: Do you know if it's still ok to have the deprecation comment above the line with the language string? Or should that be put to the end of the line with the string?

@brianteeman
Copy link
Contributor

End of the line please #34671

@infograf768
Copy link
Member

infograf768 commented Sep 27, 2021

"MOD_TAGS_SIMILAR_FIELD_ONE" to "MOD_TAGS_SIMILAR_FIELD_ANY", or do you have an idea why it hasn't been named like that from the beginning on? Does ending with "_ANY" cause similar issues on CrowdIn? Or can we use that?

Some languages may use ANY in their plural forms and crowdin may wonder there.

I suggest: MOD_TAGS_SIMILAR_FIELD_ONE_TAG

@brianteeman
Copy link
Contributor

Wont the plurals still be generated as they are still in the language file?

@richard67
Copy link
Member Author

Wont the plurals still be generated as they are still in the language file?

@brianteeman I think so. But what else can we do? We're not allowed just to remove the strings, are we?

@brianteeman
Copy link
Contributor

The arguments for not removing them was always something I did not agree with. It was so that you could use an old and outdated language file with the very latest version of joomla and/or in case the string was used by an extension which is unlikely to apply in this case.

The problem is that if you dont remove the strings then you aren't solving the problem (are you?)

@richard67
Copy link
Member Author

The problem is that if you dont remove the strings then you aren't solving the problem (are you?)

@brianteeman I solve the problem ... for Joomla 5.0.

@wilsonge Do you know what we shall do? Can we remove these strings?

@Bakual
Copy link
Contributor

Bakual commented Sep 27, 2021

I would just remove them as we're fixing a bug.
And chances are slim that the string is used by 3rd party.

@brianteeman
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on df3e40b

Doesnt fix the problem


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35681.

@richard67
Copy link
Member Author

@brianteeman l’ll update this PR tomorrow.

@richard67 richard67 changed the title [4.0] Fix content history language strings causing false plurals in crowdin [4.0] Fix language strings causing false plurals in crowdin Oct 3, 2021
@richard67 richard67 marked this pull request as ready for review October 3, 2021 11:00
@richard67
Copy link
Member Author

I've changed this PR so the bad strings are not deprecated but removed.

@brianteeman Please test again. Thanks in advance.

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 3b133cf


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35681.

1 similar comment
@infograf768
Copy link
Member

I have tested this item ✅ successfully on 3b133cf


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35681.

@infograf768
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35681.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 4, 2021
@bembelimen bembelimen merged commit 3d9cff7 into joomla:4.0-dev Oct 5, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 5, 2021
@bembelimen
Copy link
Contributor

Thx

@bembelimen bembelimen added this to the Joomla 4.0.4 milestone Oct 5, 2021
@richard67
Copy link
Member Author

Thanks all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants