Release Blocker bug PR-5.3-dev Pending

User tests: Successful: Unsuccessful:

avatar LadySolveig
LadySolveig
17 Jun 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

avatar LadySolveig LadySolveig - open - 17 Jun 2025
avatar LadySolveig LadySolveig - change - 17 Jun 2025
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 17 Jun 2025
Category Front End com_config com_contact com_content com_users
avatar LadySolveig LadySolveig - change - 18 Jun 2025
The description was changed
avatar LadySolveig LadySolveig - edited - 18 Jun 2025
avatar coolcat-creations coolcat-creations - test_item - 18 Jun 2025 - Tested successfully
avatar coolcat-creations
coolcat-creations - comment - 18 Jun 2025

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.

avatar chmst chmst - test_item - 19 Jun 2025 - Tested successfully
avatar chmst
chmst - comment - 19 Jun 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.

avatar LadySolveig LadySolveig - change - 19 Jun 2025
Labels Added: Release Blocker bug PR-5.3-dev
avatar Hackwar
Hackwar - comment - 19 Jun 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.

avatar LadySolveig
LadySolveig - comment - 19 Jun 2025

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.

avatar LadySolveig LadySolveig - change - 19 Jun 2025
The description was changed
avatar LadySolveig LadySolveig - edited - 19 Jun 2025
avatar brianteeman
brianteeman - comment - 19 Jun 2025

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">

avatar webnet-assmann webnet-assmann - test_item - 20 Jun 2025 - Tested successfully
avatar webnet-assmann
webnet-assmann - comment - 20 Jun 2025

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.

avatar richard67 richard67 - alter_testresult - 21 Jun 2025 - webnet-assmann: Tested successfully
avatar richard67
richard67 - comment - 25 Jun 2025

@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) ?

avatar LadySolveig
LadySolveig - comment - 25 Jun 2025

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

avatar LadySolveig
LadySolveig - comment - 25 Jun 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.

avatar LadySolveig
LadySolveig - comment - 25 Jun 2025

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

avatar Hackwar
Hackwar - comment - 25 Jun 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)

avatar LadySolveig LadySolveig - change - 25 Jun 2025
Title
[5.3] Fix strict routing for frontend forms (band-aid only)
[5.3] Fix strict routing for frontend forms
avatar LadySolveig LadySolveig - edited - 25 Jun 2025
avatar LadySolveig LadySolveig - change - 25 Jun 2025
The description was changed
avatar LadySolveig LadySolveig - edited - 25 Jun 2025
avatar richard67 richard67 - alter_testresult - 25 Jun 2025 - webnet-assmann: Tested successfully
avatar bembelimen
bembelimen - comment - 25 Jun 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...)

avatar Hackwar
Hackwar - comment - 25 Jun 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.

avatar richard67 richard67 - alter_testresult - 25 Jun 2025 - webnet-assmann: Tested successfully
avatar richard67
richard67 - comment - 25 Jun 2025

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.

avatar coolcat-creations coolcat-creations - test_item - 26 Jun 2025 - Tested successfully
avatar coolcat-creations
coolcat-creations - comment - 26 Jun 2025

I have tested this item ✅ successfully on 20782c8

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.

avatar dautrich dautrich - test_item - 26 Jun 2025 - Tested successfully
avatar dautrich
dautrich - comment - 26 Jun 2025

I have tested this item ✅ successfully on 20782c8


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

avatar LadySolveig
LadySolveig - comment - 26 Jun 2025
avatar LadySolveig
LadySolveig - comment - 26 Jun 2025
avatar richard67
richard67 - comment - 26 Jun 2025

@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"
avatar brianteeman
brianteeman - comment - 26 Jun 2025

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

avatar brianteeman brianteeman - test_item - 26 Jun 2025 - Tested unsuccessfully
avatar brianteeman
brianteeman - comment - 26 Jun 2025

I have tested this item 🔴 unsuccessfully on 20782c8

See failing automated tests and explanation #45619 (comment)


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

avatar LadySolveig
LadySolveig - comment - 26 Jun 2025

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.

avatar brianteeman
brianteeman - comment - 26 Jun 2025

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

avatar brianteeman
brianteeman - comment - 26 Jun 2025

If ti helps at all the relevant code for the messages is at

$langKey = $this->text_prefix . ($recordId === 0 && $this->app->isClient('site') ? '_SUBMIT' : '') . '_SAVE_SUCCESS';
$prefix = $this->app->getLanguage()->hasKey($langKey) ? $this->text_prefix : 'JLIB_APPLICATION';
$this->setMessage(Text::_($prefix . ($recordId === 0 && $this->app->isClient('site') ? '_SUBMIT' : '') . '_SAVE_SUCCESS'));

avatar LadySolveig
LadySolveig - comment - 26 Jun 2025

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

avatar LadySolveig
LadySolveig - comment - 26 Jun 2025

$recordId = $this->input->getInt($urlVar);

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 - 5607570

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

avatar joomla-cms-bot joomla-cms-bot - change - 26 Jun 2025
Category Front End com_config com_contact com_content com_users Front End com_config com_contact com_content com_users Libraries
avatar richard67
richard67 - comment - 26 Jun 2025

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.

avatar LadySolveig
LadySolveig - comment - 26 Jun 2025

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

avatar LadySolveig
LadySolveig - comment - 26 Jun 2025

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? ec1d7bb

avatar brianteeman
brianteeman - comment - 26 Jun 2025

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

avatar muhme
muhme - comment - 27 Jun 2025

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 👍

avatar richard67
richard67 - comment - 28 Jun 2025

As the link to the downloads (and custom update URL) created by Drone for this PR are not easy to find now as we have these many additional CI checks, here is the link: https://artifacts.joomla.org/drone/joomla/joomla-cms/5.3-dev/45619/downloads/85715/

@webnet-assmann @coolcat-creations @dautrich Just if you have the time, could you test this PR again? I ping you all 3 so it's more likely to get 2 tests. So if you come to test and see here on GitHub that it has already 2 tests, you are done here, and can use the saved time for testing some other PR ;-)

Cancelled, see comment below this one.

avatar LadySolveig
LadySolveig - comment - 28 Jun 2025

Please do not perform any additional tests. I'm setting the PR on draft because it turned out that the additional fix now triggers the wrong behaviour again and the menu is not found and thus the redirects no longer work.

avatar joomla-cms-bot joomla-cms-bot - change - 28 Jun 2025
Category Front End com_config com_contact com_content com_users Libraries Front End com_config com_contact com_content com_users
avatar dpollez
dpollez - comment - 29 Jun 2025

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

avatar joomla-cms-bot joomla-cms-bot - change - 30 Jun 2025
Category Front End com_config com_contact com_content com_users Front End com_config com_contact com_content com_users Libraries
avatar LadySolveig
LadySolveig - comment - 30 Jun 2025

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?

avatar dpollez
dpollez - comment - 30 Jun 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

avatar dpollez
dpollez - comment - 9 Jul 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?

avatar coolcat-creations
coolcat-creations - comment - 9 Jul 2025

Any news on this?

Add a Comment

Login with GitHub to post a comment