Skip to content

[5.3] Fix strict routing for frontend forms#45619

Merged
bembelimen merged 23 commits intojoomla:5.3-devfrom
LadySolveig:5.3/fix/strict-routing-band-aid
Aug 10, 2025
Merged

[5.3] Fix strict routing for frontend forms#45619
bembelimen merged 23 commits intojoomla:5.3-devfrom
LadySolveig:5.3/fix/strict-routing-band-aid

Conversation

@LadySolveig
Copy link
Contributor

@LadySolveig LadySolveig commented Jun 17, 2025

Pull Request to fix frontend form behaviour #42989.

Summary of Changes

The PR #42989 has unfortunately revealed an incorrect behaviour in conjunction for the routing for the forms in frontend. With the enabled strict routing option of the sef plugin, all redirects that can be set for actions like cancel, save and similar controller functions no longer work correctly as the menu parameters can no longer be transferred.
Reason is the missing connection to the assigned menu item as the url is not generated correctly.

  • update form actions to remove option
  • ensure correct routing for forms in frontend with strict routing enabled and disabled

This repesents a B/C break that was certainly not intended.
However, it is highly likely that this will also affect some third-party extensions.

Testing Instructions

Make sure the option for strict routing is set to yes in Plugins: System - SEF.

First Test: Create article redirect

  1. Create a menu item -> type: Create Article

grafik

  1. Set the options for redirecting by form submission and cancel

grafik

  1. Test the cancel and submission of the form and where the redirect leads to

Second Test: Login redirect

  1. Create a menu item -> type: Login Form

  2. Set the options for redirecting for login and logout

grafik

  1. Test where the redirect leads to after login and logout

Actual result BEFORE applying this Pull Request

Result for the testing instructions:
Redirect options are ignored and the final redirect always goes to the homepage.

Wrong urls in form
grafik

grafik

Expected result AFTER applying this Pull Request

Result for the testing instructions
Redirect options work and you will be redirected to the correct page after the action.

Correct urls in form
grafik

grafik

//cc @Hackwar

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

- update form actions to include appropriate views
- ensure correct routing for forms in frontend with strict routing enabled
@coolcat-creations
Copy link
Contributor

I have tested this item ✅ successfully on ba6f7a8

Tested successfully, thank you for the fix!


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

@chmst
Copy link
Contributor

chmst commented Jun 19, 2025

I have tested this item ✅ successfully on ba6f7a8

Tested in com_content + code review


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

Co-authored-by: Brian Teeman <brian@teeman.net>
@Hackwar
Copy link
Member

Hackwar commented Jun 19, 2025

This is not the correct fix. The right fix would be to remove the whole hard coded URL and instead use Route::_('index.php') instead.

@LadySolveig
Copy link
Contributor Author

Updated with the proposed fix from @Hackwar and additionally tested for user tasks - login, logout, password reset, registration and username reminder request.

If you find the time I would be very grateful if you could redo your tests. @chmst @coolcat-creations
Nothing has changed in the testing instructions, everything should lead to the same result again.

@brianteeman
Copy link
Contributor

it would be great if we had some consistency with the ordering of the parts of the form settings. in the long run it makes it easier to spot missing parts.
I would have done this as a separate PR to 5.4 to avoid polluting this PR and because its not a bug fix BUT as these are all views which might have overrides I wouldnt want to ask the user to check all their overrides on 5.3.x and 5.4

So please can we update this PR with changes to provide consistency in the order of parts
I would suggest

  • action
  • method
  • id
  • name
  • class

<form action="<?php echo Route::_('index.php'); ?>" method="post" id="application-form" name="adminForm" class="form-validate">

@webnet-assmann
Copy link

I have tested this item ✅ successfully on 07f17a5

Tested successfully. Thank you for your work!


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

@richard67
Copy link
Member

@LadySolveig Now after you have implemented the fix suggested by @Hackwar , does the "(band-aid only)" in the title still fit? Or is it a real fix now? And could you check and if not too much work implement @brianteeman 's suggestion #45619 (comment) ?

…nsistency

Consistent order:
- action
- method
- name
- id
- action
@LadySolveig
Copy link
Contributor Author

The attributes are now sorted consistently. 3fa4508 Many thanks to Brian for this suggestion.
I have adapted it slightly, as fewer changes to existing files were necessary.
The order is now as follows

  • action
  • method
  • name
  • id
  • class

Would you be so kind as to restore the test from webnet-assmann, as nothing has functionally changed. @richard67

@LadySolveig
Copy link
Contributor Author

LadySolveig commented Jun 25, 2025

Can you briefly explain what the plan could be @Hackwar
If I have understood you correctly, it definitely needs documentation for third-party developers, but the router behaviour remains as it is now, right? I definitely need your support for documentation. I'm not deep enough in the topic to be able to document it properly.
Then I can adjust the title and description as Richard requested.

@LadySolveig
Copy link
Contributor Author

Cypress test for the frontend edit would now unfortunately also have to be checked. My guess is that the test has already been written on the wrong behaviour. But I haven't had time to look at it yet.
May I ask you if you could validate this for me if you have time @muhme

@Hackwar
Copy link
Member

Hackwar commented Jun 25, 2025

The router indeed stays the way it is, since that has been the behavior of this for literally decades. I have no idea why someone thought it might be a good idea to add the option part to the URL. It only worked because of the falsely added Itemid and I have the feeling that this was also part of the reason why this behavior was introduced in the first place. Simply said: If you want to send a request to the current URL, then you have to use the current URL and you get that by simply handing in index.php. That will handle everything for you. You then only have to add additional POST parameters as hidden input fields, like you had to do before. This would also drop all the need for hidden Itemid input fields... (at least for component views. Modules might be different, for example the mod_finder module)

@LadySolveig LadySolveig changed the title [5.3] Fix strict routing for frontend forms (band-aid only) [5.3] Fix strict routing for frontend forms Jun 25, 2025
@bembelimen
Copy link
Contributor

bembelimen commented Jun 25, 2025

If you want to send a request to the current URL, then you have to use the current URL and you get that by simply handing in index.php. That will handle everything for you. You then only have to add additional POST parameters as hidden input fields, like you had to do before. This would also drop all the need for hidden Itemid input fields... (at least for component views. Modules might be different, for example the mod_finder module)

This sounds horrible. index.php is normally the start page not current page and if we have different behaviours depending where we are (component, module) it sounds like chaos. So when I want to link to a different view I have to do magic instead of just linking to the component?

We need to get this pr merged asap to overcome the broken behaviour in core (thanks for the fix @LadySolveig ) but if we copy a form somewhere else and its behaviour totally changes should ring all alerts!

(For current URL I would expect an empty action...)

@Hackwar
Copy link
Member

Hackwar commented Jun 25, 2025

You can expect an empty action, but the index.php thing has been the behavior for 18 years (and wasn't done by me). In general you have to define option and task by hidden input in POST forms and GET forms will default to the current URL. Geniuses in the past have decided that it is uncool to have option as a hidden input, because you could then see that the site is made with Joomla (eww) and thus they requested this to be encoded in the form action. All praise be the SEOs.

Honestly, I'd love the whole system to be different, but we are all working in the confines of a system which was whacked together in Joomla 1.5 and since then has not been fundamentally changed. Mainly because every change is considered to be an impossible b/c break.

Long story short: Don't shoot the messenger.

Co-authored-by: Brian Teeman <brian@teeman.net>
@LadySolveig LadySolveig marked this pull request as draft June 28, 2025 09:01
…ontroller - cast record ID to integer"

This reverts commit ec1d7bb.
@dpollez
Copy link

dpollez commented Jun 29, 2025

Possibly similar problem to PR #45254 , maybe merge these?

…r - cast record ID to integer

(cherry picked from commit ec1d7bb)
@LadySolveig LadySolveig marked this pull request as ready for review June 30, 2025 05:45
@LadySolveig
Copy link
Contributor Author

Found the time to test the previous fix myself again and cannot confirm the wrong behaviour.
I have added my reverted commit via cherry pick again. 😄

Ready for testing 🚀

@dpollez would you perhaps like to test this fix?

@dpollez
Copy link

dpollez commented Jun 30, 2025

I don't see anywhere where I can enter a formal test but then again I'm not that familiar with Github.
See my test results as an image below, hopefully clear.
I also tested the redirect of a Login Menu item and it worked correctly, both the Menu Item Login Redirect and the Menu Item Logout Redirect.

Joomla 5 3 2-rc2-dev+pr 45619 - test 01 - 30 06 2025

@LadySolveig LadySolveig marked this pull request as draft July 1, 2025 06:35
@dpollez
Copy link

dpollez commented Jul 9, 2025

I hope this will be followed up further and a solution will be quickly incorporated into a future Joomla version.
These incorrect redirects make creating and editing articles in the front-end very confusing, and I'm having a hard time explaining it to end users.
It's also unclear to me whether PR 45254 will now be deprecated and added to PR 45619; they are indeed the same problems.
I'd like to help further, but I'm not sure how?

@coolcat-creations
Copy link
Contributor

Any news on this?

@bembelimen bembelimen added this to the Joomla! 5.3.3 milestone Jul 31, 2025
@LadySolveig LadySolveig marked this pull request as ready for review August 5, 2025 12:03
@richard67
Copy link
Member

Any news on this?

As far as I can see it is ready again for testing.

So please test whoever has the time.

@dpollez
Copy link

dpollez commented Aug 6, 2025

I have tested it extensively, and it does not look very promising, as shown below.
Joomla ‎5 3 3-dev plus 45619 - test 01 - 06 08 2025
Joomla ‎5.3.3-dev plus 45619 - test 01 - 06.08.2025.pdf

@bembelimen
Copy link
Contributor

I have tested this item ✅ successfully on 0808754


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

@bembelimen bembelimen merged commit 23e460f into joomla:5.3-dev Aug 10, 2025
39 of 40 checks passed
@bembelimen
Copy link
Contributor

Thank you for this Pull Request.

We decided to merge it as it fixes the issues which were uncovered introducing the strict mode in 5.2. So we have to compare with 5.1.4 and get the behaviour back which exists there.

@dpollez your found issues still exists and should be fixed with another PR as they are related to teh router itself but not this PR

@dpollez
Copy link

dpollez commented Aug 10, 2025

Hello everyone,

I am not a developer, just a satisfied Joomla user. So please forgive my ignorance and lack of Github/Joomla knowledge, but this is getting a bit too much for me. The problem with front-end creating and editing articles is very confusing.

I reported this with issue #45248 on March 28, 2025 and others have reported it before. @ghost informed me that this was a duplicate of issue #45238, so I closed issue #45248.

Issue #45238 has already been closed, but I think PR #45254 stems from it.

@richard67 reports in PR #45254 that this is similar to PR #45619, so I tested this issue further on PR #45619.

What should I do now? PR #45254 is still open. I last tested on May 28, 2025, and the issue was almost resolved, but PR #45254 is at a standstill... because it was referred to PR #45619...

This message is posted on PR #45254 and #45619.

@richard67
Copy link
Member

I've just updated the other PR #45254 to the latest changes in the base branch (5.3-dev), which include the changes from this merged PR here , so the other PR #45254 is ready again for testing.

@LadySolveig LadySolveig deleted the 5.3/fix/strict-routing-band-aid branch August 14, 2025 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.