Perfect example of why I said

Without a test suite that will measure all the types of urls that can be exist now and that this, and any other future changes, can be measured against this should be reverted. Too risky especially as the "gains" are so minor and can be addressed without touching any code as already discussed.

@brianteeman we are planing a dedicated “router test suite” sprint for that reason :) so your feedback has been heard and appreciated!

great to here I was partialy listened to. Shame that it has taken precited issues taking place AFTER the merges. Would save a lot of hurt if these untested changes had not been merged until after the tests were created. After all at least one of them was addressing a multiple year old issue

I have tested this item ✅ successfully on 81b7afe

I've tested as follows:

I've added some error logging to the code to see when which rule is attached and made sure that PHP errors are logged into a log file.

E.g. here the code with the PR applied and with my additional error logs:

        if (
            $app->get('sef')
            && !$app->get('sef_suffix')
            && $this->params->get('trailingslash', -1) != -1
        ) {
            if ($this->params->get('trailingslash') == 0) {
                // Remove trailingslash
                $router->attachBuildRule([$this, 'removeTrailingSlash'], SiteRouter::PROCESS_AFTER);
                // DEBUG
                error_log('DEBUG: Rule "removeTrailingSlash" attached.');
            } elseif ($this->params->get('trailingslash') == 1) {
                // Add trailingslash
                $router->attachBuildRule([$this, 'addTrailingSlash'], SiteRouter::PROCESS_AFTER);
                 // DEBUG
                error_log('DEBUG: Rule "addTrailingSlash" attached.');
           }
        }

Then I have tested the 3 values for the "trailingslash" parameter of the SEF plugin by selecting that value, sabing the plugin settings and then calling the frontpage and watching my error log.

Without the changes from this PR, only the DEBUG: Rule "removeTrailingSlash" attached. can be found in the PHP error log when the value of the "trailingslash" parameter is 1, i.e. "Enforce URLs with trailing slash", or when it is -1, i.e. "No change", but not when it should be, and the DEBUG: Rule "addTrailingSlash" attached. can never be seen in the PHP error log.

With the changes from this PR applied, the debug logs appear as expected:

• Nothing when the value of the "trailingslash" parameter is -1, i.e. "No change"
• DEBUG: Rule "removeTrailingSlash" attached. when the value of the "trailingslash" parameter is 0, i.e. "Enforce URLs without trailing slash"
• DEBUG: Rule "addTrailingSlash" attached. when the value of the "trailingslash" parameter is 1, i.e. "Enforce URLs with trailing slash"

tested this (https://raw.githubusercontent.com/joomla/joomla-cms/81b7afeda08c2dce8aec9b34267220d086e6af0b/plugins/system/sef/src/Extension/Sef.php) successfully on a live-site.

@rfmjoe Could you mark your test result in the issue tracker so it's properly counted? For doing this, go to this PR in the issue tracker here https://issues.joomla.org/tracker/joomla-cms/43292 , use the blue "Test this" button at the top left corner, select your test result and submit. Thanks in advance.

Thank you @SniperSister ?? and also for testing @richard67 and @rfmjoe !