[5.2] SEF: Add option to don't set Itemid to homepage by default

[Hackwar]

This fixes issues #43154, #40991, #43104, #35570, #30818, #42686, #41897.

Summary of Changes

Somewhere before Joomla 2.5.0 a person decided that we need to force an Itemid in each and every case of building a URL, defaulting to the home menu item if none is set. This currently results in the URLs for core components never being allowed to hit the NoMenu rule. The end result is, that these URLs are sometimes not SEFed at all.

This PR is depending on the switch in #43432. (has been merged)

This PR contains a few changed system tests. These tests were either broken, tested broken behavior or with the latest changes were unnecessary.

I'd like to thank Joomla Agentur for sponsoring this change.

Testing Instructions

1. Install the testing sampledata
2. create an article in the uncategorised category
3. search for that article in smart search. Notice that the URL is not SEFed.
4. switch the "Disallow non-SEF URLs" in the SEF system plugin to yes
5. Reload the search result page. Notice that the URL now is at least partially SEFed with the URL starting with /component/content/article/[alias]?catid=someID

By using the switch, this is backwards compatible the way it is now. When Joomla 6.0 comes around, the code should be removed completely and the changed behavior would then be the new default.

Link to documentations

Please select:

• Documentation link for docs.joomla.org:

• No documentation changes for docs.joomla.org needed

• Pull Request link for manual.joomla.org:

• No documentation changes for manual.joomla.org needed

I checked the "no documentation changes needed" for now. I understand that some documentation for this needs to happen, but I don't want to add this to the live documentation yet when the feature has not been accepted yet.

[brianteeman]

You need to carefully look at the previous attempts to change this which result in a clusterf*k requiring a full revert

[laoneo]

This should go then into 6.0, or?

[Hackwar]

No, this can be made in a b/c way with adding a (temporary) option. This draft is here to allow us all to easily have an installable package and to test the implications. I'm not a fan of adding yet another switch, but I also don't want to throw a change like this onto everyone at once with 6.0. Adding it to 5.2 would give people a years time to give feedback and to test it and we can adjust if something unforeseen comes up.

[brianteeman]

its not unforseen when it has already happened

[Hackwar]

Then please enlighten me what already has happened.

[Hackwar]

I looked this up and there were a few attempts with #19099, #20979 and #22229, all of which have varying degrees of flaws because other code wasn't taken into consideration. This PR does, so I'm asking people to test this.

[brianteeman]

also see #19415 #19497 #19498 #19499

[brianteeman]

Not sure how this is RTC as the system tests are failing

[Hackwar]

We are aware that there is an issue here. That is the reason why it isn't merged yet. The problem most likely stems from the fact that for new installation this feature is now enabled by default and that again changes behavior which we are testing. I'm still debugging this.

[Hackwar]

I had to fix a bug in the component routers for the no-menu-behavior. So back to pending. @robbiejackson, @SniperSister could you test this again?

---

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

[Hackwar]

I edited the PR description to contain a paragraph about the changed system tests. I'm still looking for tests for this PR to add it to 5.2. Thank you!

[robbiejackson]

I have tested this item ✅ successfully on b63b69b

I tested this for com_content using the site module to display list of items in a category using both menu and non-menu rules and it worked OK.

I also patched that module code so that the catid wasn't passed to getArticleRoute() and confirmed that the old code failed but the updated code worked OK.

So this fixes a genuine problem as it could be argued that you shouldn't need the catid to form the article url as all the info is in the article id:alias.

---

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

[Hackwar]

@robbiejackson Are you sure you commented the right PR? It sounds more like you meant to comment on #43992. This PR does improve some issues, but it doesn't add the right catid by looking it up in the DB. Don't get me wrong, I'm happy for every test I can get. 😉

[robbiejackson]

Yes I'm pretty sure. In this recent version in the com_content router getArticleId you check the id is present in the query params before you use it as the catid in the database query. I just engineered a test case where it wasn't present, there were 2 articles with the same alias, and not using ids in the URLs.

[Hackwar]

Thank you!

[coolcat-creations]

I have tested this item ✅ successfully on b63b69b

Tested successfully as described.

Two personal remarks:

1. Some might disallowed component/content in their robots.txt so maybe this change or behaviour should be mentioned somewhere prominently

2. It would be nice if the "slug" component/content/article would be adjustable in future

   ---

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

[Quy]

RTC

---

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

[pe7er]

Thanks @Hackwar !