This PR adds specific code for the com_content article view into the SEF plugin.
I don't think that's a good idea. The plugin should work the same for all extensions, not only for a single view.

for the moment there is no solution that works for all extensions.
but... people might be inspired with my specific solution and create a new pull request to remove the single view.
for the moment... this solution will prevent huge amount of duplicate content.

I would advice to merge this pull request solving a lot of potential SEO problems.

for the moment there is no solution that works for all extensions.

Your PR actually even removes the function from all other extensions since it changes the event to onContentBeforeDisplay which is only triggered by com_content.
If you want to go that route, you better do it in com_content directly instead of doing it in the generic SEF plugin.

you're suggesting to create a new content plugin? Removing the existing canonical url and placing a new canonical url for content item.

Instead of a content plugin I placed it into a system plugin.
https://github.com/hans2103/plg_system_canonicalurl

you're suggesting to create a new content plugin?

No. I'm saying that with your PR, you only create canonical URLs for com_content since most other extensions don't trigger the onContentBeforeDisplay event.
But if it only works for com_content, then there is no point in doing it with a plugin. Just do it within com_content (for example in the view) directly.

Another point is that this event may be triggered multiple times during a pageload. The category and featured view will trigger it for each article, and mod_articles_news does trigger it as well. It will actually produce worse canonical URLs for those pages.
This event is just not suiteable for what you try to do. It's used in core to add the voting feature and page navigations to an article.

All I want to do is create a proper Canonical URL for content.
If, as you mention as an example in your comment, it's better to move it into the view then it's even better to remove the entire trigger onAfterRoute from the sef system plugin.
Removal of the trigger is needed to prevent duplicate content. Might seem as a strange advice since the Canonical URL is meant to reduce the amount of duplicate content.

t's better to move it into the view then it's even better to remove the entire trigger onAfterRoute from the sef system plugin

Won't that potentially break SEF for all other extensions though?

nope... creation of SEF Url will not break.
only thing what will happen is that no canonical url will be added to the < head >

I wrote this pull request since the current implementation of canonical url is not correct.
(see the examples in my pull request)
But... as Thomas states correctly... my implementation is neither perfect.
Perhaps the creation of a canonical URL shouldn't be in a plugin. Perhaps it should be in the view.
I'm not a programmer... I'm just trying to solve an issue with duplicate content.

It's indeed a bit strange place for the link creation as it's not directly related to the SEF URLs.
I guess it was added there because it is only needed if SEF is enabled and it was the easiest place to add so it works for all extensions.

Moving it to the components will break backward compatibility as it removes the canonical links for non-core extensions. That is not really an option and would also result in a lot of duplicate code in the views.

Best approach would probably be to make calls to the component helper or router and run a new method which creates the canonical URLs or so. And if the method doesn't exist fall abck to current code. I'm sure there are better ways to solve the issue

Sorry, but this PR wont work in any scenario. Closing it.

Set to "closed" on behalf of @Hackwar by The JTracker Application at issues.joomla.org/joomla-cms/3491