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

Refactoring helper of mod_languages #5952

Merged
merged 3 commits into from Feb 14, 2015

Conversation

Projects
None yet
6 participants
@Hackwar
Member

Hackwar commented Feb 2, 2015

This speeds up the helper of mod_languages a bit and reduces the amount of code.

Line 40-49: Instead of cycling through all menu items and checking each if it is a home menu item, we are cycling through all languages (which should be a lot less than the menu items) and only retrieve the home menu item per language.

Line 64 + 65: The option parameter in Joomla only allows ASCII characters and class names are case insensitive. At the same time, JPATH_COMPONENT_SITE is already sanitized and does not need to be sanitized again. This could prevent loading JString unnecessarily.

Line 106: All links have to be processed by JRoute, regardless of SEF URLs being enabled or not.

Line 110: We are already checking if the currently processed language is the currently active language. We can use that result and thus remove the call in former line 75.

Line 114ff: The Itemid of a home menu item is always removed and thus we don't need a special case for SEF/non-SEF URLs.

@jsubri

This comment has been minimized.

Contributor

jsubri commented Feb 12, 2015

upgraded from 3.3.6 to 3.4 beta3, with 5952 the side effect is the UK flag is now displayed on all FrontEnd pages when it was not the case in 3.3.6.

5952-flags

Use case below.

5952

I've only 1 unpublished article in English (that says: site not avail in English) to cover the unprobable case someone tries /index.php/en

With 5952 , /index.php/en returns a 404 while 3.3.6 return the "site not avail in English" article.

@infograf768

This comment has been minimized.

Member

infograf768 commented Feb 13, 2015

Confirming @jsubri findings

A content language flag is displayed in the module although it does not have any Home page.

@infograf768

This comment has been minimized.

Member

infograf768 commented Feb 13, 2015

Forgot to say that I do not get a 404 here (which is the correct way to do things) because I had a category and an article published both tagged to that content language without home page.
The page obtained by clicking on the flag is quite empty, showing modules tagged to ALL.

@Hackwar

This comment has been minimized.

Member

Hackwar commented Feb 13, 2015

Should be fixed. Please test again

@infograf768

This comment has been minimized.

Member

infograf768 commented Feb 13, 2015

Indeed, issue fixed here.

@jsubri

This comment has been minimized.

Contributor

jsubri commented Feb 13, 2015

Works for me, the language flag is not displayed anymore and the 404 behaviour is correct for unpublished article.

@Hackwar

This comment has been minimized.

Member

Hackwar commented Feb 13, 2015

So, do we have 2 successfull tests then?

@jsubri

This comment has been minimized.

Contributor

jsubri commented Feb 13, 2015

yes

@infograf768 infograf768 added the RTC label Feb 14, 2015

infograf768 added a commit that referenced this pull request Feb 14, 2015

Merge pull request #5952 from Hackwar/patch-20
Refactoring helper of mod_languages

@infograf768 infograf768 merged commit 9615fab into joomla:staging Feb 14, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@infograf768

This comment has been minimized.

Member

infograf768 commented Feb 14, 2015

Thanks.

@roland-d roland-d added this to the Joomla! 3.4.0 milestone Feb 14, 2015

@zero-24 zero-24 removed the RTC label Oct 14, 2015

@Hackwar Hackwar deleted the Hackwar:patch-20 branch Jan 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment