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

Pattern matching for detecting language .po files needs updating #3974

Closed
stpaultim opened this issue Aug 10, 2019 · 30 comments
Closed

Pattern matching for detecting language .po files needs updating #3974

stpaultim opened this issue Aug 10, 2019 · 30 comments

Comments

@stpaultim
Copy link
Member

stpaultim commented Aug 10, 2019

Description of the bug

I just saw this in the forum and wanted to report it here. I have not confirmed this since I've never installed Backdrop CMS in another language, so I'm not sure what to expect.

Hoping that someone else might be able to confirm whether or not this is a real problem or possible user error/misunderstanding.

Apparently the latest update (to 1.13.3) ruined the ability to install backdrop in another language. It worked ok in previous version.

I am using Danish.

There is no other language other than english no matter what I do before install. Installing Danish .po file (etc.) after install doesn't worh either . nothing gets translated (yes I clear cache etc.).

Steps To Reproduce
<anyone can add/edit this>

Actual behavior
<anyone can add/edit this>

Expected behavior
<anyone can add/edit this>

Additional information
<anyone can add/edit this>


PR by @klonos: backdrop/backdrop#2815

@findlabnet
Copy link

Potential system installation bug is installing English as additional language when user select another languages.

I did clean install using .po files from Drupal placed to files/translations and have option select from provided languages plus English. After finish non-English install I find my site have two enabled languages, one of them is English, but site is not configured for multi-language behavior, e.g. detecting/selecting and so on. Disabling then removing English from languages list is possible solution for creating non-English one-language site.

@klonos
Copy link
Member

klonos commented Aug 10, 2019

I took a look at the changelog for 1.13.3, but nothing stroke me as potential culprit. I need to test to be able to confirm.

@findlabnet I don't think that English could/should be removed. It is the default/fallback language, since most translations are not 100% complete ...furthermore, with new releases changing/add/removing English strings, we cannot expect language translations to be up to date at the time of the release.

but site is not configured for multi-language behavior

That is a bug though (or missing functionality).

Disabling then removing English from languages list is possible solution for creating non-English one-language site.

I have never actually tried this with either Drupal or Backdrop. I wasn't aware that it was possible. AFAIK, the English language exists as whatever strings are within t() function statements in the Backdrop CMS code; it is no a .po file that you can remove. I am curious now 😄

@findlabnet
Copy link

@klonos it is possible, but you right:

  • removing English is bad idea if we want use "User interface translation" via corresponded admin page (admin/config/regional/translate);
  • disabled (but not removed) English should allow do such translations.

Anyway, one-language site can use localized strings on theme level without using t() at all.

@Egmund
Copy link

Egmund commented Aug 10, 2019

To clarify: I am the author of this problem.
In 1.13.2 it worked fine. I could select language at install and did not have to disable English after install. Now there is no other option than installing in English first, then add language and then disable English + clearing cache. Not good.

@indigoxela
Copy link
Member

I can confirm that it's not possible (anymore) to install Backdrop in another language. The installer only shows english, no matter what's in the "translations" directory.

But this definitely didn't happen in the latest release, because 1.13.2 installer behaves the same.
It works fine in 1.11.6. Not sure, when/where it broke.

@Egmund
Copy link

Egmund commented Aug 10, 2019

Anyway - after 4 installs I do have a functioning site, but I feel I can no longer recommend this project (buggy, misleading manuals).

@indigoxela
Copy link
Member

@Egmund you found a bug, that's a good thing, because now we can fix it.

You're welcome to help out with testing, as soon as there's a fix for this problem.

@indigoxela
Copy link
Member

What an easy fix! The regex for file_scan_directory() must get more flexible.

core/includes/install.core.inc
$files = file_scan_directory($directory, '!(install|drupal-\d+\.\d+|backdrop(cms)?-\d+\.\d+\.\d+)\.' . (!empty($langcode) ? preg_quote($langcode, '!') : '[^\.]+') . '\.po$!', array('recurse' => FALSE));

The files provided by https://localize.backdropcms.org/translate/downloads need a slightly different pattern, as they have one digit less than drupal's po files.

An example: backdropcms-1.13.de.po

I can easily switch the availabilty of a language in the installer on/off by simply renaming it from backdropcms-1.13.de.po to backdropcms-1.13.0.de.po and back.

It should be sufficient to make the last digit in the regex optional.

@Egmund
Copy link

Egmund commented Aug 10, 2019

Cool, thank you.

@klonos
Copy link
Member

klonos commented Aug 10, 2019

I am sure that we fixed this in the past. @olafgrabienski ??

@klonos
Copy link
Member

klonos commented Aug 10, 2019

...yup, it was back in 1.12.7: #3674. Back then I recommended a more robust regex: #3674 (comment)

@klonos
Copy link
Member

klonos commented Aug 10, 2019

@Egmund I understand your frustration, but as @indigoxela pointed, Backdrop is not perfect. Reporting bugs and helping troubleshoot, and finally fix them is the natural process of evolution of all open source projects. We rely on members of the community such as yourself to help us improve the product. I hope that this experience was not as bad to drive you away indefinitely.

@Egmund
Copy link

Egmund commented Aug 10, 2019

I will 'stick around'. Had several bad experiences w. backdrop, but otherwise impressed.
I am concerned though that it is a viable project. Hope more D7 users will take it up before D7 dies.
Are we enough to keep backdrop alive?

@klonos
Copy link
Member

klonos commented Aug 10, 2019

I think we are. People have been worrying that the project will die, yet 5+ years later, here we are @Egmund 🙂

@klonos
Copy link
Member

klonos commented Aug 10, 2019

I have tried this in https://regex101.com: (install|drupal|backdrop(cms)?)(-[\d\.]+)?\.

I have tried some random combinations of possible file names:

install.xx.po
backdrop.xx.po
drupal.xx.po
backdropcms.xx.po
install-1.xx.po
backdrop-1.xx.po
drupal-1.xx.po
backdropcms-1.xx.po
install-1.23.xx.po
backdrop-1.23.xx.po
drupal-1.23.xx.po
backdropcms-1.23.xx.po
install-1.23.456.xx.po
backdrop-1.23.456.xx.po
drupal-1.23.456.xx.po
backdropcms-1.23.456.xx.po

...and they all seemed to work:

Screen Shot 2019-08-10 at 3 43 42 pm

@olafgrabienski ^^ 😉

Also here's a PR using that regex: backdrop/backdrop#2815

@klonos
Copy link
Member

klonos commented Aug 10, 2019

@Egmund @indigoxela for the record, can you please provide the filenames of the .po files that failed to be detected for you? Thanks.

@klonos klonos changed the title Problem installing in another language after v. 1.13.3? Problem installing in another language Aug 10, 2019
@klonos
Copy link
Member

klonos commented Aug 10, 2019

...I have made the issue title, since this seems to not be version-specific.

@olafgrabienski
Copy link

@klonos I think to remember that the release names on the localization server were changed at some point. That could be the reason for the current issue. I‘m not at the computer now, will try to clarify in the next days.

@Egmund
Copy link

Egmund commented Aug 10, 2019

The file is backdropcms-1.13.da.po
Comes from the backdrop lokalization server

@klonos
Copy link
Member

klonos commented Aug 10, 2019

Thanks @Egmund 👍 ...the change in the regex that detects .po files that was made in #3674 made it so that the expected file name would be exactly 3 digits for the version. The PR I have filed makes it so that any number (0 or more) of digits and dots are accepted as part of the .po filename, which should be more future-proof.

@Egmund
Copy link

Egmund commented Aug 10, 2019

  • as long as it is now-proof.

For my next site: What do I need to do? (sorry, it is a little unclear for me)

@klonos
Copy link
Member

klonos commented Aug 10, 2019

@Egmund you can apply the fix in the install.core.inc file as in this pull request, but the easiest thing for you would be to rename the language file to something like backdropcms-1.13.0.da.po (add a 0, or any number, so that the version number in the file name has 3 digits).

@herbdool
Copy link

@klonos I checked the regex in a tester too and looks good. I'll wait for someone to test it with real files before marking it ready to be committed.

@Egmund
Copy link

Egmund commented Aug 11, 2019

I can confirm that the fix works. I have patched the install.core.inc and I can install in Danish - do not even have to disable English afterwards. The test site is http://haurdalhus.dk/home
Thank you all for the very quick response and good help.

@indigoxela
Copy link
Member

Works like a charm.

I applied the changes from klonos:patch-187 to a fresh backdrop 1.13.3 and put some translation files in "translations" directory:

  • backdropcms-1.13.de.po
  • drupal-7.67.fr.po
  • install.it.po

With the patch applied, all languages are available in the installer, without the patch german (backdropcms-1.13.de.po) is missing.

I like the more generic regex, it really makes sense. It's ready to get merged from my point of view.

@klonos
Copy link
Member

klonos commented Aug 11, 2019

Thanks for testing that guys 👍

@herbdool I should have mentioned that I tested it on my local with actual .po files (actually the same blank file, duplicated multiple times, and renamed to different combinations of the possible patterns). I did not actually go forth with the installation, but I confirmed that the languages were detected and available in the "Select language" drop-down menu.

PS: I have a few follow-ups from my testing (but those are not directly related to the problem at hand here)...

@olafgrabienski
Copy link

I can easily switch the availabilty of a language in the installer on/off by simply renaming it from backdropcms-1.13.de.po to backdropcms-1.13.0.de.po and back.

I think to remember that the release names on the localization server were changed at some point. That could be the reason for the current issue. I‘m not at the computer now, will try to clarify in the next days.

Indeed, the release names have been changed, see backdrop-ops/localize.backdropcms.org#27 (comment) and below.

TL;DR: First we had language releases for Backdrop minor and bugfix versions, then we decided to only provide language releases for Backdrop minor versions. That's why the releases were renamed to something like 1.13 instead of 1.13.0, leading to respective file names.

Unfortunately, we didn't think about the installer when the change was introduced. Sorry for that!

@klonos
Copy link
Member

klonos commented Aug 14, 2019

...we decided to only provide language releases for Backdrop minor versions.

Well, we do update strings between bug releases, but if we can have multiple versions of a 1.13 translation, then I guess it's fine.

@olafgrabienski
Copy link

@klonos FYI: backdrop-ops/localize.backdropcms.org#27 (comment)

@quicksketch
Copy link
Member

Thanks everyone here for the discussion and working to solve the issue. I really appreciate the thorough testing on these patterns, because reading the code it wasn't clear to me what the change would be. Merged backdrop/backdrop#2815 into 1.x and 1.13.x. This will be in 1.13.4.

@quicksketch quicksketch added this to the 1.13.4 milestone Aug 18, 2019
@quicksketch quicksketch changed the title Problem installing in another language Pattern matching for detecting language .po files needs updating Aug 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants