Enable Localization of Link Title Attribute and Fix Ugly Breadcrumbs

If you don't like the "View" terminology (you suggested "Content" instead), I'd suggest either translating it yourself or using something like String Overrides. The other stuff in here is pretty good though...

What if we did the same thing you did for the users (title callback) for the nodes as well....

I see three things we're attempting to fix here:

Input Formats

In admin/settings/filters/1/configure and admin/settings/filters/1/order, you see "Home › Administer › Site configuration › Input formats ›" when it would be assumed you'd see "Home › Administer › Site configuration › Input formats › %FilterName%".

User Accounts

When editing other people's accounts (or tracking other people's accounts), you see "Home › My account". Seeing how you're not editing your own account, it would be wise to display "Home › %Username%".

Node Editing

When editing a node (or visiting other tabs like Dev Render), for the breadcrumb, you see "Home › View". It would make more sense to display "Home › %NodeTitle%".

Also just found:

Menu Editing

If you edit one of the menus at admin/build/menu and click on "Add item" or "Edit menu", you get "Home › Administer › Site building › %MenuName%", when you'd expect "Home › Administer › Site building › Menus › %MenuName%".

Think we should break these up into separate issues?

Found another:

Edit Previewing Nodes

When you edit a node and then hit preview, you get "Home › Create content › Submit %NodeType%", when you'd expect "Home › %NodeTitle%".

At first I got a bit concerned about security but then this is how title gets set in drupal_get_title

    $title = check_plain(menu_get_active_title());

that's safe. As for the changes, they are fine.

Some noted:

Menus:
- right the first line is a bad whitespace change
- why didn't we need the elseif($callback) check before?
- you removed(?) the special case for t() title callbacks which was supposed to be a performance improvement originally

Filter formats:
- this looks good

Nodes:
- we use verbs for tab titles where possible, so "Content" as a tab title does not look fitting into the concept

Users:
- that %user_current looked more and more suspicious through different patches... so why is it called the %user_current, if it matches any users?

Nice work, everything seems to display properly. This attached patch also fixes the Node Previews that I mentioned earlier.

Can we get a review and RTBC in here? /me runs away ...

And perhaps someone like pwolanin can comment on related issue: http://drupal.org/node/170309

Here are some steps that one can take to review this patch:

1. Fresh Drupal install
2. Generate some users and visit user/2/edit and note the ugly breadcrumb
3. Create a node and edit it, note the ugly breadcrumb
4. Create a node and preview it, note the ugly breadcrumb
5. Apply the patch and repeat the previous 3 steps, noting the pretty breadcrumbs
6. Mark patch RTBC

Breadcrumbs before patch:

• on user/2/edit: Home › My account
• on node/1/edit: Home › View
• on node/add/story (Preview): Home › Create content › Submit Story

> Applied patch
> Rebuilt menus

Breadcrumbs after patch:

• on user/2/edit: Home › Foo (for user "Foo")
• on node/1/edit: Home › Bar (for node "Bar")
• on node/add/story (Preview): Home › Create content

I set this to RTBC, as the patch apparently does exactly what is was supposed to do. Good job!

OK, I looked over #20 and do not understand how/why the call to _menu_item_localize() gets removed, and replaced with some code which does less. It does not ensure that the menu item descriptions are localized for example. Can someone enlighten me? Also, nobody explained why didn't we need the elseif($callback) check before, although I asked this above.

pwolanin: descriptions are written in English in code, and stored in English in the database. Look into what _menu_item_localize() does. It does localize the description too, so without using it, we basically loose menu item description localization at least.

Hm, so it seems I unknowingly touched on another bug :) Anyway, all I was saying is that _menu_item_localize() does more then what you do here. One thing it does it it t()-es the item description. Whether the menu item description is used or not depends entirely on the theme, right? That Drupal core itself does not display it does not mean a theme would not.

Did some basic testing and all seems OK.

Applied the patch and confirmed that it fixes the missing title/descriptions issue.

To me, this isn't 100% either. In case of editing a node, I'd expect the page is always the title of the node and not 'Edit $title'. We don't use 'Edit $username' either. I've made that change and committed the patch. Thanks all.

The commit ( http://drupal.org/cvs?commit=92964 ) was incomplete -- only the node.module and node.pages.inc chunks were committed.

I believe we should make a new patch reflecting the part(s) that were missing.

Committed, thanks.