I have tested this successfully.
In my opinion it is a bug fix and should go into 5.4. It is a bug when a link item causes a 404.

I added such menu item into different articles and the fix works for all.

I have tested this item ✅ successfully on 5574c6f

The labels are not valid now. Changes are only in libraries.

I have tested this item ✅ successfully on 5574c6f

The labels are not valid now. Changes are only in libraries.

Can you update the title please

I have tested this item ✅ successfully on 5574c6f

I have tested this item ✅ successfully on 5574c6f

I would not do this.

When currently calling an menu item type URL, with index.php?Itemid=xxx you get a 404. T

This should be fixed in backend while rendering, there a method called Route::_();
Call of Route::_(index.php?Itemid=xxx); with correct ID will produce correct URl

Also suggested implementation is "risky" and could produce unexpected bugs.

Agree with Fedik. Looks more like a band aid than a real fix.

@bembelimen How do we continue here? Have you checked @Fedik 's and @LadySolveig 's previous comments?

I just add it at the same place where "alias-redirects" are handled. Using Route::_(....) renders perhaps the correct URL (the external one I guess) but it still throws an error when opening index.php?Itemid=xxx in the browser.

Alias here is a little bit different, as there is, after all, an explicit option in the menu item to set the redirect, and this is disabled by default. If I’m not getting this wrong, it’s only used in this way here, when it is set explicitly by the user).

Yeah, the alias menu is really different. It is not a "real menu".
It should not be used directly like index.php?Itemid=alias_ID in navigation,
When, for example, you turn off SEF you will get index.php?Itemid=ID where ID is the ID of the linked menu, not id of the alias menu.

You're saying, throwing a 404 should still be the solution when calling the URL menu via Itemid. Does not sound correct for me when we just can fix it to redirect to the correct URL.

You're saying, throwing a 404 should still be the solution when calling the URL menu via Itemid

Correct, for not a real menu types. The Alias menu item is not a real menu it is an alias :)
And as @LadySolveig already wrote, User have to enable redirect for this menu type when it is needed. Otherwise it should be 404

You're saying, throwing a 404 should still be the solution when calling the URL menu via Itemid

Correct, for not a real menu types. The Alias menu item is not a real menu it is an alias :)
(It was made just for "visuals", to be able to show the same menu item in diferent modules, without creating a new menu item)

And as @LadySolveig already wrote, User have to enable redirect for this menu type when it is needed. Otherwise it should be 404

Why not at least make it consistent and add an opt-in for a redirect here, just as we have in the alias, if you really want to?

Why not at least make it consistent and add an opt-in for a redirect here (Menu Item Type: System Links -> URL), just as we have in the alias, if you really want to?

As you seem to be spending a lot of time at the moment working on optimising the UX in conjunction with TinyMCE, I can certainly see where the idea comes from.
I can’t say I’m entirely convinced that a menu item, of all things, is the right choice for something like this, but I can at least see where the idea comes from.

Why not at least make it consistent and add an opt-in for a redirect here (Menu Item Type: System Links -> URL), just as we have in the alias, if you really want to?

Or, if you prefer, you could integrate it into the redirect component as an additional option that can be used across the entire site.

As you seem to be spending a lot of time at the moment working on optimising the UX iin terms of using TinyMCE, I can certainly see where the idea comes from.
I can’t say I’m entirely convinced that a menu item, of all things, is the right choice for something like this, but I can at least see where the idea comes from.

Why not at least make it consistent and add an opt-in for a redirect here (Menu Item Type: System Links -> URL), just as we have in the alias, if you really want to?

Or, if you prefer, you could integrate it into the redirect component as an additional option that can be used across the entire site.

As you seem to be spending a lot of time at the moment working on optimising the UX iin terms of using TinyMCE, I can certainly see where the idea comes from.
I can’t say I’m entirely convinced that a menu item, of all things, is the right choice for something like this, but I understand where the idea comes from.

Why not at least make it consistent and add an opt-in for a redirect here (Menu Item Type: System Links -> URL), just as we have in the alias, if you really want to?

Or, if you prefer, you could integrate it into the redirect component as an additional option that can be used across the entire site.

I can’t say I’m entirely convinced that a menu item, of all things, is the right choice for something like this, but I understand where the idea comes from.

Why not at least make it consistent and add an opt-in for a redirect here (Menu Item Type: System Links -> URL), just as we have in the alias, if you really want to?

Or, if you prefer, you could integrate it into the redirect component as an additional option that can be used across the entire site.

I can’t say I’m entirely convinced that a menu item, of all things, is the right choice for something like this, but I understand where the idea comes from.

Why not at least make it consistent and add an opt-in for a redirect here (Menu Item Type: System Links -> URL), just as we have in the alias, if you really want to?

Or, if you prefer, you could try to integrate it into the redirect component (not sure if as not lookued up yet) as an additional option that can be used across the entire site.

I can’t say I’m entirely convinced that a menu item, of all things, is the right choice for something like this, but I understand where the idea comes from.

Why not at least make it consistent and add an opt-in for a redirect here (Menu Item Type: System Links -> URL), just as we have in the alias, if you really want to?

Or, if you prefer, you could try to integrate it into the redirect component (not sure if really possible as not lookued up yet) as an additional option that can be used across the entire site.

Technically you are all right, a menuitem is not a redirect component. Never the less it's pretty annoying if you have a 404 if we can do the right thing.

So both should be don't, that Benjamin did and also route::_ should resolve the URL and directly link to the URL.

Technically you are all right, a menuitem is not a redirect component. Never the less it's pretty annoying if you have a 404 if we can do the right thing.

So both should be done, that Benjamin did and also route::_ should resolve the URL and directly link to the URL.

So both should be don't

@HLeithner “don‘t“? Or „done“?

I would simply make it the same way as in alias. something that our users are already used to. I don’t think anyone here was trying to block, just trying to play it safe.

I would simply make it the same way as in alias. something that our users are already used to. I don’t think anyone here was trying to block, just trying to play it safe. But TBH I think, we both only mentioned it, so nothing that blocks really blocks here.

I would simply make it the same way as in alias. something that our users are already used to. I don’t think anyone here was trying to block, just trying to play it safe. But TBH I think, we both only mentioned it, so nothing that really blocks here.

I would simply make it the same way as in alias. something that our users are already used to. I don’t think anyone here was trying to block, just trying to play it safe. But TBH I think, we both only mentioned it and made suggestions, so nothing that really blocks here.

I would simply make it the same way as in alias. something that our users are already used to. I don’t think anyone here was trying to block, just trying to play it safe. But TBH I think, we both only mentioned it and made suggestions, so nothing that really blocks here.
When I lookup the commits that was exactlly the initial commit, so would have been easy to say, why it was removed then.

So both should be don't

@HLeithner “don‘t“? Or „done“?

Autocorrect... It shloud be "done"

After relooking the code I think the PR is fine for what it is.
Sorry for the confusion :)

After relooking the code I think the PR is fine for what it is.
Sorry for the confusion :)

Need also test if path access also works, eg /blabla/alias-for-url-menu

Need also test if path access also works, eg /blabla/alias-for-url-menu

@chmst @brianteeman Could you test this PR again regarding this aspect? Your previous test for the testing instructions should still be valid as there has not been made any change on code, so I think it just needs to test that part. Thanks in advance.

MENU1 = top level menu item URL
MENU2 = child level menu item URL
MENU3 = top level menu alias pointint to menu 1

Before PR

Test the menu items directly

Menu 1 ✔️
Menu 2 ✔️
Menu 3 ✖️

Test the menu item inserted into article

Menu 1 ✖️
Menu 2 ✖️
Menu 3 ✖️

After PR

Test the menu items directly

Menu 1 ✔️
Menu 2 ✔️
Menu 3 ✔️

Test the menu item inserted into article

Menu 1 ✔️
Menu 2 ✔️
Menu 3 ✖️

MENU1 = top level menu item URL
MENU2 = child level menu item URL
MENU3 = top level menu alias pointing to menu 1

Before PR

Test the menu items directly

Menu 1 ✔️
Menu 2 ✔️
Menu 3 ✖️

Test the menu item inserted into article

Menu 1 ✖️
Menu 2 ✖️
Menu 3 ✖️

After PR

Test the menu items directly

Menu 1 ✔️
Menu 2 ✔️
Menu 3 ✔️

Test the menu item inserted into article

Menu 1 ✔️
Menu 2 ✔️
Menu 3 ✖️

I have tested this item ✅ successfully on 632c5fb

I have tested this item ✅ successfully on 632c5fb

I have tested this item ✅ successfully on 632c5fb

I have tested this item ✅ successfully on 632c5fb

Tested this PR together with #48138.

I have tested this item 🔴 unsuccessfully on 632c5fb

The described case works, but the redirect bypasses the menu item's access level.

Tested on 632c5fb (6.2-dev, PHP 8.3, SEF on, no URL rewriting), as guest, with curl; baseline = same instance with the patch reverted.

Testing instructions: index.php?Itemid=<url item> and /index.php/<alias> → 404 before, 301 to the configured URL after.
Child URL items and index.php?option=… links (→ SEF URL) work too.

Findings

1. Access level is ignored (blocker). The redirect runs before $this->authorise($Itemid), so a guest gets the target of a Registered/Special URL menu item.

Item IDs are sequential, so the links of all restricted URL items can be enumerated.

2. Links that are not http(s) or index.php? produce wrong redirects. redirect() prefixes everything else with the site base:

3. Any request carrying the Itemid is redirected, e.g. index.php?option=com_content&view=featured&Itemid=102 rendered the component before (200), now 301 to the external URL.

I have tested this item 🔴 unsuccessfully on 632c5fb

The described case works, but the redirect bypasses the menu item's access level.

Tested on 632c5fb (6.2-dev, PHP 8.3, SEF on, no URL rewriting), as guest, with curl; baseline = same instance with the patch reverted.

Testing instructions: index.php?Itemid=<url item> and /index.php/<alias> → 404 before, 301 to the configured URL after.
Child URL items and index.php?option=… links (→ SEF URL) work too.

Findings

1. Access level is ignored (blocker). The redirect runs before $this->authorise($Itemid), so a guest gets the target of a Registered/Special URL menu item.

Item IDs are sequential, so the links of all restricted URL items can be enumerated.

2. Links that are not http(s) or index.php? produce wrong redirects. redirect() prefixes everything else with the site base:

3. Any request carrying the Itemid is redirected, e.g. index.php?option=com_content&view=featured&Itemid=102 rendered the component before (200), now 301 to the external URL.