[5.3] Fix strict routing for frontend forms (band-aid only)

[LadySolveig]

Pull Request to fix frontend form behaviour #42989.

Summary of Changes

The PR #42989 has unfortunately introduced an incorrect behaviour when creating 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 include appropriate views
• ensure correct routing for forms in frontend with strict routing enabled

It's not a complete fix, but at most a band-aid. In my opinion, this should definitely be finally fixed in the router and represents 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

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

3. 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

3. 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

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

//cc @Hackwar

Link to documentations

Please select:

• [ ] Documentation link for docs.joomla.org:

• [ ] No documentation changes for docs.joomla.org needed

• [X] Pull Request link for manual.joomla.org: <TBD>

• [ ] No documentation changes for manual.joomla.org needed

[coolcat-creations]

I have tested this item :white_check_mark: successfully on ba6f7a8a71d2edf1790d7fe2fc67d6a93231ebc5

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]

I have tested this item :white_check_mark: successfully on ba6f7a8a71d2edf1790d7fe2fc67d6a93231ebc5

Tested in com_content + code review

---

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

[Hackwar]

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]

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]

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]

I have tested this item :white_check_mark: successfully on 07f17a5d28eb7036f489a3394443247d45f0da9c

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]

@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 https://github.com/joomla/joomla-cms/pull/45619#issuecomment-2989276397 ?

[LadySolveig]

The attributes are now sorted consistently. 3fa4508d22b56fcd7fb3c11ccfcb0e898b61b640 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]

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]

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]

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)

[bembelimen]

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]

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.

[richard67]

I've just restored again the human test result in the issue tracker as all changes after that were only changes in the order of html attributes, which I've just reviewed with success, so there was no functional change since the test.

One more test needed.

[coolcat-creations]

I have tested this item :white_check_mark: successfully on 20782c8bbacb9fdf7d9de1a9e7c72b793ccc1703

Tested independently. Reverted the applied patch. Redirect does not work. Applied updated patch, redirect works. Thank you for the Fix!

---

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

[dautrich]

I have tested this item :white_check_mark: successfully on 20782c8bbacb9fdf7d9de1a9e7c72b793ccc1703

---

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

[LadySolveig]

Thank you for Testing @coolcat-creations @dautrich @webnet-assmann

[richard67]

@LadySolveig Can it be that the system tests need some adjustment to this PR? Currently they are consistently failing for both database types at

Running:  site/components/com_content/Article.cy.js                                    (59 of 148)

  Test in frontend that the content article form
    1) can edit an article in menu item
    2) "after each" hook for "can edit an article in menu item"

[brianteeman]

The problem is that the wrong success message is now being used.

Before this PR it was "Article Submitted" and thats what the test is looking for

With this PR it is now "Article Saved" - hence the failure

[brianteeman]

I have tested this item :red_circle: unsuccessfully on 20782c8bbacb9fdf7d9de1a9e7c72b793ccc1703

See failing automated tests and explanation https://github.com/joomla/joomla-cms/pull/45619#issuecomment-3008833574

---

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

[LadySolveig]

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

exactly what I wrote yesterday.

[brianteeman]

The correct and intended message when submitting a new article is "Article Submitted" not "article saved" . Saved should be on edits not new article submissions,

For example on one of my sites I have a language override for "article submitted" which says "Thank you for your submission it will be reviewed by an editor before publishing". With the changes in this PR that language override is not used and its not appropriate to edit the "article saved" message

[brianteeman]

If ti helps at all the relevant code for the messages is at https://github.com/joomla/joomla-cms/blob/1594ea7cd0dde2aa8eef9e6e3c010f8518208048/libraries/src/MVC/Controller/FormController.php#L726-L729

[LadySolveig]

Thank you @brianteeman I have opened it now and have a look, that helps me a lot!

[LadySolveig]

https://github.com/joomla/joomla-cms/blob/1594ea7cd0dde2aa8eef9e6e3c010f8518208048/libraries/src/MVC/Controller/FormController.php#L569

With the new url index.php&a_id the line above gives null as result with the old one index.php&option=com_content&a_idit gives 0

I think with this fix from you this will be stripped out right?

Revertet ~Would fix it this way, but not sure if this could affect anything else. Can you also have a look? @Hackwar @bembelimen This is the commit - 560757088c1ba4598a3330eae3ab9d8c5f8faffa~

~Would you be so kind as to test again whether it now works as expected? @brianteeman At least the system tests are now running again. But since it is the global FormController that I have now changed, maybe one or two tests in the backend would be good to validate that something has not got mixed up here. I have tested and I have not noticed anything, but I would be happy about validation.~

too soon sorry

[richard67]

Would fix it this way, but not sure if this could affect anything else. Can you also have a look? @Hackwar @benjaminpick

@bembelimen I guess you were meant.

[LadySolveig]

The test now throws a php warning here because the key is not there. So another problem with the striped id from the url.

[LadySolveig]

Would you be so kind as to test again whether it now works as expected? @brianteeman At least the system tests are now running again. But since it is the global FormController that I have now changed, maybe one or two tests in the backend would be good to validate that something has not got mixed up here. I have tested and I have not noticed anything, but I would be happy about validation.

Could you also have a look @bembelimen if my new fix could break anything I have overlooked? ec1d7bba747ebed3c204f9e5cb97888669c81c2c

[brianteeman]

The issue I commented on regarding the string on new article submissions has now been resolved.

[muhme]

May I ask you if you could validate this for me if you have time @muhme

@LadySolveig Sorry, I didn't have time earlier. And I see you solved the problem with the System Tests yourself, which is the best solution anyway :+1: