joomla-cms
joomla-cms copied to clipboard
[5.1] SEF: Enforce suffix
Pull Request for Issue #15025 .
Summary of Changes
Joomla has been improving its SEO performance constantly and one issue which is still open is the behavior of the suffix. Right now, you can access a URL in Joomla with or without suffix when the option is enabled in the global configuration. This PR introduces a new setting the SEF system plugin which enforces a consistent suffix behavior.
When SEF URLs are enabled, the suffix is enabled and this option is enabled, Joomla will always redirect GET requests to a URL with a suffix if none is present. It will also redirect URLs with a query format
parameter to the nicer URL and replace the "nice" suffix with the format
parameter if the two collide.
With Joomla 6.0 the option to switch this on/off should be removed again and this should be the default behavior, which would then be added to SiteRouter::parseFormat()
. Right now this is YASO (Yet Another Stupid Option) to allow people to test this in live systems and to switch it off if we encounter unforseen issues. The time from 5.1 to 6.0 could be seen as a grace period.
This PR depends on #42692.
I'd like to thank djumla GmbH for sponsoring this feature.
Testing Instructions
- Make sure that you have PR #42692 in your test installation
- Apply the patch
- Enable SEF URLs and the "Add Suffix to URL" options in global configuration
- Go to "System - SEF" plugin and enable the option to "Enforce Suffix"
- Open a URL in the frontend and see that URLs ending in a
/
don't have a suffix, everything else has a suffix. - Take a URL with a suffix and remove the suffix. See that Joomla redirects back to a URL with
.html
. - Take a URL with a suffix and add
?format=feed
to it, for example for a category blog view. See that you get the feed output with the right URL.
Link to documentations
Please select:
-
[X] Documentation link for docs.joomla.org: https://docs.joomla.org/Search_Engine_Friendly_URLs
-
[ ] No documentation changes for docs.joomla.org needed
-
[ ] Pull Request link for manual.joomla.org:
-
[X] No documentation changes for manual.joomla.org needed
This looks like it addresses a 7 year old bug #15025
I have tested this item :white_check_mark: successfully on 784cfdae7d1732ea9ac9f40e699b9c8e36ad82ab
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
I have tested this item :white_check_mark: successfully on 784cfdae7d1732ea9ac9f40e699b9c8e36ad82ab
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
I have tested this item :white_check_mark: successfully on 784cfdae7d1732ea9ac9f40e699b9c8e36ad82ab
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
In favour of the bug fix. Not in favour of the option. As such this should be in 4.4 as a bug fix and not as a new feature in a future release
Link to documentation has been added.
This is a change in behavior and not just a bugfix. That's why we wont get around this option. For Joomla 6.0 this option will be removed again and the behavior added to the normal behavior of the Add Suffix to URL
option. So as long as we will stick to semantic versioning we will have to add this as an option.
Failed. Too bad, because it's a strong request from a client of mine and for that reason SEO agency it's working with ask to migrate to another CMS...
@Nuyonuyonoina could you comment what is failing on your end? Otherwise I can't fix that and this PR wont be merged.
failed. followed the instructions, but if I remove .html from the URL it does not get added back
I have tested this item :red_circle: unsuccessfully on 784cfdae7d1732ea9ac9f40e699b9c8e36ad82ab
failed. followed the instructions, but if I remove .html from the URL it does not get added back
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
Tested on PHP 8.1.27, 10.4.32-MariaDB, Joomla! 5.1.0-alpha4 Alpha [ Kudumisha ] 20-February-2024 16:31 GMT
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
the pages can now be accessed with 3 URLs: with .html with / and without /. All show the same article
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
Did you apply the PR #42692 as well?
yes, both patches are installed and active in the patchtester
Found out, why it failed! I had yootheme installed, although not set as standard template, the framework is doing something that interferes with this patch. When I deactivate all yootheme extensions, the patch works
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
@saschadube the yootheme system plugin is keeping this patch from working. Any idea what is going on?
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
My assumption would be that they are creating the router object so early in the process that no other plugin is able to run and thus these new options can't attach themselfs to the router. To be honest, this is something Yootheme needs to fix.
I've unset @webfeuerflo 's negative test result as it has turned out that it was not caused by this PR but by a 3rd party plugin.
Thanks, the next YOOtheme Pro version will resolve the problem with booting com_media and by that creating the SiteRouter too early. 🙏
Hello @Hackwar ,
today in the maintainer meeting we decided, that we would really like to have this function in core but there shouldn't be two parameters (one in the global configuration and one on the plugin). Could you merge them, so we have only one parameter in the global configuration and no in the SEF plugin? (so remove the xml + line 74 in the "if".
I'm sorry, but in that case I rather pull this PR back and not have this in 5.1. I'm not going to take the chances of sites breaking because this is enabled by default. There are way to many sites out there which have weird code to just take the chance on this.
I have noticed to late.
Can you please do not do redirect in parseRule
?
The rule should do "rule thing", nothing more.
The same problem with https://github.com/joomla/joomla-cms/pull/42702 (that already merged and should be also fixed)
I not very concerned about extra Option, however the redirect is a bigger issue.
Revert as it's a draft.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
In the CMS maintainer meeting we talked about this again and the team agreed to the current implementation. Since it is to late for 5.1, I rebased this to 5.2, but 5.2 is in a desperate need of an upmerge... Anyway, this is theoretically ready to test.
Thank you, @SniperSister, I fixed that. @ceford, @nielsnuebel, @viocassel and of course anybody else who is intersted, would you be so kind to test this again, so that we can merge this for 5.2?
I have tested this item :white_check_mark: successfully on 5900b39ef1985e6b5c29dd7befb62c6e746c00c6
I see #42692 has been merged. I am using a cms clone - so I did a git pull them npm ci, applied the patch and I get this:
An error has occurred.
0 Call to undefined method Joomla\Plugin\System\Sef\Extension\Sef::setSiteRouter()
I needed to disable the sef plugin in the database to get admin working again.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
I have tested this item :red_circle: unsuccessfully on 5900b39ef1985e6b5c29dd7befb62c6e746c00c6
I see #42692 has been merged. I am using a cms clone - so I did a git pull them npm ci, applied the patch and I get this:
An error has occurred.
0 Call to undefined method Joomla\Plugin\System\Sef\Extension\Sef::setSiteRouter()
I needed to disable the sef plugin in the database to get admin working again.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42850.
@ceford The comment about #42692 actually isn't true anymore and that PR actually has been reverted before the release of 5.1. Since this PR also doesn't touch that code, your git clone most likely is very outdated and should be updated to latest 5.2-dev before testing this.