My path-based custom breadcrumbs were not working. I kept getting the first (id-wise, and most likely being shortest matching path as these often get created first before 'deeper' paths are needed).

Looking at the code for beta3, it appears that the 'gist' of the logic to find the best-fit path custom breadcrumb was there, but the devil is in the detail. That is, the current code was using substr_count to get the $num var which, I believe, was intended to be the position/depth of the wildcard asterisk char in the path being tested for a match. Rather, this function gives a count of the occurrences of the wildcard char which is most often (at least in my situation) 1. This position/depth number was then compared with the $min var to determine if the currently tested path is a better fit for the path than the best fit seen so far (in the iteration of the available custom breadcrumbs). Here, too, the logic seemed akimbo in that $num was tested for a 'less than' condition to $min.

At least for my situation, the best fit path is the longest matching custom breadcrumb. Therefore the patch here uses strrpos to get the postion/depth of the wildcard char and then does a 'greater than' test of $num versus $min, thereby accepting the longer path as a better fit.

While this is working for my situation, I can't say that this is the right thing to do in all cases. But I think it is. If so, then this patch should make its way into the custom_breadcrumbs_paths.module codebase. Otherwise, if you are experiencing the same issue that I am, this simple patch should work for you too.

CommentFileSizeAuthor
custom_breadcrumbs_paths.module.patch751 bytesSohodojo Jim
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MGN’s picture

Category: bug » support
Status: Needs review » Postponed (maintainer needs more info)

The current code counts the instances of the wildcard character and selects the custom breadcrumb that matches with fewest number of wildcards. A no-wildcard match takes precedence over a match using one wildcard, a one-wildcard match wins out over a custom breadcrumbs path breadcrumb that has two wildcards, etc.

For example, lets say there are custom breadcrumbs defined for the following paths

1. path/*/some/*
2. path/to/some/*
3. path/to/some/content

When viewing the page at path/to/some/content, breadcrumb 3 should be selected.
When viewing the page at path/to/some/page, breadcrumb 2 should be selected.
When viewing the page at path/at/some/page, breadcrumb 1 should be selected.

This was instituted in #513702: Weigh / Re-order custom paths. Hopefully this example helps explain the intention in the current code.

So I believe the current code is correct, by design, but that doesn't mean that your approach shouldn't also be considered. Unfortunately there wasn't any discussion on that previous issue, and the more feedback the better!

Can you provide a specific example that illustrates a case not handled correctly in the current code?

Thanks for the feedback.

Sohodojo Jim’s picture

Hi Michael,

Thanks for the explanation. This is helpful.

I'll factor this info into my use case and determine why I wasn't getting the results anticipated and if it makes sense to broaden the cases by which path matches are made. Otherwise, I'll just maintain it as a "whatever I need to do to get it to work" adjustment.

It may, however, be that I'm not getting the anticipated result because I was fuzzy on how best to write a set of overlapping/tiered path specs for effective matching. I'll take your info into account and do a bit of experimenting.

More soon,
--Jim--

MGN’s picture

Category: support » feature
Status: Postponed (maintainer needs more info) » Active

Thanks, I think the current code can be modified to suit both approaches. I think both the depth where the wildcard(s) appears and the number of wildcards both need to be considered...

Marking this as an active feature request for now.

SamH’s picture

I think that matching based on the depth where the first wildcard appears is the better solution, and support adding Jim's patch to the code base.

Matching based on the number of wildcards has two main problems. First, it favors simple breadcrumbs with one wildcard over more specific breadcrumbs with multiple wildcards. For example, with the following custom breadcrumbs defined:

1: path/*
2: path/to/*/preferences/*

When we view the page "path/to/user-id/preferences/video", breadcrumb 2 should be selected, because it is more specific. However, breadcrumb 1 is selected instead, because it has fewer wildcards. Depth-based matching would correctly select breadcrumb 2.

Second, matching based on the number of wildcards does not handle ties effectively. Like Jim, almost all of my custom breadcrumbs have 1 wildcard, for example:

1: path/*
2: path/to/*
3: path/to/some/content/*

Depth-based matching will correctly select the most specific breadcrumb, while matching based on the number of wildcards will treat them all equally, and select the path with the first id.

Is there a case where matching based on the number of wildcards will work but depth-based matching will not? Depth-based matching works on the examples in #2.

MGN’s picture

Version: 6.x-2.0-beta3 » 6.x-2.x-dev
Status: Active » Fixed

Thanks for the feedback. I agree that using the depth of the first wildcard is the most intuitive approach, so I've committed a slightly modified version of Sohodojo Jim's patch to 6.x-2.x-dev. I really just updated the variable names replacing min with max, num with pos, etc. Thanks for contributing!

Status: Fixed » Closed (fixed)

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