Fixing tag routing #5347
Merged
Fixing tag routing #5347
Conversation
While this fixes the router up, the TagsHelperRoute class is fixed in #5105. So please have a look there, too. Apply both PRs and it should make a world of difference. |
Closed
@test - works as described |
@test success |
@test works for me in various different tags views. Merging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
This is an improved and complete fix of #4971. Copying the testing instructions to here:
Steps to reproduce the issue
Install Joomla (Test English (GB) Sample Data), for example
From the backend set the configuration ‘Search Engine Friendly URLs’ = ‘YES’
Use the url: index.php?option=com_tags&view=tag&layout=list&id[0]=3 (not sef url into a sef one. The id can have another value and NO Itemid has to be set)
You will have 3 times the error:
Notice: Array to string conversion in /libraries/cms/router/site.php on line 465
The proposal to fix is to replace
$id = $query['id'];
with
$id = (int) $query['id'];
which seems coherent with the test line 80
Expected result
No error
Actual result
Notice: Array to string conversion in /libraries/cms/router/site.php on line 465
System information (as much as possible)
Joomla 3.3.6
I further cleaned up the router. Basically it should work the same as before, except that that notice is gone. And we have quite a bit less code.😉