[com_content] Fix layout in category link by Septdir · Pull Request #20200 · joomla/joomla-cms

Septdir · 2018-04-18T20:37:31Z

Pull Request for Issue #20199.

Summary of Changes

Fix bag with ?layout= in com_content category link after #19681

Testing Instructions

If you have this bug, apply the patch

How reproduce bug by @Quy

Install the last demo.
Under System > Global Configuration > Articles > Integration > URL Routing, select Modern
Search Park Blog
Hover Park Blog link to see URL:
http://localhost/joomla387/index.php/using-joomla/extensions/components/content-component/article-category-blog?layout=

Expected result

Link was /category

Actual result

Now link /category?layout= or /category?layout=templatelayout if override com_content category view

Quy · 2018-04-18T20:51:05Z

components/com_content/helpers/route.php

-
-			if ($layout !== '')
+			$layout = JFactory::getApplication()->input->get('layout', '', 'string');
+			if (!empty($layout))


Add blank line above.

csthomas · 2018-04-18T20:50:46Z

components/com_content/helpers/route.php

 				$link .= '&lang=' . $language;
 			}

-			$jinput = JFactory::getApplication()->input;


IMO 'cmd' should be better than 'string'

if CMD layout was templatelayout and bug still alive if string template:layout this bug was on my site =)

If the layout can contain colon, then yes, I'm wrong.

@csthomas You were not wrong, just a filter designed for parameters why ignores the colon

The pattern for CMD is $pattern = '/[^A-Z0-9_\.-]/i';.

There could be a two options: add colon to filter code or replace colon to dot in layout.
The same as work for task=controller.method.

At now you do not have an option, stay with string.

csthomas · 2018-04-18T20:54:00Z

components/com_content/helpers/route.php

-			$jinput = JFactory::getApplication()->input;
-			$layout = $jinput->get('layout');
-
-			if ($layout !== '')


It's just a cosmetic suggestion. Instead of if (!empty($layout)) would be better to use if ($layout). We do not need to check if $layout is set.

You are right, I somehow did not even think. Done

brianteeman · 2018-04-18T22:35:02Z

Never mind the code style has anyone replicated the bug as I could not

…

On 18 Apr 2018 10:14 pm, "Tomasz Narloch" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In components/com_content/helpers/route.php <#20200 (comment)>: > @@ -79,10 +79,8 @@ public static function getCategoryRoute($catid, $language = 0) $link .= '&lang=' . $language; } - $jinput = JFactory::getApplication()->input; The pattern for CMD is $pattern = '/[^A-Z0-9_\.-]/i';. There could be a two options: add colon to filter code or replace colon to dot in layout. The same as work for task=controller.method. At now you do not have an option, stay with string. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#20200 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABPH8fHbx8LQi1JPKgMxTgtrnrtXDEYmks5tp6yggaJpZM4TavDK> .

Septdir · 2018-04-18T22:53:01Z

@brianteeman I, too, failed to reproduce the bug on the newly installed joomla, can only suggest creating an alternative / override layout for the test

But I can provide details of why #19681 caused a bug

In the past, PR layout turned out so

$jinput = JFactory::getApplication()->input;
$layout = $jinput->get('layout');

if ($layout !== '')
{
	$link .= '&layout=' . $layout;
}

What led to the following results

If category view use standard blog layout layout = NULL NULL !== ''
It was on two of my test sites updated with 3.8.6 to 3.8.7
If category use alternative/override layout layout = templatelayout And for the correct formation of a link layout = template:layout With a colon as a separator
It was on several client sites. And if they did not notice, then maybe in a week or two, the search results will have duplicate pages. That will negatively seo affect

This code will correctly add layout to the link

$layout = JFactory::getApplication()->input->get('layout', '', 'string');

if ($layout)
{
	$link .= '&layout=' . $layout;
}

In fact in this PR, I just corrected what was added by the filter 'string' and changing the check

Quy · 2018-04-18T23:01:32Z

Install the last demo.
Under System > Global Configuration > Articles > Integration > URL Routing, select Modern
Search Park Blog
Hover Park Blog link to see URL:
http://localhost/joomla387/index.php/using-joomla/extensions/components/content-component/article-category-blog?layout=

Quy · 2018-04-18T23:13:39Z

I have tested this item ✅ successfully on f8ad03c

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

Septdir · 2018-04-18T23:18:46Z

@Quy I tried to reproduce this bug with a clean installation of joomla on one of my subdomains, But I could not reproduce the bug.

Thank you.
Added your instruction How reproduce to the description

ReLater · 2018-04-19T12:42:07Z

I have tested this item ✅ successfully on f8ad03c

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

Quy · 2018-04-19T12:50:48Z

RTC

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

Septdir · 2018-04-22T09:10:39Z

Then what. If you display a module with links to com_tags or view article with its layout, then again there will not be a correct link.

@ReLater , @Quy Please test again.

P.S ontinuous-integration/drone/pr — the build failed?

Septdir · 2018-04-22T09:35:26Z

In fact, I would delete the addition of layout to the link, because personally I think it is not correct #19681 PR.

Because if I choose another for another language I choose another layout. Maybe I need to have another layout.

If there are those who think the same way, Say and I just delete the layout from the formation of the link in this PR

Septdir · 2018-04-22T10:37:15Z

Create new PR #20211 with full revert #19681
Please test

Fix layout in category link

94ac3ac

joomla-cms-bot added the PR-staging label Apr 18, 2018

Septdir mentioned this pull request Apr 18, 2018

Category link with ?layout= when active Modern route #20199

Closed

Quy reviewed Apr 18, 2018

View reviewed changes

csthomas reviewed Apr 18, 2018

View reviewed changes

Septdir added 2 commits April 18, 2018 23:54

Add blank line above layout check

7ac84db

Change layout check

f8ad03c

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 19, 2018

Add component and view check

8f32ad2

Code optimize

f3c4b47

Septdir closed this Apr 22, 2018

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

Septdir mentioned this pull request Apr 22, 2018

[com_content] Correctly adding layout to links #20211

Closed

Septdir deleted the fix-layout-in-category-link branch April 22, 2018 18:33

This was referenced Apr 25, 2018

Fix for creating links to categories #20238

Closed

Pass the default value of layout while getting it from input #20237

Closed

Uh oh!

Conversation

Septdir commented Apr 18, 2018 • edited by joomla-cms-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Testing Instructions

How reproduce bug by @Quy

Expected result

Actual result

Uh oh!

Quy Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

Septdir Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

csthomas Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

Septdir Apr 18, 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!

csthomas Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

Septdir Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

csthomas Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

csthomas Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

Septdir Apr 18, 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!

brianteeman commented Apr 18, 2018 via email

Uh oh!

Septdir commented Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Quy commented Apr 18, 2018

Uh oh!

Quy commented Apr 18, 2018

Uh oh!

Septdir commented Apr 18, 2018

Uh oh!

ReLater commented Apr 19, 2018

Uh oh!

Quy commented Apr 19, 2018

Uh oh!

Septdir commented Apr 22, 2018

Uh oh!

Septdir commented Apr 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Septdir commented Apr 22, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Septdir commented Apr 18, 2018 •

edited by joomla-cms-bot

Loading

Septdir Apr 18, 2018 •

edited

Loading

Septdir Apr 18, 2018 •

edited

Loading

Septdir commented Apr 18, 2018 •

edited

Loading

Septdir commented Apr 22, 2018 •

edited

Loading