[4.0] Fix build SEF URL for component view without own menu item

[csthomas]

Pull Request for Issue #18923

Summary of Changes

1. Always add prefix /component/[COMPONENT_NAME]/ if menu item component is not equal to query[option].
2. Optimise code, remove a code that set again $query['Itemid'].
   I removed a way when the build rule can change menu route by changing value in query['Itemid'].
   This example describe old way that I removed.

// Store Itemid in other variable
$itemID    = empty($query['Itemid']) ? null : $query['Itemid'];

$crouter   = $this->getComponentRouter($component);
// Now build rule can change query['Itemid']
$parts     = $crouter->build($query);

// Restore Itemid if deleted
if (empty($query['Itemid']) && $itemID)
{
	$query['Itemid'] = $itemID;
}

// After that we load menu item
$item = empty($query['Itemid']) ? null : $this->menu->getItem($query['Itemid']);

Testing Instructions

See issue and code review.

Expected result

SEF links is build in correct way.

Note:
IMO similar PR also can be applied to J.3.x

[infograf768]

Tested this patch, it solves the issue of creating a new account from the module when we do not have a Registration Menu item.

[csthomas]

Thanks, I will change one more line, replace isset() to empty(), then almost everything will be done, there is still problem with unit tests.

[joomdonation]

Could you please explain why you get $item from $query['Itemid'] before component router build process? Look like this change the original logic of the routing

As I understand, the original code give component router a chance to find correct Itemid (logical to me) for the routing, so we should try to get $item after component build process rater than before. So the change like this 4.0-dev...joomdonation:patch-2 seems more safe?

[csthomas]

Could you please explain why you get $item from $query['Itemid'] before component router build process? Look like this change the original logic of the routing

In general, build URL process is split into 3 stages: (preprocess, build and postprecess).

Itemid is set up in preprocess stage, ex: MenuRules::preprocess()
SiteRouter::buildSefRoute() is called in build stage and calls StandardRules::build().

Maybe you would agree with me that StandardRules::build() or NomenuRules::build() should not change $query[Itemid] to different valid value.

Non SEF process does not call build stage.
IMO allowed option to change Itemid in build stage is wrong.

[joomdonation]

Thanks for explanation @csthomas , it is clear to me now (and makes sense, too). I can mark this PR as success but would like to see @Hackwar confirm this change doesn't cause any potential issues?

[Bakual]

I have tested this item ✅ successfully on d55cd2a

This solves my issues reported in #18952.
Imho this is a very important fix as otherwise we will see issues arise with a lot of sites where there are no specific menuitems for a given component.

---

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

[infograf768]

I have not tested this item.

---

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

[joomdonation]

I have tested this item ✅ successfully on d55cd2a

The PR solves the reported issue. I also did a code review, too

---

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

[ghost]

Ready to Commit after two successful tests.

[anuragteapot]

I have tested this item ✅ successfully on d55cd2a

With PR . I have tested this item ✅ successfully.

---

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

[Hackwar]

The code change looks good and the logic is correct. I would sign off on this. 😉

[joomdonation]

@wilsonge This PR should be merged, please

[wilsonge]

Thanks :)

[joeforjoomla]

This code breaks several components logic out there that rely on the router to find an Itemid #19829

[csthomas]

@joeforjoomla Yes, this PR breaks it.

When you build a URL you can add your own Itemid to the query, otherwise core rules MenuRules will do it for you based on match to menu items.

Before (3.x) you could re-change Itemid again in SEF build method of your component router.

My purpose was:

• SEF and non SEF build should return the same Itemid
• the component router rule build is to build path in component, not to repair menu item id.