Fix error 404 in modern routing for links without own menu item (NomenuRules)

[csthomas]

Summary of Changes

When there is no menu item for category/categories/article view in com_content and option Remove IDs is enabled then a link like:

• /index.php/en/component/content/article/article-en-gb?catid=8 returns error 404

In this PR I rewrote NomenuRules class to build and parse links:

• index.php/en/component/content/article/category-en-gb/article-en-gb (option Remove IDs is enabled)
• index.php/en/component/content/article/3-category-en-gb/2-article-en-gb

Testing Instructions

Disable all menu items for com_content categories/category/article views.
Go to frontend index.php/component/content/categories and check links to category and article items.

Expected result

Links work, no 404 when option Remove Ids is enabled/disabled.

Actual result

When option Remove Ids is enabled then links to articles return error 404.

[jsubri]

Quite large PR, congrats. I've tested with a copy of a customer site on my PC. At a fist glance the links do not have the IDs anymore which is good, and 404 are gone.

I've a draw back for Links displayed via Modules . In my case the links are displayed via the Joomla Custom Modules in position Right using the popular 3rd party extension https://www.regularlabs.com/extensions/articlesanywhere . I can reproduce without the 3rd party plug-in using the "Modules: Articles - Newsflash" once le links are clicked they are loaded in the "main" position. At this point this is B/C the Modules in Right position do not load anymore using Menu Assignment "Only on the pages selected" as we lost the menu "context".

[csthomas]

Quite large PR, congrats. I've tested with a copy of a customer site on my PC. At a fist glance the links do not have the IDs anymore which is good, and 404 are gone.

So it is success test.

I've a draw back for Links displayed via Modules . In my case the links are displayed via the Joomla Custom Modules in position Right using the popular 3rd party extension https://www.regularlabs.com/extensions/articlesanywhere . I can reproduce without the 3rd party plug-in using the "Modules: Articles - Newsflash" once le links are clicked they are loaded in the "main" position. At this point this is B/C the Modules in Right position do not load anymore using Menu Assignment "Only on the pages selected" as we lost the menu "context".

In general, this is unrelated to this PR.
If you have links start with index.php/component/ or /component/ then it means that there is no menu item for such pages. If there is no menu item then modules won't work as you expected.

I understand that you complain, that before you have (invalid - from my perspective but) working link like '/menu-category-a/category-alias-b/articlealias' but now you have /component/content/article/category-alias-b/articlealias.

If there is no menu item then only modules with setting "On all pages" (and IIRC On all except selected) will be displayed.
To fix this issue you should create menu item for missing category (category-alias-b).

Ex, if you see link to article like /component/content/article/alias-category/alias-article this means that you should create a menu item for category "Alias Category".
If you do not want to create a massive items per each top category you can create menu item for categories view only.

[jsubri]

I understand that you complain, that before you have (invalid - from my perspective but) working link

Correct that is the reason I mentioned B/C

To fix this issue you should create menu item for missing category (category-alias-b).

You are correct again and worth mentioning the menu item need to be Published otherwise it won't work. In my specific case if this PR is accepted, I'll end-up creating 3-5 extra menu and editing 20+ modules to have the same user experience. On the positive note, SEO and sitemap will be enhanced.

It breaks my site and I can deal with that, so can break others.

I'll vote for this PR subject to super-duper assessment.

Updated formatting.

[csthomas]

To be clear, mentioned issue about B/C is related to already merged PR #19099, not to this one.

I'll end-up creating 3-5 extra menu

Yes, this should be done earlier in you website but earlier joomla allows to missing menu items and put there (in link) menu items from other contents. The problem was hidden.

Ex. if you had 2 different categories (dogs and cats) on the same top level and you missed to add menu item for dogs then when you are on article about cats and you have a module with links to dogs, then links to dogs (in an old way) looks like /menu-for-cats/2-category-dogs/21-article-about-dogs
in a new way it will be /component/content/article/2-category-dogs/21-article-about-dogs.

The new way does not borrow the menu item from previous page, which created a lot of duplicate contents in Google.

and editing 20+ modules to have the same user experience.

After you add a few menu items then you can assign the old modules to them. That all.

[jsubri]

I'll update the site accordingly.
Thank you

[Quy]

@jsubri Please mark your test result here: https://issues.joomla.org/tracker/joomla-cms/19280

[jsubri]

I have tested this item ✅ successfully on 0fb789d

Modern router with SEF is far better with this PR, issue with languagefilter is a separated PR
As stated above we do not have the IDs anymore and 404 are gone.
Up to other testers to provide feedback and for the production team to decide if the side effects I mentioned are bug fixes.

---

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

[Hackwar]

Hmm, I don't have a good feeling with this. From my perspective, this is not backwards compatible. it changes existing URLs from released versions of Joomla, so if we honour our dev guidelines and our commitment to backwards compatibility, we can not change this in this way.

A way to make this backwards compatible would be to add a nomenu2 rule and a switch in the components where you can switch between the different behaviors, like we have with modern and legacy routing.

Besides that, I would also ask you to change your unittests. The fact that you had to change the unittests in this way, shows that this is a break in B/C. The tests in this PR do not test the same as they did before and prior tests would fail. You are also using sample data from the database, which I would consider to be bad practice, and instead have the test-data be part of the test-suite.

[csthomas]

We have at least six routing rules for SEF:

1. Stable - with menu item
2. Stable - without menu item
3. Modern - with menu item (Remove ids NO)
4. Modern - with menu item (Remove ids YES)
5. Modern - without menu item (Remove ids NO)
6. Modern - without menu item (Remove ids YES)

This PR breaks B/C (error 404) for rule 5 (only one example below), which has been using rarely.
The released versions of Joomla 3.8 has a habit to use the other menu item to generate a link to the page without own menu item.

I wrote this PR to fix rule 6, which is completely broken.

Examples of 5 and 6 rule before and after PR:

We can start at index.php/en/component/content/categories.
Remeber to disable all menu items for categories/category/article views.

Before PR (Remove IDs is OFF - rule 5)

Build and parse:

• OK - index.php/en/component/content/category/8-category-en-gb
• OK - index.php/en/component/content/article/1-article-en-gb?catid=8
• OK - index.php/en/component/content/category/1190-subcategory-en-gb
• OK - index.php/en/component/content/article/4006-article-in-subcategory-en?catid=1190

Before PR (Remove IDs is ON - rule 6)

Build ok and the parser results:

• Error 404 - index.php/en/component/content/category/category-en-gb
• Error 404 - index.php/en/component/content/article/article-en-gb?catid=8
• Error 404 - index.php/en/component/content/category/subcategory-en-gb
• Error 404 - index.php/en/component/content/article/article-in-subcategory-en?catid=1190

After PR (Remove IDs is OFF - rule 5)

Parse old links:

• Works - as - before - - index.php/en/component/content/category/8-category-en-gb - (the same link)
• Works - as - before - - index.php/en/component/content/article/1-article-en-gb?catid=8
• Break B/C - Error 404 - index.php/en/component/content/category/1190-subcategory-en-gb
• Works - as - before - - index.php/en/component/content/article/4006-article-in-subcategory-en?catid=1190

Build and parse new links:

• The same - index.php/en/component/content/category/8-category-en-gb
• New Link - index.php/en/component/content/article/8-category-en-gb/1-article-en-gb
• New link - index.php/en/component/content/category/8-category-en-gb/1190-subcategory-en-gb
• New link - index.php/en/component/content/article/8-category-en-gb/1190-subcategory-en-gb/4006-article-in-subcategory-en

After PR (Remove IDs is ON - rule 6)

Parse old links, which does not work before:

• OK +fixed - index.php/en/component/content/category/category-en-gb
• Error 404 - index.php/en/component/content/article/article-en-gb?catid=8
• Error 404 - index.php/en/component/content/category/subcategory-en-gb
• Error 404 - index.php/en/component/content/article/article-in-subcategory-en?catid=1190

Build and parse new links:

• The same - index.php/en/component/content/category/category-en-gb
• New link - index.php/en/component/content/article/category-en-gb/article-en-gb
• New link - `index.php/en/component/content/category/category-en-gb/subcategory-en-gb
• New link - index.php/en/component/content/article/category-en-gb/subcategory-en-gb/article-in-subcategory-en

[Hackwar]

I understand that. However, a break in B/C is a break in B/C. The broken behavior in rule 6 is indeed something we need to fix and I'm currently considering if we can switch off removing the ID for that router rule. That would be extremely dirty in my book... I don't know the right solution for that right now.

The other change in behavior however is not really acceptable. Either we stick to our B/C claim or we don't and it would be extremely hypocritical if a change like this now would be handled differently than the router modernization efforts before this.

[csthomas]

For modern routing I'm not convinced to be B/C at this issue.
There is a real problem for the rule 6.

IMHO we can use an exception to fix the real problem,
but I am only a contributor, I will not decide.

I only put my solution there. I'm waiting for better solutions.
The unit tests can be changed later as you wish.

If we do not fix it at the beginning, the problem of rule 6 will grow.

Please do not leave people with choice:

• use old stable routing and be "safe"
• or use modern routing (with removed Ids) and live with error 404 as expected behaviour till J4.

[csthomas]

A way to make this backwards compatible would be to add a nomenu2 rule

OK

and a switch in the components where you can switch between the different behaviors, like we have with modern and legacy routing.

IMO there will be too many switches but If others agree I can do it.
I prefer to create nomenu2 and force it to joomla core components.

[joomdonation]

I have tested this item ✅ successfully on 614dda6

This PR solves 404 issue as described. It solves the issue #19430, too, so I think it needs to be merged for 3.8.4

---

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

[ghost]

@jsubri can you please retest?

[joomdonation]

My test is for single language website only.

[jsubri]

I have tested this item ✅ successfully on 614dda6

This PR solves #19430

IMHO, due to #19099 and #19280, 3.8.4 shall probably advertised as a bit more than a simple patch for the ones using the Modern Router.

---

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

[ghost]

Ready to Commit after two successful tests.

[infograf768]

Has someone tested this with multilingual?

[jsubri]

My initial test was on multilingual, today was English only (Sample site). Given the potential impact with such PR, addl testers can’ hurt as we had recent changes in the languagefilter.

[laoneo]

Why did you close this one? As I was integrating the new router in DPCalendar I was confronting with the same problem when there is no menu item available.

I had to do some dirty hacks to get rid of the 404 error in DPCalendar. There is an alternative approach which contains the id in #20117. But from a quick glance this here is the logical way. I mean how can a fix for an invalid url which leads to an invalid 404 be a BC break? I'm reopening it, as this bug needs to be fixed in J3.

[laoneo]

I have tested this item ✅ successfully on 614dda6

---

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

[ghost]

Ready to Commit after 3 successful tests.

[csthomas]

:)

This PR has a one weakness, it breaks B/C in only one case, but we can add a workaround for J3.

This comment is about parsing an an old links if Remove IDs is DISABLED. for modern routing.

Subject:
Break B/C - Error 404 - index.php/en/component/content/category/1190-subcategory-en-gb

To test issue please create a tree of categories:

Animal -> Dog -> Dachshund

Before PR go to:
index.php/en/component/content/category/10-dachshund

After PR the above link won't work, Joomla will build a new one, the correct will be:
index.php/en/component/content/category/3-animal/4-dog/10-dachshund

On your test, you can ignore language prefix.

To support B/C, we can scan all categories and find the correct ID, (ex: for 10-dachshund).
IMO it should be done only for J3 and in J4 the hack should be removed.

[laoneo]

For me it is not a BC break when such an obvious bug is fixed. In the release announcement we need to add then a special note.

[csthomas]

Can we push it forward until not many people use it?

[laoneo]

Maintainers feel a bit hesitant to ship it with a patch release. So I changed the base branch to 3.9.

@mbabker your turn here :)

[csthomas]

Thanks @laoneo,

3.9 is great.

[mbabker]

Bug or not it still creates a B/C break as pointed out. And that's the reason I've been hesitant on hitting the merge button, this isn't "just" fixing some bug, it has user facing impact because it changes URI structure (even if the outcome is what is desired) and inherently changes what routes can be parsed and routed correctly.

[csthomas]

We can try to add solutions like:

1. I'm not a fun of this - add changes to a new class NomenuRules2 in order to not disturb 3rd party extensions. But it will be still a problem for core extensions.

2. To eliminate error 404, add a workaround to parse old addresses by default, which gets 404 and redirect them to new URLs - it may not be an easy task.

[mbabker]

Personally I don't like either one of the options. It just seems like it's asking for trouble. But so does blatantly merging something that everyone acknowledges is a B/C break.

[csthomas]

Is there any decision from maintainers about what to do with this PR?

Should I move it to 4.0-dev branch or close it and we will wait for a better solution?

[ghost]

@laoneo can you please comment above Comment by @csthomas?

[csthomas]

Because I am not aware of the decision what to do next with this PR, this week I will try to move this PR to 4.0.

[csthomas]

Closing in favour of PR #20979