[4.0] Multilingual : Correcting info-block category and parent category urls by infograf768 · Pull Request #21916 · joomla/joomla-cms

infograf768 · 2018-08-30T10:24:22Z

Problem: on a multilingual site, when displaying the info-block for articles in frontend , the urls of the category and parent category are real bad.

Summary of Changes

ContentHelperRoute::getCategoryRoute() should ALWAYS contain the $language variable.
This PR adds it wherever necessary, including in layouts/joomla/content/info-block/ corresponding files. Queries are modified to also select the category language. The language is also added in data.
com_content/helpers/route.php should cope with categories set to ALL languages
The code is modified to take this into account as, in this case, the language to use in the url should be the language of the UI content language.
com_content/helpers/route.php should not add for example &layout=cassiopeamyblog to the end of the sef url when such a specific html/com_content/category/myblog.etc menu item type is present in the template.
To solve this, I had to revert [com_content][Multilanguage] - fix link when layout and association #19681

Testing Instructions

Install a multilingual site with 2 content languages.
Tests can be done on a single language. Choose en-GB for example as it is easier.

Create some categories and articles in these categories both tagged to the same language [CATen-GB, CATfr-FR].
Create a home blog menu item for each content language to display one CATen-GB and one CATfr-FR
Make sure you create a (hidden or not) List All Categories menu item tagged to en-GB in mainmenu en-GB.
=> This is important as any category which has no specific menu item will still get a nice sef url.

Create also a Category set to ALL language and a child category of that category (tagged to ALL or to en-GB). Add to the child category an article tagged to en-GB ("Articletest").
Create a single menu item in mainmenu en-GB to display the "articletest" article.

Now, the second part of the test concerns the revert of the layout code in route.php.
To test this, it is necessary to add a new menu item type.
Find here a .zip of the files to add in /templates/cassiopeia/html/com_content/category/
myblog.zip

Create a menu item using this type in mainmenu en-GB to display any category not yet displayed by the former menu items.

In Articles Options, make sure show category and link as well as show Parent category and link are set to yes.
Make sure all your menu items use these global settings

Expected result

When clicking on the Category or Parent category links you should get nice urls
not containing some stuff like
?view=category&id=18
or
&layout=cassiopeamyblog when using the custom menu item type

Instead one may get for example when no menu item for the category
/en/allcategories-en-gb/category-all (here set to ALL)
or
/en/allcategories-en-gb/category-all/othercaten-gb (category set to en-GB)

and same for the custom menu item type (myblog menu item type)

Note A

Reverting #19681 is necessary.
We should look for another way to implement this feature.

Note B

The modifications concerning getCategoryRoute() and route.php should also be done for other components using this method.

@alikon @Hackwar @csthomas

Hackwar

We need these changs for the other core components, too.

Hackwar · 2018-08-30T14:08:34Z

components/com_content/View/Archive/HtmlView.php

+			$item->catslug        = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
+			$item->parent_slug    = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
+			$item->catlanguage    = $item->category_language;
+			$item->parentlanguage = $item->parent_language;


Why are you adding these attributes again here? We don't need to duplicate these attributes.

We also don't need the slugs for the categories. For the categories we only need the ID.

Why are you adding these attributes again here? We don't need to duplicate these attributes.

Looks like they are needed for the info-block JLayout

We also don't need the slugs for the categories. For the categories we only need the ID.

I was adapting to the info-block layout which uses slugs.

Hackwar · 2018-08-30T14:09:44Z

components/com_content/View/Archive/HtmlView.php

 			if ($item->parent_alias === 'root')
 			{
-				$item->parent_slug = null;
+				$item->parent_slug    = null;


Can you revert this change here? Don't want to have unnecessary changes...

Hackwar · 2018-08-30T14:10:12Z

components/com_content/View/Article/HtmlView.php

+		$item->catslug        = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
+		$item->catlanguage    = $item->category_language;
+		$item->parent_slug    = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
+		$item->parentlanguage = $item->parent_language;


Again, don't duplicate the attributes and we don't need the slugs.

Hackwar · 2018-08-30T14:10:54Z

components/com_content/View/Category/HtmlView.php

+			$item->catslug        = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
+			$item->catlanguage    = $item->category_language;
+			$item->parent_slug    = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
+			$item->parentlanguage = $item->parent_language;


Again, don't duplicate the attributes and we don't need the slug.

Hackwar · 2018-08-30T14:11:06Z

components/com_content/View/Featured/HtmlView.php

+			$item->catslug        = $item->category_alias ? ($item->catid . ':' . $item->category_alias) : $item->catid;
+			$item->parent_slug    = $item->parent_alias ? ($item->parent_id . ':' . $item->parent_alias) : $item->parent_id;
+			$item->catlanguage    = $item->category_language;
+			$item->parentlanguage = $item->parent_language;


See comment above...

Hackwar · 2018-08-30T14:15:22Z

components/com_content/helpers/route.php

+				{
+					$lang  = Factory::getLanguage()->getTag();
+					$link .= '&lang=' . $lang;
+				}


The $lang changes should not be necessary. It defaults to the current language if no language is given. Did you check if this is necessary? The $layout is indeed wrong here. In any case, you should change the fingerprint of these methods to not have the $language (and also the $catid) be optional anymore. These are always required.

The $lang changes should not be necessary. It defaults to the current language if no language is given. Did you check if this is necessary?

Alas it is. Tested that. Specially for the info-block at least.

In any case, you should change the fingerprint of these methods to not have the $language (and also the $catid) be optional anymore. These are always required.

Any hint how to do that? And would it be B/C ?

Simply remove the optional parameter stuff here. So from

public static function getCategoryRoute($catid, $language = 0)

to

public static function getCategoryRoute($catid, $language)

This is not B/C, but we are talking about Joomla 4.0 here, where we can break B/C and this is a case where we have to break it, to clear up this longlasting bug.

Also, please remove the $lang changes. We should never have to add something if the language is *. If this currently is necessary, we have to fix this in another place, but not here. Not only because it is wrong, but also because this method is just an amenity, not a necessity. These methods only concatenate a string, nothing more, they don't look up anything. People can also just hand in their own handcrafted URL into the router, which could not contain the language parameter. So we have to see what the issue is here in the router and fix that and not hand in fake parameters. This code has to go.

Also, please remove the $lang changes.

My problem here is that if I remove it, full tests can't be done on this PR as soon as we have a category set to ALL languages.

This is not B/C, but we are talking about Joomla 4.0 here, where we can break B/C and this is a case where we have to break it, to clear up this longlasting bug.

I would also do for getArticleRoute() I guess
I am not opposed to it. Need OK from
@wilsonge @mbabker

I have a solution for the $lang stuff. Will add later on.

infograf768 · 2018-08-31T05:04:10Z

On it for most parts except the fingerprint of the method

infograf768 · 2018-08-31T06:56:44Z

@Hackwar
Got rid of slugs for category and parent category. Modified info-block to fit.
Did not modify getCategoryRoute() method. I guess this can/should be done in another PR.

After merge, will work on other components.

infograf768 · 2018-08-31T15:12:17Z

@Hackwar
Set the $language to $active->language when multilang and item tagged to ALL in the Router, therefore getting rid of $lang in route.

csthomas · 2018-08-31T15:51:23Z

components/com_content/helpers/route.php

+				if ($language !== '*')
+				{
+					$link .= '&lang=' . $language;
+				}


Can you undo changes that do not do anything in this file, I mean just delete the part with the layout.

infograf768 · 2018-08-31T17:02:02Z

after some further tests, i think my changes in menurules are not enough.

csthomas · 2018-09-01T09:02:56Z

libraries/src/Component/Router/Rules/MenuRules.php

+		// Set the language to the current one when multilang is enabled and item is tagged to ALL
+		if (Multilanguage::isEnabled() && $language === '*')
+		{
+			$language = Factory::getLanguage()->getTag();


This should work too $language = $this->router->app->get('language');

It does indeed. More in the style of that file than the present code. Changing now. :)

infograf768 · 2018-09-01T15:05:31Z

restarted drone as non-related error

csthomas · 2018-09-01T19:49:50Z

The code is OK. I wonder if this issue occurs on J3?

csthomas · 2018-09-01T20:07:13Z

I have tested this item ✅ successfully on e06a014

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21916.}

zero-24 · 2018-09-04T18:57:27Z

I have tested this item ✅ successfully on e06a014

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21916.}

laoneo · 2018-09-06T05:56:20Z

Thanks

@csthomas

…ry urls (joomla#21916) * [4.0] Correcting info-block category and parent category urls * Getting rid of category slugs thanks Hannes * missing one cat language variable * Correcting routing for archived * Getting the active language when item is tagged to ALL * Load current language when item is set to ALL * factory not used * better code for menurules thanks @csthomas

[4.0] Correcting info-block category and parent category urls

299689a

joomla-cms-bot added the PR-4.0-dev label Aug 30, 2018

infograf768 mentioned this pull request Aug 30, 2018

[com_content][Multilanguage] - fix link when layout and association #19681

Merged

Hackwar requested changes Aug 30, 2018

View reviewed changes

Getting rid of category slugs thanks Hannes

e50668a

infograf768 added 2 commits August 31, 2018 10:05

missing one cat language variable

c1d382f

Correcting routing for archived

c21eb52

infograf768 force-pushed the 4.0-infoblock branch from 2ef6a95 to c21eb52 Compare August 31, 2018 09:34

infograf768 mentioned this pull request Aug 31, 2018

[4.0] Re-add archive in the frontend #21701

Merged

Getting the active language when item is tagged to ALL

2141933

infograf768 mentioned this pull request Aug 31, 2018

[4.0] Fix error 404 in modern routing for links without own menu item (NomenuRules) #20979

Closed

csthomas reviewed Aug 31, 2018

View reviewed changes

infograf768 added 2 commits September 1, 2018 05:47

Load current language when item is set to ALL

70f455e

factory not used

f3dc6e9

csthomas reviewed Sep 1, 2018

View reviewed changes

better code for menurules thanks @csthomas

e06a014

zero-24 added the RTC This Pull Request is Ready To Commit label Sep 4, 2018

zero-24 added this to the Joomla 4.0 milestone Sep 4, 2018

laoneo merged commit 169a8e1 into joomla:4.0-dev Sep 6, 2018

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 6, 2018

infograf768 deleted the 4.0-infoblock branch September 6, 2018 05:56

infograf768 mentioned this pull request Sep 6, 2018

[4.0] Follow-up adding category language to getCategoryRoute() #22022

Merged

Uh oh!

Conversation

infograf768 commented Aug 30, 2018

Summary of Changes

Testing Instructions

Expected result

Note A

Note B

Uh oh!

Hackwar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infograf768 Aug 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infograf768 Aug 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infograf768 commented Aug 31, 2018

Uh oh!

infograf768 commented Aug 31, 2018

Uh oh!

infograf768 commented Aug 31, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infograf768 commented Aug 31, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infograf768 commented Sep 1, 2018

Uh oh!

csthomas commented Sep 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

csthomas commented Sep 1, 2018

Uh oh!

zero-24 commented Sep 4, 2018

Uh oh!

laoneo commented Sep 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

infograf768 Aug 30, 2018 •

edited

Loading

infograf768 Aug 31, 2018 •

edited

Loading

csthomas commented Sep 1, 2018 •

edited

Loading