[5.4] Fix condition to check request format in MenusHelper - deprecation warning

[MacJoom]

Pull Request for Issue #45707

Summary of Changes

Check for index.php? instead of index.php before entering parse_url

Testing Instructions

As in described in the issue:

1/ Create a menu item "URL" type - enter just index.php
2/ Set errors report to maximum in global config
3/ A warning shows up in the menu item (backend)

Actual result BEFORE applying this Pull Request

Deprecated: parse_str(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/clients/client2/web46/web/administrator/components/com_menus/src/Helper/MenusHelper.php on line 76

Expected result AFTER applying this Pull Request

No warning

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

[brianteeman]

I have tested this item 🔴 unsuccessfully on f8d1efd

This does remove the notices but the link does not work
Any url in the format index.php/link will not work (make sure you test from a page that is not the home page

---

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

[MacJoom]

I have tested this item 🔴 unsuccessfully on f8d1efdThis does remove the notices but the link does not work Any url in the format index.php/link will not work (make sure you test from a page that is not the home page

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46341.

Yes i see - but this is yet another bug - happens without the patch as well - only happens if you do use things like index.php/team works if you use only /team
We could just replace index.php/xxx with /xxx

[brianteeman]

urls in the format index.php/xxx are created when you enabled SEF urls (the default) and do not enable URL rewriting (the default)

[MacJoom]

Can we just pass this PR that fixes the original issue and open a new issue for the index.php/ bug. Needs to be fixed somewhere else. In the router i guess

[exlemor]

I have tested this item ✅ successfully on f8d1efd

I have tested this successfully. Love to see Deprecation messages fixed! :)

Thanks @MacJoom!

---

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

[dautrich]

I have tested this item ✅ successfully on f8d1efd

---

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

[richard67]

Can we just pass this PR that fixes the original issue and open a new issue for the index.php/ bug. Needs to be fixed somewhere else. In the router i guess

@brianteeman Could you check and possibly change your test result?

[brianteeman]

It fixes the deprecation but doesn't fix the URL. For me there's no point in fixing one without the other.

[richard67]

It fixes the deprecation but doesn't fix the URL. For me there's no point in fixing one without the other.

@brianteeman But does that justify an unsuccessful test result? The PR doesn't claim to fix the URL.

[brianteeman]

for me its all related. It might be if the url is fixed so that it works then this PR is redundant

[ceford]

I have tested this item ✅ successfully on f8d1efd

For me, with the URL set to index.php and the patch applied the link always reloaded the current page. The current page path is being appended after index.php/ - I would be inclined to treat what it is supposed to do as a separate issue.

---

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

[tecpromotion]

RTC

---

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

[MacJoom]

On further investigation this PR is not needed, because entering just index.php/xzy is wrong or at least not intented behaviour.
We should make clear that entering index.php/xyz in a System Links URL works only at home page level. on a subpage 'index.php/xyz ' is relative to the subpage and will fail accordingly. The correct usage would be /index.php/xyz - in which case this PR is not needed. Every System Link URL should start with http, https or / - i don't think we should fix user error. May be some message in the Backend helps

[brianteeman]

we should validate the link and warn with appropriate message

[brianteeman]

Also I just tested on a site with htaccess sef enabled and it works perfectly without the / in that scenario