404 errors should be harmonized

[Hlavtox]

What is wrong

Depending on type of the object, there are multiple ways 404 errors are handled for objects that don't exist.

Redirect vs 404 template

• Manufacturer and Supplier controllers are redirecting to 404 controller, if the object can't be loaded.
• Category and Product controllers are staying on the current page, but setting a 404 template to be used.

Different look of pages

For products, it's showing broken breadcrumbs and an error.
URL stays https://www.domain.cz/categoryrewrite/123-productrewrite.html

For categories, only the message.
URL stays https://www.domain.cz/123-categoryrewrite

For 404 controller, only the message.
Page is redirected to URL https://www.domain.cz/index.php?controller=404

TODO

• Unify the way 404 errors are handled. Redirect or template changes?
• Setup a rewrite for 404 error controller.
• Verify that all ways to get to 404 errors behave the same.
  • A completely nonsense URL that can't be matched by the dispatcher.
  • Objects that don't exist.
  • Objects that are disabled or inaccessible.

[maniT4c]

Hello,
for SEO, it's important to keep the url instead of redirect to the index.php?controller=404 page.
And I think the breadcrumb is useless, except if we are able to know in which category the page was, but I think it's not possible if the object doesn't exist anymore.

[Eolia]

Since when is a 404 page important for SEO?

[Hlavtox]

@Eolia If your server is providing misleading responses, you will have pages indexed that you should not have, robots wasting crawl budget and other issues.

[Eolia]

Since when is a 404 page indexed? The server is responding that the page doesn't exist (404 response); that's perfectly clear. The problem lies with the merchant, who has to manage their product deactivations/deletions.

[Eolia]

There is no redirection to do, just assign the controller type:
$this->controller_type = '404'; $this->errors[] = Tools::displayError('Product not found');
and in Controller.php:
public function setTemplate($template) { if ($this->controller_type == '404') { header('HTTP/1.1 404 Not Found'); header('Status: 404 Not Found'); $template = _PS_THEME_DIR_.'404.tpl'; } $this->template = $template; }

[Hlavtox]

@Eolia Yes, but the problem is that some pages redirect to 404 controller instead of staying on the curent URL and showing the 404 response.

[pvujic]

Hello, for SEO, it's important to keep the url instead of redirect to the index.php?controller=404 page. And I think the breadcrumb is useless, except if we are able to know in which category the page was, but I think it's not possible if the object doesn't exist anymore.

If a category was deleted tracing the category back will be impossible, but the breadcrumb would be useful in case the object is disabled or inaccessible.

In case of a product we could check the ID in the URL to get the category.

[Hlavtox]

@maniT4c @pvujic

Yes, I agree. I did some research and apparently keeping the URL with a 404 response is a better solution than a redirect to PageNotFoundController, because it makes the errors harder to track and detect.

One downside is that if we set the 404 template like category and product controller does ($this->setTemplate('errors/404')), it still runs many of the code of the controller, hooks etc. And we have to settle for things like $this->notfound etc.

It would be best if we were able to run the PageNotFoundController controller directly, but keeping the URL.

And I think the breadcrumb is useless

Yeah, that's just one of the bugs and incosistencies that I wanted to show.

If a category was deleted tracing the category back will be impossible, but the breadcrumb would be useful in case the object is disabled or inaccessible.

We don't have to worry about tracing the category. If we can't find any content for the URL, just leave it be. :-)

[kpodemski]

I converted this issue to the Idea since it fits better here.

[Progi1984]

Fixed by #41299

[Hlavtox]

@Progi1984 Not really, it was just one of the pages.