Views breadcrumb: Wrong href for titles

Looking into includes/view.inc, view::_build_arguments():
http://drupalcode.org/project/views.git/blame/refs/heads/7.x-3.x:/includ...

  function _build_arguments() {
    [..]
    $breadcrumb_args = array();
    [..]
    foreach ($this->argument as $id => $arg) {
      [..]
          $path = $this->get_url($breadcrumb_args);
      [..]
        $breadcrumb_args[] = $arg;

The $breadcrumb_args[] = $arg; comes too late. When the url is built, it cannot consider all the arguments that it should consider.

Proposed fix.
Btw, I wonder why noone else has reported this yet? Seems like quite a basic misbehavior.. Or did I do something wrong?

Btw, here is where I come from: #1914718-31: Crumbs on Views with Arguments/Contextual Filters

A test case would be nice..

You know people are so used to the fact that breadcrumbs are problematic in drupal. Maybe this will change one day with the crumbs module :)

The "will" and "one day" indicate that you have not tried it yet :)
Since I made this module, I never had any problem with breadcrumbs that I couldn't solve.

Ideally, Views would set a title callback in hook_menu(_alter). (Ideally, this title callback would not need to load and process the entire view, but just the argument info..) Then Crumbs wouldn't need to do anything extra.

But since this is probably not so easy, we are going to make a plugin for views, which should look into drupal_get_breadcrumb() (which it normally ignores), and extract the titles from there. So we just need the title/href of breadcrumb elements fixed :)

Maybe we should set a small documentation so we never do the same mistake again?

Hm, I am confused, what do you want to document?

Does this have a future? rtbc?
Seems quite straightforward to me..

Related: #616922: Set title in menu_router table

Maybe I see it wrong, but for me this is a bug, not a documentation issue.
Commit the patch, and we are done.
*Unless* this patch has some unpleasant side effects.

What additional documentation do we need?

Ok, I think it is good this was not committed yet.

There is one aspect I missed.
E.g. you have the typical News > (year) > (month) > (day)

You configure the "day" display, the one with 3 contextual filters.
You configure the "day" argument of that display.
For "title" you would put "%3" (but probably you just don't).
But for "breadcrumb name" you put %2, to specify the title of the parent breadcrumb item.

This is because by default, Drupal breadcrumbs never show the current page, they only show from Home to the parent page of the current page.

Read this way, the existing breadcrumb logic is correct. So this is not a bug.
If we would change it, it might harm thousands of websites.

I mark this as "works as designed".

This being said:
- The behavior could be better documented / labeled.
- For Crumbs, it means we can not use the breadcrumb that was put in drupal_set_breadcrumb(). Instead, we need some creative head-scratching to find a suitable solution. This will become interesting.

Ideally, I want something like this:

$view = views_get_view(...);
$view->set_arguments(..);
$view->_build_arguments();
$last_arg = end($view->argument);
$title = $last_arg->get_title();

If this works, we will get very nice breadcrumbs, easily expanding to listed entities, such as
Home > News > (year) > (month) > (day) > (node title)
with only minimal configuration

I am only a bit worried about performance, if we load the view.

Ok, seems to work!
#1914718-35: Crumbs on Views with Arguments/Contextual Filters