Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apply sef autocorrection #16758

Closed
wants to merge 3 commits into from
Closed

Conversation

alex7r
Copy link
Contributor

@alex7r alex7r commented Jun 19, 2017

Summary of Changes

Add (and enable by default) SEF url auto correction by redirecting to appropriate url.

Testing Instructions

Create an article in category and use aliases like:
cagetory: subcategory
article: qtestucat
Correct link for the article is /index.php/8-subcategory/2-qtestucat
You should have access only for correct urls.
And 404 or 301 at incorrect urls.

Expected result

You can't access article by /index.php/8-subcategory/2-qtest(?somevar=exists)
it redirects you to /index.php/8-subcategory/2-qtestcat(?somevar=exists)

Actual result

You can access article by /index.php/8-subcategory/2-qtest(?somevar=exists)

@brianteeman
Copy link
Contributor

this works as described and its great

but it has other consequences which may or not be desired

create two menu items to exactly the same thing eg category list to the same category
before this pr you would have /menu1 and /menu2
after the pr /menu2 would always redirect to /menu1

Also I think that because this can change the current behaviour of a site as above then it should be disabled by default

@alex7r
Copy link
Contributor Author

alex7r commented Jun 19, 2017

No it should not, as Itemid is respected.
But I will retest.

@alex7r
Copy link
Contributor Author

alex7r commented Jun 19, 2017

Ok, I've re-tested and yes, the situation is there:
plugin respects Itemid and com_content do not.

@brianteeman so do you think system should encourage settings that are bad for SEO by default?

Or should I change joomla installation SQL so plugin is enabled on install? It will fix the issue for new sites and allow to preserve this 'bad feature' for existing installations by default?

@ghost ghost added the J3 Issue label Apr 5, 2019
@ghost ghost removed the J3 Issue label Apr 19, 2019
@particthistle
Copy link
Member

particthistle commented Jan 18, 2021

@alex7r just wondering if this was a PR you're going to further pursue?

From experience, the need to end up with 2 urls for otherwise identical items is common enough to not apply the PR. This might be due to a need to assign different modules to different pages, or show different templates for some reason (eg main template vs a stripped back landing page)

Autocorrection I know would be something I'd turn off if it was a default feature, and if I did need a redirection in place, I'd just manually add it in the Redirect component, so I'm possibly not a target audience for what you're attempting to do.

Happy to discuss more what it is the PR is trying to achieve.

Looking at this PR as part of a "Outstanding PR Status List" exercise to work on reducing the number of outstanding PR that have been otherwise orphaned, neglected or forgotten instead of being merged. See Glip Bugs & Fun @ Home channel for more information.

@laoneo
Copy link
Member

laoneo commented Mar 18, 2022

Sorry that it has taken so long to move on with this pull request. As time was flying, Joomla evolved and new features should be ported to 4. Would you mind to rebase your pr to the latest 4.2 branch? Also update your code to use the namespaced classes. In the meantime I close the pr. When ready to be tested with Joomla 4.2, don't hesitate to reopen again. Thanks for your contribution, making Joomla 4 better.

@laoneo laoneo closed this Mar 18, 2022
@laoneo laoneo closed this Mar 18, 2022
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators labels Mar 18, 2022
@joomla joomla deleted a comment from laoneo Mar 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants