[5.1] fix wrong parameter value of new trailingslash parameter in SEF plugin

[SniperSister]

trafficstars

Summary of Changes

The new trailing slash parameter, introduced in #42702 had received another patch that changed the parameter values in the XML, see #43195. That second change was incomplete, leaving the onAfterInitialise function unpatched.

This PR fixes the issue.

Testing Instructions

See #42702 and/or code review (tbh it's a no-brainer as we are obviously comparing with a non-existing value...)

Actual result BEFORE applying this Pull Request

• Build rules not attached

Expected result AFTER applying this Pull Request

• Build rules attached

[brianteeman]

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.

[SniperSister]

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

[brianteeman]

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

[richard67]

I have tested this item :white_check_mark: successfully on 81b7afeda08c2dce8aec9b34267220d086e6af0b

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"

---

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

[rfmjoe]

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

[richard67]

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

[LadySolveig]

Thank you @SniperSister 👍🏼 and also for testing @richard67 and @rfmjoe !