Hi,

I've noticed an issue when the breadcrumb trail is built that it is not including any paths that have changed and the redirect module has put a redirect in place.

I'm not sure where the fix for this lies, but I thought I would document here and see where we go from there.

First off you'll need the redirect and the pathauto modules installed. For the pauthauto module settings you'll need to set [node:menu-link:parents:join-path]/[node:title]in /admin/config/search/path/patterns as the default pattern for all content.

Steps to re-produce:

  • Create a piece of content (pathauto should give it an alias)
  • Place it in the menu (top level is fine)
  • Create another piece of content and place it in the menu as a child of the above content
  • Navigate to the child page and see the parent page in the URL path and in the breadcrumb
  • Now edit the parent page and change the title - This will re-generate a new alias and create a redirect
  • Re-visit the child page and note the URL path is still the same and that the parent page has dropped out of the breadcrumb

Example:

  • Page: /leader-different-content-types is in the top level of the menu
  • This page has had its title changed at some point
  • Child page: leader-content-types/demo-content-leader-page/test-page-2 has the old path in its URL (which is setup as a redirect to: /leader-different-content-types)
  • Re-generating the URL aliases for all site content doesn't update the child URL as pathauto is happy with it.
  • A re-save of the child page doesn't update the URL alias either

So if pathauto is happy with the URL not to change for the child, should easy breadcrumb try and lookup redirects in the alias?

I'd love to hear someone elses thoughts on this.

Cheers

Tom

Comments

blacklabel_tom created an issue. See original summary.

greg boggs’s picture

I'm with you. My first thought is that you should update the child page URL when changing the parent page URL. However, if we can come up with a reasonable solution that supports redirects, that sounds pretty darned cool to me...

~G

blacklabel_tom’s picture

StatusFileSize
new2.1 KB

Hi,

I've added a VERY hacky patch to show how this could work. Is there a way to optionally add services via DI? That would be a much better way to get the RedirectRepositry that I'm using to look up the redirects.

Again the root cause for this is that when a URL alias is updated it isn't aware of the child nodes it needs to update too. This patch just attempts to treat the symptom when it gets down to easy breadcrumb.

Cheers

Tom

blacklabel_tom’s picture

StatusFileSize
new2.09 KB
tim-diels’s picture

I'm experiencing the same problem but in a slightly different context.

Our pathauto settings are the same as the reporter of the issue.
We allow to change the menu title and so the url alias is different.

Default menu title:

  • one adds a page with a default menu title, ex: Medical disciplines; so the url alias = /medical-desciplines
  • one adds a sub-page with a default menu title, ex: Anesthesia; so the url alias = /medical-desciplines/anesthesia
  • The breadcrumb for the subpage = Home / Medical Desciplines / Anesthesia

Changed menu title:

  • one adds a page with another menu title then the page, ex: Desciplines; but the url alias = /medical-desciplines
  • one adds a sub-page with a default menu title, ex: Anesthesia; so the url alias = /desciplines/anesthesia (Not what you would expect)
  • The breadcrumb for the subpage = Home / Anesthesia
  • The breadcrumb for the subpage should be = Home / Desciplines / Anesthesia

Patch #3 works for us as expected.
Patch #4 doesn't work for us as it adds both a plain text segment and the linked segment.

tim-diels’s picture

At the end the patches don't work in our situation. If the menu title for the parent node is changed, and there is no redirect, the beadcrumb fails.
I ended up using a different token: [node:menu-link:parent:url:path]/[node:title]

blacklabel_tom’s picture

StatusFileSize
new2.13 KB

New patch as the old one was causing duplicates on the site I'm working on. This is purely for me so I can keep the module patched with composer.

blacklabel_tom’s picture

StatusFileSize
new2.13 KB
feng-shui’s picture

StatusFileSize
new4.97 KB

I've taken a different approach to get this in, dropping a block code in `getRequestForPath()` which checks if the path is a redirect, and if it is, resolving it to its source before the request object does it's thing.

greg boggs’s picture

This patch needs a reroll but is otherwise ready to go.

greg boggs’s picture

Status: Active » Needs work
feng-shui’s picture

StatusFileSize
new5.86 KB

Reroll.

greg boggs’s picture

We need to add an empty updb to cache clear since this patch changes the services.

feng-shui’s picture

StatusFileSize
new6.34 KB

Added update hook.

feng-shui’s picture

Status: Needs work » Needs review
greg boggs’s picture

Status: Needs review » Reviewed & tested by the community
basvredeling’s picture

+1 for #14

neslee canil pinto’s picture

Status: Reviewed & tested by the community » Fixed

Committed, Thanks Everyone.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.