PR-4.0-dev

Success

User tests: Successful: Unsuccessful:

avatar csthomas
csthomas
18 Sep 2018

Fix issues:

  • #15730 (19092) - stops the redirection loop when the home page is only for registered users and the login page does not have any menu item (/component/users/login),
  • #22036 - remove ugly links: index.php?view=article&id=1-article&catid=2 and replace them by /component/content/article/2-category/1-article
  • remove URL parameter &Itemid=xxx from all /component/content/... links
  • stop display that you are on "Home" when you are not, ex /component/users/login
  • I have fixed some of the previous issues mentioned at #19537 - as much as I could (#19537 (comment))

Summary of Changes

This PR is an improved redo of #19099 and #19295.
There is a few difference to previous PRs.

Basic assumptions

  1. These links will inherit Itemid from the current page

    • Route::_('index.php') - missing option and view
    • Route::_('index.php?id=....') - missing option and view
    • Route::_('index.php&option=com_content') - missing view
    • Route::_('index.php&option=com_users&task=user.login') - missing view
    • Route::_('index.php&view=categories') - missing option
  2. This not

    • Route::_('index.php&option=com_content&view=categories') - if menu item won't exists then /component/content/categories will be generated
    • Route::_('index.php&option=com_users&view=login&task=edit....') - if menu item won't exists then /component/users/login?task=user.login will be generated
  3. The redirect URL in &return= parameter should be stored in Non SEF mode, ex: /login-form?return=urlencode(base64_encode('index.php?Itemid=111')).

    • if not (ex: /login-form?return=urlencode(base64_encode('/news'))) then association in multilingual website won't work correctly, because languagefilter plugin is looking for raw URL parameter Itemid.

This patch may break B/C.

Regards of changes in this PR, some pages like '/component/content/categories' won't have any active menu item, Itemid. This way we won't have headline "Home" in pages that does not have its own menu item.

A suffix &Itemid=xxx should disappear from SEF link.

After applying this patch, joomla will stop adding the default Itemid to links without their own menu item.
Previously it looked like /component/content/article/1-article&catid=2&Itemid=102 now it will be /component/content/article/2-uncategorised/1-article (without suffix &Itemid=102, which is/was redundant).

Without this patch, for pages without their own menu item, joomla generates a lot of duplicated links.
See #22450 (comment)

Testing Instructions

Look at the test instructions for issues:
#15730
#22036
#19295

If you get a 404 error on pages like /component/content/... then also apply #20979.

Documentation Changes Required

??

avatar csthomas csthomas - open - 18 Sep 2018
avatar csthomas csthomas - change - 18 Sep 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 18 Sep 2018
Category Front End com_content com_users Layout Libraries Modules Plugins Templates (site)
avatar PhilETaylor
PhilETaylor - comment - 18 Sep 2018
avatar mahagr
mahagr - comment - 18 Sep 2018

I love how this PR adds the option/view to the URL and moves the task to the post. It will surely fix multiple issues and has improved security. There is one more change I would make, though: adding ItemId (route) to the links would allow the non-used extensions to be blocked in the future. Both Kunena/Gantry 5 prevent all access the component if you're not using menu item.

avatar infograf768
infograf768 - comment - 18 Sep 2018

The redirect URL in &return= parameter should be stored in Non SEF mode, ex: /login-form?return=urlencode(base64_encode('index.php?Itemid=111')).
if not (ex: /login-form?return=urlencode(base64_encode('/news'))) then association in multilingual website won't work correctly, because languagefilter plugin is looking for raw URL parameter Itemid.

Do we have such situation in core?

avatar csthomas
csthomas - comment - 18 Sep 2018

Do we have such situation in core?

Yes. When SEF mode is ON.

Take a look at line from J3/J4 code:
$this->setUserState('users.login.form.data', array('return' => Uri::getInstance()->toString()));

where Uri::getInstance()->toString() returns SEF link (https://.../company/about) without Itemid as parameter.

A workaround done in J3/J4 is to force the active Itemid to the form action link.
Thanks to this Itemid in the action link (/login?Itemid=), the association works.

avatar csthomas
csthomas - comment - 18 Sep 2018

The second method used by J3/J4 is to insert return URL (the current page - mod_login) directly in the form action link
<form id="login-form" class="mod-login" action="<?php echo Route::_('index.php', true); ?>" method="post">

Then SEF link is parsed and plugin languagefilter uses active menu item to find the correct association.

avatar csthomas
csthomas - comment - 18 Sep 2018

There is one more change I would make, though: adding ItemId (route) to the links would allow the non-used extensions to be blocked in the future.

You can try to build routing based on com_content from J3.
The component in legacy or modern mode adds automatically &Itemid=xxx to links:
JRoute::_('index.php?option=com_content&view=categories');

The old implementation of routing (com_tags) requires the manual addition of Itemid to JRoute :: _('...&Itemid=xxx');

If you do not want extension to be used then disable it.

avatar Hackwar
Hackwar - comment - 18 Sep 2018

This is working against the concepts of the routing. No, adding the current menu item to the URL is NOT the solution to people giving a fuck and not setting up their website correctly.

avatar csthomas
csthomas - comment - 18 Sep 2018

adding the current menu item to the URL is NOT the solution ...

Yes. This is one of the reasons why I created this PR to eliminate adding an active menu item id in many places:

  • components/com_content/tmpl/article/default.php
  • components/com_content/tmpl/category/blog_item.php
  • components/com_content/tmpl/category/default_articles.php
  • components/com_content/tmpl/featured/default_item.php
avatar csthomas csthomas - change - 18 Sep 2018
The description was changed
avatar csthomas csthomas - edited - 18 Sep 2018
avatar laoneo laoneo - change - 19 Sep 2018
Labels Added: PR-4.0-dev ?
avatar mahagr
mahagr - comment - 19 Sep 2018

@Hackwar Haha, I get your point. It has always caused a lot of questions in Kunena support because of people have no idea how Joomla menu system works. In Gantry it doesn't really matter as the purpose of the menu item is to only to show blank content (there are no tasks that are independent to the menu item).

Though that said, most people do get it, but only after multiple messages in support. Also, this doesn't seem to be an issue in Grav, where pages are always defined by the hierarchy.

I was just asking this because of to me having less known URLs to attack would make it less likely that something you installed but later forgot does not create security issues in your site. Though it is not such a large issue as it used to be before you could automatically update extensions.

avatar csthomas csthomas - change - 19 Sep 2018
The description was changed
avatar csthomas csthomas - edited - 19 Sep 2018
avatar vagabondo
vagabondo - comment - 19 Sep 2018

@csthomas
I set patch, make new page (Categories - Uncategorised, Featured), open site.
Display link: http://www.joomla-tested.ru/component/content/article/555678.html?catid=2
When I open the link:
The requested page can't be found. Error 404

Now don't work

avatar csthomas
csthomas - comment - 19 Sep 2018

@vagabondo I answered at #22036 (comment)

You have to apply one more patch from #20979

avatar vagabondo
vagabondo - comment - 19 Sep 2018

@csthomas
If use patch from #20979, display link: http://joomla-tested.ru/component/content/article/uncategorised/555678.
In general, it works, but part of the address (component/content/article) is not needed.

avatar csthomas
csthomas - comment - 19 Sep 2018

The structure of routing need it but only if joomla can not find a correct menu item.

The path /component/content/article inform you that you don't have any menu for "all categories" nor "Uncategorised" category, nor this article.

You can leave it as it is and it will work correctly OR if you care about SEO, create a menu item for at least "All Categories".

avatar vagabondo
vagabondo - comment - 19 Sep 2018

@csthomas
it's not correctly. If you look at 3.8.12, then when you display the link to the page (if there is no menu item and the page - Uncategorized and Featured), the address is displayed correctly, without /component/content/article. Creating the menu item "All categories" - the correct solution is not.

If set Unpablished for "All categories" - part /component/content/article - exist. If set Published and access Super User - error.
Any page go to http://www.joomla-tested.ru/component/users/login. If module Login - Unpublished, also - error.
Any page go to at http://www.joomla-tested.ru/component/users/login

The user should not think about how to overcome the error that is present.

avatar vagabondo
vagabondo - comment - 19 Sep 2018

@csthomas
Now make category "experiment", make new articles at this category and set featured for articles.
At home page see article "123456 ". Link - http://www.joomla-tested.ru/component/content/article/experiment/654321. Categories - different. Mistake - exist.

avatar csthomas
csthomas - comment - 19 Sep 2018

If you want to compare to J3.8.12 then be sure that you use modern routing in com_content.

If set Unpablished for "All categories" - part /component/content/article - exist. If set Published and access Super User - error.
Any page go to http://www.joomla-tested.ru/component/users/login.

If you set access only to "Super User" for "All categories" then everything inside this menu requires access of "Super User".
After you login (as "Super User"), you will back to your article.
If you login as not "Super User" then you get 403 Access Denied.

avatar vagabondo
vagabondo - comment - 19 Sep 2018

@csthomas
I repeat once again that the option with a solution in the form of a menu item - "all categories" - is not correct.
But we will try in a different way. I make "All categories" as hidden and access - Public, open home page.
If open page, I see link - http://www.joomla-tested.ru/all-categories/experiment/654321. Do you think this link is correct? There is an extra element - all-categories. And if you think about it, then you can have duplicate pages with different addresses.
And when you click on the links inside the pages, this additional element begins to appear. there is no need.
example: page '555' at site: http://www.joomla-tested.ru/. At page exist - link to different page.
The menu item editor also works incorrectly

Then in the release also write that for the correct display (with an additional element in the path) - necessary to include the menu item "All categories"? Sorry, but I think that this will not lead to good results

avatar infograf768
infograf768 - comment - 19 Sep 2018

That is what I do, and also in 3.x with modern routing on (List all categories menu item set to hidden, for each Content Language when the site is multilingual), and similar for Contacts and Newsfeeds.
It does not create issues with duplicate as far as I have remarked.
For other components: Users, SmartSearch are good examples where one HAS to create specific menu items, even if they are hidden as they can be called by modules or articles, or others.

avatar vagabondo
vagabondo - comment - 19 Sep 2018

Next error.
Make item menu "probed". Item menu link with article - "555", see only link nemu at adress. Next we pen link at page. And have adress: http://www.joomla-tested.ru/all-categories/experiment/654321. Also look all-categories. But if make menu item "All categories" unpublished, also error 404.
Solution don't work

avatar vagabondo
vagabondo - comment - 19 Sep 2018

@infograf768
If I have several hundred categories on my site, there will be problems. Why, because of an error in the router and unnecessary actions (due to the inability to eliminate correctly) - should users suffer? The user simply goes to another СMS, where there is no such problem.

avatar infograf768
infograf768 - comment - 19 Sep 2018

Evidently the all categories menu item has to be published, even if hidden, and access set to public...

avatar vagabondo
vagabondo - comment - 19 Sep 2018

@infograf768
If I have several hundred categories on my site, there will be problems. Why, because of an error in the router and unnecessary actions (due to the inability to eliminate correctly) - should users suffer? The user simply goes to another СMS, where there is no such problem.

avatar infograf768
infograf768 - comment - 19 Sep 2018

If you have hundred categories on your site, this will treat them the same.
I personally agree though that having to use a "trick" like this is not nice and certainly not user friendly.

avatar vagabondo
vagabondo - comment - 19 Sep 2018

@infograf768
When potential users learn about this, the transition to other engines will begin. With more friendly settings and more correct work. A solution that is not a solution - a very unpleasant fact that will alienate people.

avatar csthomas
csthomas - comment - 19 Sep 2018

@vagabondo If you want to remove /all-categories and /component/content/.../ prefixes in the path then you have to create a hierarchy of your com_content categories, ex:

One parent category "Home Category" (and all your categories will be inside) and set menu item for it as home. I mean, replace featured view by category view in your home menu item.

The other way is to set "All Categories" as a home menu item.

IMO the featured view (com_content) should be removed and functionality should be moved to category/categories view.
Then all this things will be easier.

avatar vagabondo
vagabondo - comment - 19 Sep 2018

Next error. I rename menu item all-categories at all. In path have all-categories. Router correctly is not working

avatar vagabondo
vagabondo - comment - 19 Sep 2018

@csthomas
I apologize, but the solution is not correct. So will be a very small number of people. Most will use other engines

avatar csthomas
csthomas - comment - 19 Sep 2018

This PR may solve only part of many problems.

I understand that you want to get rid of /all-categories and /component/content/.../ but this PR is not designed for that.

If this PR helps then please mark the test as a success at https://issues.joomla.org/tracker/joomla-cms/22229

Otherwise, we will wait for a better PR or we will remain with current J4 behavior.

avatar vagabondo
vagabondo - comment - 19 Sep 2018

Not successful

avatar vagabondo
vagabondo - comment - 20 Sep 2018

Next errors.
Without patch, but menu item "All_categories" - Published and hidden.
Open site http://www.joomla-tested.ru. Open menu item probed1. At page exist link to page "555" (not featured, category - experiment). Open link. Adress: http://www.joomla-tested.ru/all-categories/experiment/555678. all-categories?

With patch, menu item "All_categories" - Unpublished.
Finished adress: http://www.joomla-tested.ru/component/content/article/experiment/555678

With patch, menu item "All_categories" - Published and Hidden.
Finished adress: http://www.joomla-tested.ru/all-categories/experiment/555678

In all cases, the last page is in the category "experiment" and not Featured. And on it there is a link from the previous page. The address of the site will be changed to the form

. And in this addition there is no need.
Once again I repeat that solution - for each category its menu item - is not correct.

Summary: Not successful.

avatar csthomas
csthomas - comment - 20 Sep 2018

The problem exists in your menu configuration.

What do you want?

  • no menu per category
  • no /component/content prefix
  • no /all-categories prefix

How can you get it:

  • Create only one top category "Home Category"
  • Move other your categories to "Home Category"
  • Go to menu configuration and change your home menu item from "Featured Articles" to "Category Blog". Then choose a category "Home Category" and save.

Result:

  • article from "Home Category" -> "Experiment" -> "555" will be at /experiment/555678

This should work even without this PR.

avatar csthomas csthomas - change - 22 Sep 2018
Title
[4.0][RFC] Another idea how to fix or break routing in joomla
[4.0] Another idea how to fix routing in joomla
avatar csthomas csthomas - edited - 22 Sep 2018
avatar csthomas csthomas - change - 22 Sep 2018
Title
[4.0][RFC] Another idea how to fix or break routing in joomla
[4.0] Another idea how to fix routing in joomla
avatar csthomas csthomas - change - 25 Sep 2018
The description was changed
avatar csthomas csthomas - edited - 25 Sep 2018
avatar csthomas csthomas - change - 2 Oct 2018
The description was changed
Labels Removed: ?
avatar csthomas csthomas - change - 2 Oct 2018
The description was changed
avatar csthomas csthomas - edited - 2 Oct 2018
avatar csthomas
csthomas - comment - 2 Oct 2018

I reverted changes with the access level settings to the menu items. These were the same changes that I tried to do in #22445

avatar csthomas
csthomas - comment - 2 Oct 2018

Dron's fail tests are not related.

avatar csthomas csthomas - change - 4 Oct 2018
The description was changed
avatar csthomas csthomas - edited - 4 Oct 2018
avatar csthomas csthomas - change - 9 Oct 2018
The description was changed
avatar csthomas csthomas - edited - 9 Oct 2018
avatar csthomas csthomas - change - 22 Oct 2018
The description was changed
avatar csthomas csthomas - edited - 22 Oct 2018
avatar roland-d
roland-d - comment - 1 Aug 2020

This can be closed and picked up in another PR if someone is interested.

avatar roland-d roland-d - change - 1 Aug 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-08-01 15:55:10
Closed_By roland-d
avatar roland-d roland-d - close - 1 Aug 2020

Add a Comment

Login with GitHub to post a comment