Menu system: Default tab behavior
kkaefer - February 4, 2007 - 14:50
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | menu system |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
With the new menu system, tabs changed radically in how they work behind the façade. However, I noticed that the default tab doesn’t point to it’s parent.
Let’s take an example: on user/1, the (active) ‘view’ tab points to user/1/view rather than to user/1. The same happens all around Drupal, for example in admin/content/types vs. admin/content/types/list. This leads to several problems:
- Duplicate content:
user/1anduser/1/viewshare the exact same content, even though they have different URLs. - Help module breaks: If you specify a help text for
admin/content/types(there is in fact one), it won’t show up onadmin/content/types/list, even though it’s the same content. user/1anduser/1/viewdon’t have the same page titles: Whileuser/1shows the user name as page title,user/1/viewhas the title ‘View’. That means, page titles do now change from tab to tab rather than staying the same on one tab level. This is not the intended behavior: Tabs are usually displayed below the page title and are therefore “sub pages” of that title.
#1
Ok, this doesn’t happen for
user/1, because the page title is set somewhere else and not taken from the menu system. However,admin/content/typesvs.admin/content/types/listis a good example.#2
This patch restores all behaviour mostly -- if you have an object for the default tab then it points to itself otherwise it's the parent.
#3
There are some glitches:
When you’re on a non-default tab:
admin/content/types/listinstead ofadmin/content/types)admin/content/typesinstead ofadmin/content/types/add)#4
Here is a patch from chx via IRC - I tested it and it correctly fixes default tab behavior. I had noticed some problems that popped up within statistics.module (http://drupal.org/node/143081).
#5
Can we add some code comments please? Thanks! :)
#6
I thought I fixed the problem with the Help module in an earlier patch?
(from menu.inc - look at the last line)
function menu_get_active_help() {
$output = '';
$item = menu_get_item();
if (!$item || !$item['access']) {
// Don't return help text for areas the user cannot access.
return;
}
$path = ($item['type'] == MENU_DEFAULT_LOCAL_TASK) ? $item['tab_parent'] : $item['path'];
#7
@Peter - this patch fixes the default tab link, not the help.
#8
I need to work through this more, but I don't think this code is going to work at depths below the first level of local tasks.
#9
this isn't maximally efficient, but seems to work. I'll post some additional code to use for testing as well.
#10
ok, trying to get around the upload restrictions ;-)
rename the attached file to tabtest.zip and unzip it. Inside is a small test module and a replacement for the page.tpl.php of garland. This version just shows tabs at depths below 0 and 1.
Substitute the page.tpl.php and enable the module. Visit /tabtest and /tabtest/6 (or any arg)
#11
with additional/corrected code comments.
#12
Steven reviewed the patch and complained that default tasks within default tasks does not point high enough. That's solved by a nice empty for loop which climbs up until it finds a non default local task. Also, he mentioned that we should not show single tabs. Fixed, too.
#13
Since this patch stores the full item in the $items array, it seems the $tab_parent array is unused and can be eliminated.
+ $tab_parent[$item['path']] = $item['tab_parent'];#14
using chx's logic to find the right href for local tasks, but rewrote the code and re-tested.
#15
yeap this patch works fine ;)
tested it on both blocks and the little tab module you gave...works great.
#16
All seems to be fine here. How does this affect the help use case and pwolanin's workaround in particular? It seems this patch should remove that.
#17
Discussed the issue with chx on IRC, and the help issue is different, will be addressed in another patch.
Done another review and it looks good, so committed.
#18