ex: menu item for single article can not be use for category view

If I'm reading the code correctly, couldn't this also cause an issue for an article view trying to use a parent category's menu item?

Hi, this change also 'impacts' this issue: https://issues.joomla.org/tracker/joomla-cms/17096

When using this route:

<?php echo JRoute::_('index.php?option=com_content&view=form&layout=edit&Itemid=' . $menuId . '&return=' . base64_encode($uri), false); ?>

without the patch I get routed to this url:

https://mydomain/och_test/nl/?view=form&layout=edit&return=aHR0cHM6Ly9vZmZpY2Uub25saW5lY29tbXVuaXR5aHViLm5sL29...uYWdlbWVudA==

Which shows all frontpage modules and therefore doesn't display the edit / create form as on my templates these module are full screen.

with the patch I get routed to this url:

https://mydomain/och_test/nl/component/content/form?layout=edit&Itemid=621&return=aHR0cHM6Ly9vZmZpY2Uub25saW5lY29tbXVuaXR5...bWFuYWdlbWVudA==

The URL is still NOT routed to the menu I created for adding a new article, but at least now it displays the edit / create form correct (no modules from the frontpage overwriting the form)

I have tested this item ? unsuccessfully on 82dfb44

1. create menu for create new article (alias: edit-blog)
2. add button in view with the following route:

                        <div class="js-stools-container-buttonbar clearfix">
                                <button type="button" class="btn btn-success hasTooltip" title=""
                                data-original-title="<?php echo JText::_('COM_CONTENT_NEW_ARTICLE'); ?>"
                                onclick="location.href='<?php echo JRoute::_('index.php?option=com_content&task=article.add&return=' . base64_encode($uri), false); ?>';">
                                        <?php echo JText::_('COM_CONTENT_NEW_ARTICLE'); ?>
                                </button>
                        </div>

without patch the button redirects to the created menu edit blog

https://domain/och_test/nl/edit-blog?return=aHR0cHM6Ly9vZmZpY2Uub25saW...5sL29jaF90ZXN0L25sL3Rvb2xzL2Jsb2ctbWFuYWdlbWVudA==

with the patch routing is incorrect

https://domain/och_test/nl/?view=form&a_id=0&layout=edit&return=aHR0cHM6Ly9vZmZpY2Uub25saW5...25sL3Rvb2xzL2Jsb2ctbWFuYWdlbWVudA==
```<hr /><sub>This comment was created with the <a href="https://github.com/joomla/jissues">J!Tracker Application</a> at <a href="https://issues.joomla.org/tracker/joomla-cms/17746">issues.joomla.org/tracker/joomla-cms/17746</a>.</sub>

The issue has the same origin as in #17652. That patch solves this problem too.

@Ruud68 The last commit should fix your issue.

Happy to see that now we have this PR that also solves the issue I signaled in #17599 (about the same problem).
This PR uses, at point 4. , for MenuRules.php, basically the same idea I suggested in issue #17599 too and that I proposed to solve with PR #17652:
in libraries/src/Component/Router/Rules/MenuRules.php in preprocess function do not replace Itemid if Itemid is supplied.

This PR is more complete and faces other issues too. Good!

I will add more changes to not break B/C which currently exists. Please be patient.

I have tested this item ✅ successfully on b0a7507

With this patch the problem described in PR #17719, in PR #17652 and in issue #17599 are solved.

@LivioCavallo can you please close #17652?

closed #17652

I have tested this item ? unsuccessfully on ab898f8

Hi,

The latest versions works when you have a menu entry for create new article.
But if you do not create this entry or unpublish it, the JRoute url is routed wrong

1. unpublish menu item 'edit-blog'

JRoute::_('index.php?option=com_content&task=article.add&return=' . base64_encode($uri), false);

routes to

https://mydomain/och_test/?view=form&a_id=0&layout=edit&return=aHR0cHM6...0L3Rvb2xzL2Jsb2ctbWFuYWdlbWVudA==

But if you do not create this entry or unpublish it, the JRoute url is routed wrong
...

and it should have route to which URL ?

Hi, what it does is route to the frontpage of the site (which leads to unexpected results). What I would expect is that when no menu item is found that the url is a construction of the component / view, just like when you have no menu entry to an article / category > the article then is also not displayed on the frontpage url.

This is not only for create article, but applies (if memory serves me right) also for displaying the (edit) user profile.
When you do not have a menu entry for Users » Edit User Profile then the routing also goes to the frontpage.

Problem is that there are frontpages out there where the mainbody is replaced with a module > the edit / create form then just do not display.
Or they display with all modules assigned to the frontpage also displaying.

Standard behavior for a component with no router.php is [domain]/component/[componentname without com_]/?view=...etc.

so for this case it should be (at least that is what I would expect):

[domain]/component/content/?view=form&a_id=0&layout=edit&return=161896498126871

Just my thoughts :)

@Ruud68 After this changes your issue will be resolved: [It has been merged to this PR]

diff --git a/libraries/src/Component/Router/Rules/MenuRules.php b/libraries/src/Component/Router/Rules/MenuRules.php
index b82c44a92b..4298ed00b2 100644
--- a/libraries/src/Component/Router/Rules/MenuRules.php
+++ b/libraries/src/Component/Router/Rules/MenuRules.php
@@ -162,12 +162,15 @@ class MenuRules implements RulesInterface
                        }
                }
 
-               // If not found, return language specific home link
-               $default = $this->router->menu->getDefault($language);
-
-               if (!empty($default->id))
+               if (!isset($query['view']))
                {
-                       $query['Itemid'] = $default->id;
+                       // If not found, return language specific home link
+                       $default = $this->router->menu->getDefault($language);
+
+                       if (!empty($default->id))
+                       {
+                               $query['Itemid'] = $default->id;
+                       }
                }
        }
 

but I'm not sure if that can be done.

Generated links without menu item will not have default Itemid
but
links will be independent from default homepage - means more deterministic:)

PR is now ready to test again

I have tested this item ✅ successfully on 76013b0

Hi,

latest patch tested successful (also on multi-lingual site)

This also fixes the issue (same) for when you want to edit your user profile from the front end but there is no menu item for that.

after patch

with menu item: https://[domain]/nl/edit-blog?return=aHR0cHM6Ly9vZmZp...=
without menu item: https://[domain]/nl/component/content/form/?layout=edit&return=aHR0cHM6Ly9...udA==

without patch

with menu item: https://[domain]/nl/edit-blog?return=aH...ZXN0L25sL3Rvb2xzL2Jsb2ctbWFuYWdlbWVudA==
without menu item: https://[domain]/nl/?view=form&a_id=0&layout=edit&return=aHR0cHM6Ly9vZm...YWdlbWVudA==

I have tested this item ? unsuccessfully on 76013b0

Hi, sorry.
Just did another test:
when trying to EDIT an existing article, without the patch it works, with the patch the edit form is empty

$item->slug = $item->alias ? ($item->id . ':' . $item->alias) : $item->id;
$contentUrl = ContentHelperRoute::getArticleRoute($item->slug, $item->catid, $item->language);
$url = $contentUrl . '&task=article.edit&a_id=' . $item->id . '&return=' . base64_encode($uri);

<?php echo JRoute::_($url); ?>

results in routed url BEFORE patch (working)

https://[domain]/nl/community-tools?view=form&layout=edit&a_id=81&catid=13&return=aHR0cHM6Ly9vZmZpY2Uub25saW5lY29t...WVudA==

AFTER patch (empty form)

https://[domain]/nl/component/content/form/[blog title]?layout=edit&catid=13&return=aHR0cHM6Ly9vZmZp...2xzL2Jsb2ctbWFuYWdlbWVudA==

note that menu item 'edit-blog' is NOT converted
EDIT: also when you want to edit an article where the category is not assigned to a menu, the the routing is NOT to the edit form but again to the frontpage...

@Ruud68 This is a little unrelated:

https://[domain]/nl/component/content/form/[blog title]?layout=edit&catid=13&return=aHR0cHM6Ly9vZmZp...2xzL2Jsb2ctbWFuYWdlbWVudA==

because IMO [blog_title] is real article alias (without id prefix) and you tested it with experimental routing, on stable routing it should works. Can you confirm?

hi @csthomas ,
correct I had unstable routing enabled + remove ID = yes.
When setting remove ID to no it works, when switching to stable it works.

So I think this has something to do with the remove ID (unstable) function.

If you connect with me via email I can sent you a component that adds front-end article management to your test site. That makes it easier to reproduce I think:

add new article button
edit blog that is in category that has menu assigned
edit blog that is in category that is NOT assigned to menu

Or you can use UAM from the JED: https://extensions.joomla.org/extensions/extension/authoring-a-content/content-submission/user-article-manager/

That would make testing easier

PR is ready to test again:)

Hi, here you go.
I ran 8 test cases against:

• 3.7.5
• 3.8.0b4 with routing = stable
• 3.8.0b4 with routing = experimental
• 3.8.0b4 with routing = experimental + remove id = yes
• Joomla 3.8.0b4 + Deterministic patch (Applied Commit SHA: f3f5db5) > Routing = stable
• Joomla 3.8.0b4 + Deterministic patch (Applied Commit SHA: f3f5db5) > Routing = experimental
• Joomla 3.8.0b4 + Deterministic patch (Applied Commit SHA: f3f5db5) > Routing = experimental + remove item id

Test cases are all create and edit related, with and without create article menu, etc.

Test cases and results in attached file (looking good :)
20170831Deterministic version of URL routing.pdf

Thank you @Ruud68 for test results.

@Ruud68 can i alter your test as successfully at issue Tracker?

Hi, yes you can :)
Thnx @csthomas for your work on this patch!

@Ruud68 done.

ex: menu item for single article can not be use for category view

If I'm reading the code correctly, couldn't this also cause an issue for an article view trying to use a parent category's menu item?

@mbabker This is only related to search Itemid using current page (active menu) - it is a shortcut.
And it can not be used for link which has different parameter view, option, etc included in menu item query from database.
The correct Itemid will be find later in MenuRules::preprocess() at https://github.com/joomla/joomla-cms/pull/17746/files#diff-b4715241c608ab78691c308049a1aac1R125.

There is still missing a few unit tests which I want to add.

I added (not translated for now) an application option (below sef options) to disable adding default itemid to URL.

My test on a mulingual site, Experimental Router on for com_content.
WITHOUT PATCH
A specific menu module (tagged to ALL) only assigned to Home pages
No menu item for a category.
Link from an article is : /index.php/fr/?view=category&id=9
Specific Menu module displays.

AFTER PATCH
COM_CONFIG_FIELD_ITEMID_ALWAYS_REQUIRED_LABEL set to No
/index.php/fr/component/content/category/categorie-fr-fr
Specific Menu module does NOT display

Link to a subcategory of categorie-fr-fr
/index.php/fr/component/content/category/newcatfrench
=> 404

COM_CONFIG_FIELD_ITEMID_ALWAYS_REQUIRED_LABEL set to Yes
/index.php/fr/?view=category&id=9

Specific Menu module displays.

Link to a subcategory of categorie-fr-fr displays the category OK
/index.php/fr/?view=category&id=11

Looks like we are not there yet.

@infograf768 I fixed it.

I also added more unit tests.

For B/C 'COM_CONFIG_FIELD_ITEMID_ALWAYS_REQUIRED_LABEL' is set to Yes for default.

In component router file, line:
$category->setKey('id')->setParent($categories)->setNestable()->addLayout('blog');
is generally a equivalent to the old way:
$category->setKey('id')->setParent($categories, 'catid')->setNestable()->addLayout('blog'); when view is nestable and parent key has a key.

The second argument 'catid' in above setParent() is not useful.

Category view does not use parent_id in any links like index.php?option=com_content&view=category&id=1.

I stuck in stable routing with a B/C problem.

Does joomla should stay with old behaviour of build URL and add an active menu to every URL that does not have own menu item, ex:

Default Itemid is 1. There is only 2 menu items (com_content) in menu in database.

1. Default to featured view.
2. Menu for Category X - for category view

Current active URL is (Menu for Category X)
index.php?option=com_content&view=category&id=3&Itemid=2
and when I try to generate URL for Category Y without own menu item and this is not a child of Category X.

What should be the link to Category Y:

1. index.php?option=com_content&view=category&id=4&Itemid=2 which means use active Itemid from Category X.
2. index.php?option=com_content&view=category&id=4&Itemid=1 which means use default Itemid if global option Itemid always required is enabled (default).

If a link cannot be built using something in the existing menu tree, then the default menu item should be used. So it would be &Itemid=1. Note that even if the link is built without that menu item, if you put in the index.php?option=com_content&view=category&id=4 URI and it doesn't resolve to a menu item the system will internally use the default anyway. So the build behavior should be consistent with the rest of the system.

then the default menu item should be used. So it would be &Itemid=1.

Isn't the default menu item whatever has the home flag - it doesnt have to be 1?

Right, just following the examples he gave in his comment.

It is only useless to you because you do not face any barriers when you use the internet

oops sorry - wrong post

Note that even if the link is built without that menu item, if you put in the index.php?option=com_content&view=category&id=4 URI

this is equal to /component/content/category/4-cat on SEF enabled

and it doesn't resolve to a menu item the system will internally use the default anyway

I have the opposite experience, system does not use default, Itemid is NULL, active is null. Default menu item has a number.

Page can have:

• no Itemid, means NULL as above or when url start with index.php/component/
• Itemid equal to home page, means default, (there is more variants for multilingual), when we use some segments that is not belong to any menu, ex index.php/14-cat/15-article or index.php?view=category&id=2
• no Itemid if you put not exists id, like index.php?option=...&Itemid=999999
• or some exists item

To do a simple test you can create custom module only for home page menu item.

I looked through this PR and I agree that the current behavior of Joomla is an issue. Simply using the default Itemid upon building the URL is wrong and generates several issues, among them the problem that we don't know if the URL that we are processing has a legitimate Itemid, or if the Itemid that is being used has just been added because, meh, aint got nothing better...

However, I disagree with the proposed solution. First of all, I would not fix this in the 3.x branch, since we are almost guaranteed to somehow mess this up and introduce behavior that will break someones site. Then there is the issue of yet another config option and one that also will massively confuse people. I doubt that you can explain this setting even to someone who really knows Joomla without at least half a page of text. Last but not least, I'd like to see less code. From my perspective, this issue should be solved by removing the fallback to the home menu item in Joomla\CMS\Component\Router\Rules\MenuRules lines 114 to 140 and by removing the additional checks for "intentional or uninentional Itemid?". We have to make the code maintaineable and that means less code and special cases. We should never have code like $url .= '&Itemid=' .$Itemid anywhere in our general code. All of this should always and only be handled in the router(s).

@Hackwar

Too much work in this PR , too much time spent on it

maybe it would be better if you could review it deeper
and help make it acceptable

I believe, this PR addresses a few more issues with routing, not only default menu item
@csthomas maybe you can compile a list ?

However, I disagree with the proposed solution. First of all, I would not fix this in the 3.x branch, since we are almost guaranteed to somehow mess this up and introduce behavior that will break someones site.

If it is going to be included in J4, at least this work would have been wasted !
Can you provide example of something broken ? maybe @csthomas can address it

Too much work in this PR , too much time spent on it

Just because someone invested a lot of time into something, doesn't mean we should simply accept it.

maybe it would be better if you could review it deeper
and help make it acceptable

Ok:

1. Find a better name for the option. The current option is not something that a user will understand. Even advanced users.
2. Add this behavior only to the new routing and don't modify the legacy routers.
3. Make the setting a component specific setting and not a global one. It will break third party components.
4. Don't remove the identifier for the parent of the category view in the new router. That breaks existing behavior.

That would be the first few things that would have to be adressed.

I believe, this PR addresses a few more issues with routing, not only default menu item

That would actually be a case for me to disregard this PR altogether. If you want to modify the routing, do it one thing at a time. PRs that fix lots of things are the devil.

If it is going to be included in J4, at least this work would have been wasted !

Are you really going to argue with me about wasted work? Are you really going to ask me to dig up this https://github.com/joomla/joomla-cms/pulls?utf8=%E2%9C%93&q=is%3Apr%20is%3Aclosed%20author%3AHackwar%20router and the dozens of other PRs that are not covered by that query or the issues on joomlacode.org regarding the router and all the public and private discussions about this that I had to fight. Do you really want me to whine about the 8 years that I wasted on this? The numerous times this was dropped from a release because people didn't feel like it was ready, even though it was basically the exact same code that has now been released with Joomla 3.8? Please don't talk to me about wasted work.

@Hackwar

i suspected you would answer something like this,
but still i felt like argueing about it

The problem with accepting something is the extra work and frustration to later support it (or even fix the problems it creates)
i know a lot about it, have been there hundrends of times

about wasted time, simple way out, contribute never again anything that is bigger than few lines of code or that is "big" but "very reviewable" and "simple" / "repetitive"

i am sorry if i have annoyed you, i respect your works
and i did not say that this PR is ready to be accepted,
and you are right it will cause problems in some websites,
i have not tested it yet

I am not saying that you should not invest time into something. I am saying that invested time does not automatically mean that it has to be accepted by the project. And especially from the perspective of maintaineability, we should not add something that should be the default and only behavior as an option and thus introducing hundreds of lines of hard-to-maintain code.

All that said: This is getting off topic and I will not further the discussion about wasted or not wasted work here.

Then there is the issue of yet another config option and one that also will massively confuse people.

By default there is the old behaviour. Itemid Always Required is set to Yes. Default item id stay.

If admin do not want to have a home page menu item (means default) on every page that does not have own menu item then should change that option to No.

[UPDATED] In below commit the config option has been removed. Legacy routing can not use this feature. Modern routing using it if developer decide that by adding sefAdvanced = true to $router. Below I will mention about modern routing and this will also means that sefAdvanced = true.

If you set it to No then links like:
In new routing
index.php?view=category&id=[2:category-alias] will be changed to /component/content/category/[2-]category-alias

I would not fix this in the 3.x branch, since we are almost guaranteed to somehow mess this up and introduce behavior that will break someones site.

I think that you are against removing active menu item from every URL without own menu, for legacy routing.
If this is a break B/C then yes I failed and I should change that or try again on J4.

[UPDATED] Ok, I fixed it. Only new routing will have featured like, do not add active/default menu to other pages that does not have it.

This is an example when I am on article view with menu item and have a link to category without menu item.
I put into brackets the id from modern routing version.

Active is: /category-1-menu/[1-]article and I put there (to article) a link to the next category without own menu item, parent categories also does not have any menu item.

• 3.x - /category-1-menu/[2-]category-alias If you enable new routing+remove IDs then you get error 404. There is somewhere an issue.
• PR - /index.php?option=config&view=category&id=[2:]category-alias or /component/content/category/[2-]category-alias

The second URL you get when you set the mentioned config to No

From my perspective, this issue should be solved by removing the fallback to the home menu item in Joomla\CMS\Component\Router\Rules\MenuRules lines 114 to 140 and by removing the additional checks for "intentional or uninentional Itemid?"

You say home menu item, not active menu item. I someone want to stay with old way then do not change mentioned config and still has default (home) menu item.

[UPDATED] For new routing fallback to home page has been removed.

If you talk about active menu item in

and by removing the additional checks for "intentional or uninentional Itemid?"

then Yes I still think about this block of code https://github.com/joomla/joomla-cms/pull/17746/files#diff-b4715241c608ab78691c308049a1aac1R90
but #17719 and #16774 block me.

[UPDATED] I fixed it. I removed magic method to check that:). Legacy routing works as before. Modern
routing unset active menu item if not found any match.

There is also 3rd party extensions that still using routing from < J3.3 when joomla extensions add &itemid=... to every link.

IIRC home menu item has to be added when you create a link to for ex: index.php?lang=en.

We have to make the code maintaineable and that means less code and special cases.

I changed hard to understand loops to simpler, added more comments. Nomenurules code is bigger but works better for more advanced configuration.

We should never have code like $url .= '&Itemid=' .$Itemid anywhere in our general code.

Yes I think the same and I removed such old code from J3 at https://github.com/joomla/joomla-cms/pull/17746/files#diff-5f3b2b67cf0f2cd864b81712b165c102L304

Don't remove the identifier for the parent of the category view in the new router. That breaks existing behavior.

It is B/C. It is not break anything what I know. Unit tests proved it.

Code from com_content in J3.8

$categories = new JComponentRouterViewconfiguration('categories');
$categories->setKey('id');
$this->registerView($categories);
$category = new JComponentRouterViewconfiguration('category');
$category->setKey('id')->setParent($categories, 'catid')->setNestable()->addLayout('blog');
$this->registerView($category);

and my PR:

$categories = new JComponentRouterViewconfiguration('categories');
$categories->setKey('id');
$this->registerView($categories);
$category = new JComponentRouterViewconfiguration('category');
$category->setKey('id')->setParent($categories)->setNestable()->addLayout('blog');
$this->registerView($category);

are equal, and I can revert it for 3 component/com_[content/contact/newsfeeds]/router.php files without any problem. This is old way and new way.

[UPDATED] I reverted it. It still works.

You can add or remove 'catid' on $category->setKey('id')->setParent($categories); to test that.

New version allow to build something like:

$categories = new JComponentRouterViewconfiguration('categories');
$categories->setKey('id');
$this->registerView($categories);
$archive = new JComponentRouterViewconfiguration('archive');
$archive->setKey('catid')->setParent($categories)->setNestable();
$this->registerView($archive);
$category = new JComponentRouterViewconfiguration('category');
$category->setKey('id')->setParent($archive)->setNestable()->addLayout('blog');
$this->registerView($category);
$article = new JComponentRouterViewconfiguration('article');
$article->setKey('id')->setParent($category, 'catid');
$this->registerView($article);

When category is published you get URL like:
/menu-for-published/3-category/4-article-in-category
and when category is archived you get:
/menu-for-archived/3-category/4-article-in-category

this is probably not practical but it show how much it is advanced.

Category view can be nestable in other nestable view which does not need to exists in URL or can appear on some circumstances.
When category is archived then category view do not add menu or any segments but archive view add own menu and segments.

The current old StandardRules parser does not parse it, but NomenuRules does.

The old way with 'catid' in setParent() is partially tested by unit tests.

I want to show developers how should looks like link to category when he/she is looking on the code:
$category->setKey('id')->setParent($categories)->setNestable()->addLayout('blog');
means:
index.php?option=...&id=[value] not index.php?option=...&id=[value]&catid=[value2]

where:
$article->setKey('id')->setParent($category, 'catid'); means:
index.php?option=...&id=[value]&catid=[value2]

Changes in legacyrouter.php at https://github.com/joomla/joomla-cms/pull/17746/files#diff-df69d79e813cd61b724d507d228b28d6L65 are not very important and they change:

• other way for if else
• add one more important isset
• I added a segment for view=form and unset empty a_id.

If I have to revert it then I will do it. I only wanted to do it in simpler way.
The main idea of this PR will stay.

I probably made a lots of mistakes in my English:)

After above comments I made decision to remove config 'Itemid Always Required' and do not add default item id to links without own menu item for modern routing. This can be configurable by sefAdvanced variable per component.
Legacy routing will still have active and default menu item id as before.

Is this PR still valid now that #18429 has been merged?

In general this PR can be closed because I split it in a few PRs, not all has been merged yet but this won't go any where.