codefog / contao-news_categories

Extend the Contao news module with categories
MIT License
31 stars 25 forks source link

Implement canonical tag for Contao 4.13 and up #247

Closed fritzmg closed 1 year ago

fritzmg commented 1 year ago

Currently news_enableCanonicalUrls has no effect in Contao 4.13. May be the original intention was to manually define the canonical tag to reference itself in the page's settings - however consider the following example:

Assume you only have one newscategories module - but you want to integrate it into 20 pages. You would now need to manually set the canonical tag on all these pages and you would need to remember to do it every time you integrate this module on a new page.

Instead news_enableCanonicalUrls should still be supported in Contao 4.13 and up. In fact it is intended for modules to manage the canonical tag whenever a module uses Contao's path parameters via Input::get() - as Contao will not automatically set a canonical URL to the actual page for such parameters.

fritzmg commented 1 year ago

I see you have already dropped the support in c2aa43aba6dc7b45124a979e7fc34d087036ec3b

However, if we agree that this feature should be kept I can make a separate PR for master.

qzminski commented 1 year ago

I think we can bring this feature back. Do you need it for 3.4.x as well? If not, I would prefer it to have the PR against master only.

fritzmg commented 1 year ago

Do you need it for 3.4.x as well?

In our case yes, as these instances cannot upgrade due to other dependencies not allowing Haste 5 over which we have no control. I thought the 3.4 branch is still actively supported since it had bugfix releases together with 3.5. But if not I can continue using my fork there.

qzminski commented 1 year ago

It is actively supported so I will merge it right away, thanks 🙂

fritzmg commented 1 year ago

Cool! However, you also merged this upstream into master - that won't work because the field got removed there altogether. I'll provide a separate PR to fix this in master.