Skip to content

Comments

[4.0] Flags are country flags, not language flags#28575

Closed
Bakual wants to merge 6 commits intojoomla:4.0-devfrom
Bakual:40_UseCountryFlag
Closed

[4.0] Flags are country flags, not language flags#28575
Bakual wants to merge 6 commits intojoomla:4.0-devfrom
Bakual:40_UseCountryFlag

Conversation

@Bakual
Copy link
Contributor

@Bakual Bakual commented Apr 4, 2020

When installing a language, the corresponding content language is automatically created and also an image ("flag") associated with it.

However the logic for the language flags applied there is no proper logic at all: The issue is that there exists no language flags, they are country (or district) flags.

When a new official language is created by our translation teams, we manually have to upload a new image matching the new language code. For example for the german dialects this was done in a PR (see #11868).

Original PR was #12014. Feel free to read through the lengthy discussion there.

Summary of Changes

This PR suggest to change the logic and assign the flags based on the country code in our language codes. Eg for de-CH (Swiss German) it would take the image ch.gif instead of de-ch.gif.
To still allow language specific images, it will first look for an existing file with the language code and fall back to the country code.

This PR also removes all now unneeded (since duplicated) files and renames some:

33 files are renamed to their corresponding country code.
75 files are deleted because they were duplicates and are no longer needed.
6 files are renamed to a name which already existed with a wrong image before. Those are si.gif, ca.gif, br.gif, be.gif, at.gif and af.gif. This accounts for a small B/C break for people who used those images before, as they will now change. Solution is simple: The user needs to select the image fresh in the content language.

Testing Instructions

  • Install a language and check that the created content languages have proper flags for the language / country they are referring.

Documentation Changes Required

None that I am aware of.

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Apr 4, 2020
@infograf768
Copy link
Member

Clean install from your branch: problems for en-GB
Screen Shot 2020-04-05 at 09 08 19

needs change from en_gb to gb in base.sql as well as in update I guess.

INSERT INTO `#__languages` (`lang_id`, `lang_code`, `title`, `title_native`, `sef`, `image`, `description`, `metakey`, `metadesc`, `sitename`, `published`, `access`, `ordering`) VALUES
(1, 'en-GB', 'English (en-GB)', 'English (United Kingdom)', 'en', 'en_gb', '', '', '', '', 1, 1, 1);

@infograf768
Copy link
Member

Suggestion:
Set to empty the image if none is available.

		$image = strtolower(str_replace('-', '_',  $tag));

		if (!file_exists(JPATH_ROOT . '/media/mod_languages/images/' . $image . '.gif'))
		{
			// Use country (eg "gb") as the image.
			$image = strtolower(substr($tag, strpos($tag, '-') + 1));
			
			// Set to none if no image is available
			if (!file_exists(JPATH_ROOT . '/media/mod_languages/images/' . $image . '.gif'))
			{
				$image = '';
			}
		}

@Bakual
Copy link
Contributor Author

Bakual commented Apr 5, 2020

Ah, good find. Will do that today!
For updates I would not delete the now unneeded files. If we delete them a lot more users would face a B/C issue which isn't needed at all. So for updates we also don't have to change the SQL.
@wilsonge Is it possible to exclude this folder from your script which collects removed files?

@brianteeman
Copy link
Contributor

By not removing images only on new installs you create a scenario whereby on one site (an upgrade) you select a flag of de_DE.gif and on another site (clean install) you select a flag of de.gif

On the other hand for upgraded sites that have been set to use de_DE.gif and you do remove the image on upgrade then you get a 404 on the image.

@Bakual
Copy link
Contributor Author

Bakual commented Apr 5, 2020

By not removing images only on new installs you create a scenario whereby on one site (an upgrade) you select a flag of de_DE.gif and on another site (clean install) you select a flag of de.gif

Which is no problem. Or do I miss something?

@Bakual
Copy link
Contributor Author

Bakual commented Apr 5, 2020

Updated with findings from JM

@brianteeman
Copy link
Contributor

Which is no problem. Or do I miss something?

I am not a fan of different settings based on an upgrade or a clean install - personal opinion - others may disagree

@wilsonge
Copy link
Contributor

@wilsonge Is it possible to exclude this folder from your script which collects removed files?

Yes. Add it into this array and we are good https://github.com/joomla/joomla-cms/pull/25559/files#diff-364166631f7d7383b58d579f097b86a2R63

@Bakual Bakual force-pushed the 40_UseCountryFlag branch from fd25811 to 9b5da1e Compare April 11, 2020 07:39
@coolcat-creations
Copy link
Contributor

I tested with Persian which worked but when I try to install for example fr-CA, en-us,... the language packages can't be found ...?

@alikon
Copy link
Contributor

alikon commented Apr 25, 2020

iirc at the moment there are only 3 languages ready for j4
french , german and persian
ping @infograf768 for confirmation

@infograf768
Copy link
Member

They are not "ready" for J4. Their 3.9 versions are only available for installing in J4 at J4 install time and through the Install Languages interface.
This was decided quite a while ago, to be able to test multiligual sites and RTL display in J4.

@adj9
Copy link

adj9 commented Apr 25, 2020

I have tested this item ✅ successfully on 9b5da1e

J! 3.9.16

Step

  • Multilanguage activation
  • Installation of French and Persist, English was already there
  • Applying the Patch from the component Your text to link here...
  • Creation of Art1 Art2 - eng-UK, Art3, Art4 - persian and Art5 - fre

Schermata 2020-04-25 alle 12 43 03

@obuisard
Copy link
Contributor

There are not enough languages to test this successfully. Ideally, we should be able to install a language available for different countries. For instance, French from France and French from Canada. Depending on the language file installed, we should get either the French flag or the Canadian flag.

@infograf768
Copy link
Member

@obuisard
You can install any language by fetching the packs on joomlacode and install as an extension.
http://joomlacode.org/gf/project/jtranslation3_x/frs/

@Bakual
Copy link
Contributor Author

Bakual commented Apr 25, 2020

You could manually download a package and install that zip like a regular extension.
The list of languages is here: https://community.joomla.org/translations/joomla-3-translations.html. French Canada here: http://joomlacode.org/gf/project/jtranslation3_x/frs/?action=FrsReleaseBrowse&frs_package_id=6871

Codewise it is the exact same thing that happens.

@Bakual
Copy link
Contributor Author

Bakual commented Apr 25, 2020

Oh, JM was faster 😄

@obuisard
Copy link
Contributor

Thank you!

@coolcat-creations
Copy link
Contributor

I tested now and I don't understand.
Before the patch I installed de-ch - created an article, applied de-ch to it. The language flag was de-ch.gif, afterwards I applied the patch and installed en-aus, en-us, ... and the image was en-aus.gif, en-us.gif ,... As far as I understood the test instructions it should be us.gif and so on after applying the patch ?

@infograf768
Copy link
Member

infograf768 commented May 2, 2020

@coolcat-creations
If you use patchtester, it may not fully work.
I can't also fully apply the patch with eclipse, i.e. the .gif are not renamed and existing .gif to be deleted are not deleted.
The only way in this case to test the PR is to install @Bakual branch https://github.com/Bakual/joomla-cms/archive/40_UseCountryFlag.zip

@Bakual
Copy link
Contributor Author

Bakual commented May 2, 2020

Aye, it doesn't delete existing images to keep being backward compatible.
So since de-ch and en-us still exist in your folder, it will take those.
If you manually delete the de-ch or en-us files prior to create the respective language, you should see it pick the ch or us file instead.

@brianteeman
Copy link
Contributor

#28575 (comment)

@richard67
Copy link
Member

Maybe I'm a bit late to the party, but I was pointed to this by the bugs and fun at home PR list.

2 questions:

  1. How does it come that @adj9 has tested this PR, which is for Joomla 4, on a Joomla 3.9.x? Is there something I'm missing?
  2. Is this PR abandoned, or will it be continued? @Bakual : Maybe it needs some decision by a release lead or whatever to see if it makes sense to continue so you know you're not wasting your time? If it's not abandoned you should fix the conflicts, then we could try to find testers.

@astridx
Copy link
Contributor

astridx commented Aug 1, 2020

Because languages don't match national borders, I think this PR is right. I will test it as soon as the conflicts are resolved.

@Bakual
Copy link
Contributor Author

Bakual commented Aug 1, 2020

I still think this PR would be useful. But I certainly don't have time to fix conflicts during the next week. However the conflict itself is minimal and easy to solve as it's only in the postgresql file (which has almost no impact).

A decision would certainly be welcome.

@richard67
Copy link
Member

@Bakual I can fix the conflict for you here via GitHub UI. But the conflict shows there has been made an error before with another PR which has been merged. Will have to make a separate PR to fix that.

@richard67
Copy link
Member

Conflicts solved.

@astridx
Copy link
Contributor

astridx commented Aug 1, 2020

I tested the following:

  1. git fetch origin pull/28575/head:40_UseCountryFlag
  2. git checkout 40_UseCountryFlag
  3. I deleted everything and run git reset --hard HEAD is now at 794fad4 Merge branch '4.0-dev' into 40_UseCountryFlag
  4. composer install and npm ci
  5. I checked the icons in build/media_source/mod_languages/images/ and in the media folder for mod_language. For German language I see only de.gif (no de_de.gif, de_ch.gif or de_at.gif).
  6. I made a new installation with Persian and Frensh
  7. I installed multilingual sample data
  8. I opened front end. The icons are fine. (So old way should be fine?)
  9. I added German language and everything was fine too.

Then

  1. I deleted configuration.php and made a new installation.
  2. I made a new installation without any additional language
  3. i installed the German language and published the German content language.
  4. I created sample data for the German and English language (article, categorie, menu).
  5. I enabled the System Language plugin and added the switcher module. The icons are fine.
  6. I added a new icon and renamed it to de-de.gif and after that in zzz.gif. Both times I could select the icon via content language and is was fine in the front end

Joomla Installer
Control Panel test Administration
خانه
Extensions Languages test Administration
Languages Edit Content Language test Administration


System Information
php: Linux astrid-TravelMate-5760G 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64
dbserver: mysql
dbversion: 5.7.31-0ubuntu0.18.04.1
dbcollation: utf8_general_ci
dbconnectioncollation: utf8mb4_general_ci
dbconnectionencryption:
dbconnencryptsupported: true
phpversion: 7.2.31-1+ubuntu18.04.1+deb.sury.org+1
server: Apache/2.4.29 (Ubuntu)
sapi_name: apache2handler
version: Joomla! 4.0.0-beta4-dev Development [ Mañana ] 29-July-2020 18:21 GMT
useragent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:79.0) Gecko/20100101 Firefox/79.0

@astridx
Copy link
Contributor

astridx commented Aug 1, 2020

I have tested this item ✅ successfully on 794fad4


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

@Bakual
Copy link
Contributor Author

Bakual commented Oct 19, 2020

Merged current 4.0-dev so hopefully tests pass

@ashvini77
Copy link

I have tested this item ✅ successfully on 7433fd3


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

@ChavanAnkitaMahesh
Copy link

Tested successfully


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

@ChavanAnkitaMahesh
Copy link

I have tested this item ✅ successfully on 7433fd3


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

@rdeutz rdeutz added RMDQ ReleaseManagerDecisionQueue RTC This Pull Request is Ready To Commit and removed RMDQ ReleaseManagerDecisionQueue labels Mar 15, 2021
@rdeutz
Copy link
Contributor

rdeutz commented Mar 18, 2021

I am closing this, seems not the perfect solution. Good would be a migration script that let us not have a b/c break. Thanks for the discusion and work on this.

@rdeutz rdeutz closed this Mar 18, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 18, 2021
@alikon
Copy link
Contributor

alikon commented Mar 18, 2021

p.s.

RLDQ 👍

@Bakual Bakual deleted the 40_UseCountryFlag branch March 18, 2021 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester RMDQ ReleaseManagerDecisionQueue

Projects

None yet

Development

Successfully merging this pull request may close these issues.