? Composer Dependency Changed ? Pending

User tests: Successful: Unsuccessful:

avatar bembelimen
bembelimen
12 Mar 2022

Summary of Changes

Fix the language select default value when installing a fresh Joomla!

Testing Instructions

  • Use a browser where "en" is not the default language
  • Start the installation process of Joomla!

Actual result BEFORE applying this Pull Request

Although the language is discovered correctly, the "English" is displayed as the selected language:

grafik

Expected result AFTER applying this Pull Request

Correct language is selected:

grafik

avatar bembelimen bembelimen - open - 12 Mar 2022
avatar bembelimen bembelimen - change - 12 Mar 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 12 Mar 2022
Category Installation
avatar brianteeman
brianteeman - comment - 12 Mar 2022

woohoo!!

avatar brianteeman
brianteeman - comment - 12 Mar 2022

Can we use the same technique to do something that @wilsonge and I talked about for the language installation step which is to display the current installation language as the default option

avatar bembelimen
bembelimen - comment - 12 Mar 2022

Can we use the same technique to do something that @wilsonge and I talked about for the language installation step which is to display the current installation language as the default option

You mean, when I select de-DE, that this language pack is installed by default (when available) or is on top of the list?

avatar brianteeman
brianteeman - comment - 12 Mar 2022

There are good reasons for it not to be installed by default. For example you are installing in german but the site is for a french client.

I just mean that german is either at the top of the list or it stays in the same position but it is preselected as the default in that list.

Hope that makes sense

avatar bembelimen
bembelimen - comment - 12 Mar 2022

This method is not needed, to pre-select the installation language, we can just use the value of the dropdown:

$model   = new SetupModel;
$options = $model->getOptions();

print_r($options['language']);

Then we just have to sort it up here based on this value.

avatar brianteeman
brianteeman - comment - 12 Mar 2022

To be honest I never really looked to see how to do it. Something I can work on tomorrow

avatar richard67
richard67 - comment - 13 Mar 2022

Unrelated to this PR, but I've seen it when testing: Have you noticed that there are 2 entries "English (United Kingdom)" in the language selection? One of them should be "English (New Zealand)", but that's only clear when checking the HTML:

j4 2-test-pr-37252_before_2

Is this a known issue and already reported somewhere?

avatar richard67
richard67 - comment - 13 Mar 2022

I have tested this item successfully on cd2e2db

Works for me. However I'm not sure yet if the failing drone could be related.


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

avatar richard67 richard67 - test_item - 13 Mar 2022 - Tested successfully
avatar brianteeman
brianteeman - comment - 13 Mar 2022

no its a bug. If you look at the xml

<name>English (United Kingdom)</name>
<version>4.1.0</version>
<creationDate>February 2022</creationDate>
<author>Joomla! Project</author>
<copyright>(C) 2005 Open Source Matters, Inc.</copyright>
<license>GNU General Public License version 2 or later; see LICENSE.txt</license>
<files>
<filename>joomla.ini</filename>
</files>
<metadata>
<name>English (en-NZ)</name>
<nativeName>English (United Kingdom)</nativeName>

avatar richard67
richard67 - comment - 13 Mar 2022

no its a bug. If you look at the xml

@brianteeman Do you know if this has to be fixed here in the repo or on Crowdin?

avatar richard67
richard67 - comment - 13 Mar 2022

@bembelimen Drone failing again at the API tests at the installation step. Last time it was the system test at the same place, and the time before again the API test. Always at the installation after having selected the language. It seems it is not always failing, but often. Maybe the tests need a little adjustment?

avatar brianteeman
brianteeman - comment - 13 Mar 2022

no its a bug. If you look at the xml

@brianteeman Do you know if this has to be fixed here in the repo or on Crowdin?

sorry dont know

avatar richard67
richard67 - comment - 13 Mar 2022

no its a bug. If you look at the xml

@brianteeman Do you know if this has to be fixed here in the repo or on Crowdin?

sorry dont know

I've proposed the fix on Crowdin.

avatar brianteeman
brianteeman - comment - 13 Mar 2022

This is almost perfect but not quite - the problem is when the browser has both a language option and a language+country variant as for example in german

image

If you have one of the variants as your default browser language and joomla has that variant then it works perfectly

image

image

if you have one of the variants as your default browser language and joomla does not have any variants then it works perfectly

image

image

The problem is if you have one of the pure languages which joomla obviously does not have
OR if you have one of the variants and joomla has variants but not that one.

In this case Joomla offers you the last variant in the list
image

image

image

image

Is this solvable? I suspect its not and I am only reporting this just in case. It is definitely better than it was before this pr

avatar infograf768
infograf768 - comment - 14 Mar 2022

I have tested this item successfully on cd2e2db


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

avatar infograf768 infograf768 - test_item - 14 Mar 2022 - Tested successfully
avatar richard67
richard67 - comment - 14 Mar 2022

@bembelimen I'd set RTC now as it has 2 tests, but there is still the drone failure of which I'm not sure if it is related.

avatar laoneo laoneo - change - 20 Mar 2022
Status Pending Ready to Commit
avatar laoneo
laoneo - comment - 20 Mar 2022

RTC


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

avatar richard67
richard67 - comment - 20 Mar 2022

@laoneo Hmm, we still have the constantly failing installation in drone, due to which I was told I should not set RTC.

When @roland-d would do an update of the 4.2-dev branch to the current 4.1-dev branch (so-called "upmerge"), we could see if the fix in the installation language file for the duplicate "English (United Kingdom)" will help with the tests so it's clear they do not fail due to this PR (and beside that would help me with rebasing some other PRs from 4.1-dev to 4.2-dev).

avatar laoneo
laoneo - comment - 20 Mar 2022

Ok, restarted drone and it fails again. So time to remove RTC again.

avatar richard67 richard67 - change - 20 Mar 2022
Status Ready to Commit Pending
Labels Added: ? ?
avatar richard67
richard67 - comment - 20 Mar 2022

Back to pending.


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

avatar roland-d
roland-d - comment - 20 Mar 2022

@richard67 Upmerge done

avatar richard67
richard67 - comment - 20 Mar 2022

@roland-d Thanks. Will update the branch of this PR here now.

avatar richard67 richard67 - change - 20 Mar 2022
Labels Removed: ?
avatar richard67
richard67 - comment - 20 Mar 2022

Unfortunately it seems that it did not help with the installation failing at the first or sometimes the 2nd api test in drone.

avatar joomla-cms-bot joomla-cms-bot - change - 14 Apr 2022
Category Installation External Library Composer Change Installation
avatar bembelimen bembelimen - change - 14 Apr 2022
Labels Added: Composer Dependency Changed
avatar chmst chmst - change - 14 Apr 2022
Status Pending Ready to Commit
avatar chmst
chmst - comment - 14 Apr 2022

Re-set RTC, as tests are still valid.


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

avatar brianteeman
brianteeman - comment - 14 Apr 2022

RTC but with an unanswered bug ? #37252 (comment)

avatar bembelimen bembelimen - change - 14 Apr 2022
Labels Added: ?
avatar bembelimen
bembelimen - comment - 14 Apr 2022

That bug already exists in 3.x (and should be fixed for sure) but is not related to this PR.
As Joomla! looks only in the first two characters (de-DE => de) or some browser only send the first two characters (de) it takes the first "de" entry from the list. And as de-AT < de-DE it's not the Germany German one which is selected.

avatar brianteeman
brianteeman - comment - 14 Apr 2022

That bug already exists in 3.x (and should be fixed for sure) but is not related to this PR

But prior to this PR the language is never preselected.

avatar roland-d
roland-d - comment - 14 Apr 2022

@bembelimen

As Joomla! looks only in the first two characters (de-DE => de) or some browser only send the first two characters (de) it takes the first "de" entry from the list.

Where is this done?

avatar bembelimen
bembelimen - comment - 14 Apr 2022

That bug already exists in 3.x (and should be fixed for sure) but is not related to this PR

But prior to this PR the language is never preselected.

In 3.x it is, it was broken in 4.0 (which is fixed with this PR again) but the old behaviour is not improved.

avatar brianteeman
brianteeman - comment - 14 Apr 2022

Ok

avatar bembelimen
bembelimen - comment - 14 Apr 2022

@bembelimen

As Joomla! looks only in the first two characters (de-DE => de) or some browser only send the first two characters (de) it takes the first "de" entry from the list.

Where is this done?

$primary_browserLang = substr($browserLang, 0, 2);

avatar laoneo
laoneo - comment - 22 Apr 2022

Is the change in the composer lock file correct?

avatar roland-d
roland-d - comment - 23 Apr 2022

Removed RTC as the pull request is still pending.

avatar Hackwar
Hackwar - comment - 23 Apr 2022

Yes, the change in composer lock is correct. That pulls the updated Joomla Browser version, which fixes the issue we had with the tests.

avatar laoneo
laoneo - comment - 24 Apr 2022

Then this needs to be fixed, before it can be merged.

avatar Hackwar
Hackwar - comment - 24 Apr 2022

What needs to be fixed?

avatar laoneo
laoneo - comment - 24 Apr 2022

The conflict

avatar richard67 richard67 - change - 30 Apr 2022
Labels Removed: ?
avatar richard67
richard67 - comment - 30 Apr 2022

I've solved the conflicting composer.lock file. The version from this PR was the right one because newer than the one from #37565 .

RTC is still ok, it doesn't need new human tests as all changes after these were only for the automated tests (Joomla browser update) and clean branch updates.

avatar laoneo laoneo - change - 1 May 2022
Labels Added: ?
avatar laoneo laoneo - close - 2 May 2022
avatar laoneo laoneo - merge - 2 May 2022
avatar laoneo laoneo - change - 2 May 2022
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-05-02 07:57:45
Closed_By laoneo
avatar laoneo
laoneo - comment - 2 May 2022

Thanks!

Add a Comment

Login with GitHub to post a comment