Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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/1
anduser/1/view
share 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/1
anduser/1/view
don’t have the same page titles: Whileuser/1
shows the user name as page title,user/1/view
has 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.
Comment | File | Size | Author |
---|---|---|---|
#14 | local_tasks_path_3.patch | 4.15 KB | pwolanin |
#12 | local_tasks_path.patch | 3.97 KB | chx |
#11 | local_tasks_path_2.patch | 3.6 KB | pwolanin |
#10 | tabtest.zip_.doc | 2.35 KB | pwolanin |
#9 | local_tasks_path_1.patch | 2.66 KB | pwolanin |
Comments
Comment #1
kkaefer CreditAttribution: kkaefer commentedOk, 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/types
vs.admin/content/types/list
is a good example.Comment #2
chx CreditAttribution: chx commentedThis patch restores all behaviour mostly -- if you have an object for the default tab then it points to itself otherwise it's the parent.
Comment #3
kkaefer CreditAttribution: kkaefer commentedThere are some glitches:
When you’re on a non-default tab:
admin/content/types/list
instead ofadmin/content/types
)admin/content/types
instead ofadmin/content/types/add
)Comment #4
ChrisKennedy CreditAttribution: ChrisKennedy commentedHere 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).
Comment #5
Dries CreditAttribution: Dries commentedCan we add some code comments please? Thanks! :)
Comment #6
pwolanin CreditAttribution: pwolanin commentedI thought I fixed the problem with the Help module in an earlier patch?
(from menu.inc - look at the last line)
Comment #7
ChrisKennedy CreditAttribution: ChrisKennedy commented@Peter - this patch fixes the default tab link, not the help.
Comment #8
pwolanin CreditAttribution: pwolanin commentedI 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.
Comment #9
pwolanin CreditAttribution: pwolanin commentedthis isn't maximally efficient, but seems to work. I'll post some additional code to use for testing as well.
Comment #10
pwolanin CreditAttribution: pwolanin commentedok, 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)
Comment #11
pwolanin CreditAttribution: pwolanin commentedwith additional/corrected code comments.
Comment #12
chx CreditAttribution: chx commentedSteven 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.
Comment #13
pwolanin CreditAttribution: pwolanin commentedSince this patch stores the full item in the $items array, it seems the $tab_parent array is unused and can be eliminated.
Comment #14
pwolanin CreditAttribution: pwolanin commentedusing chx's logic to find the right href for local tasks, but rewrote the code and re-tested.
Comment #15
flk CreditAttribution: flk commentedyeap this patch works fine ;)
tested it on both blocks and the little tab module you gave...works great.
Comment #16
Gábor HojtsyAll 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.
Comment #17
Gábor HojtsyDiscussed 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.
Comment #18
(not verified) CreditAttribution: commented