No headline shown when i send a message with user ID

No idea how to fix the menu inheritance, but the following code may help make this less noticeable.

in pm_new, replace

  if (isset($user)) {
    $recipient  = $user->name .', ';
  }

with

  if (isset($user)) {
    $recipient  = $user->name .', ';
    drupal_set_title(t('Send a message to !name:', array('!name' => $user->name)));
  }

Good idea?

An actual description of the problem:

1. Go to "messages/new". You'll get picture 1.
2. Go to "messages/new/{user_id}". You'll get picture 2.

The problem is that all pages at "messages/%" inherit the same local tasks, but pages at messages/%/% don't inherit them.

This is seen on the above mentioned messages/new/% and also when viewing the messages at messages/view/% - in both cases the local tasks from the menu level above do not show.

Yes, because these are defined as MENU_CALLBACK and not as MENU_LOCAL_TASK.

Atleast for the new message, there is an easy fix...

Instead of defining two different menu items, we could just have one and check the user_id instead of letting the menu system do this for us.

However, this does not work for messages/view/%

This also does not add the local menu for the view messages pages, but apart from the the above patch is rtbc (I added a slight change to it here).

Yes, the view messages is a slightly different issue and harder to fix.

In difference to write message, there is no "default" view message page that *is* listed as an local tab.

And there seems to be no easy way to force the display of the local tabs of page X. I tried trough every menu_set_x function i could find, none did what wanted (in fact, they didn't do anything, except of breaking the breadcrumb ;) ) and asked twice in IRC..

i'm not sure but it should be able to change the tabroot in the menu router-item could solve, perhaps the problem

<?php
menu_set_item('messages/view/%',
array (
  'path' => 'messages/view/%',
  'load_functions' => 
  array (
    2 => NULL,
  ),
  'to_arg_functions' => '',
  'access_callback' => 'user_access',
  'access_arguments' => 'a:1:{i:0;s:15:"read privatemsg";}',
  'page_callback' => 'privatemsg_view',
  'page_arguments' => 
  array (
    0 => '%',
  ),
  'fit' => '6',
  'number_parts' => '3',
  'tab_parent' => '',
  'tab_root' => 'messages/view',
  'title' => 'Read private message',
  'title_callback' => 't',
  'title_arguments' => '',
  'type' => '4',
  'block_callback' => '',
  'description' => '',
  'position' => '',
  'weight' => '-10',
  'file' => '',
  'href' => 'messages/view/%',
  'options' => 
  array (
  ),
  'access' => true,
  'localized_options' => 
  array (
  ),
  'map' => 
  array (
    0 => 'messages',
    1 => 'view',
    2 => '%',
  ),
));?>

But it seems to be ignored in hook_menu, looked in the talbe menu_router

hey i got something working

i changed the type of messages/view/% to menu_local_task
so this adds the feature itself, that it shows the local tasks on viewing a privatemsg

The only thing to do is, to hide the tab on all other pages
Therefore i created a extra access callback which proves this

The patch is from head

Added some screenshots to show that it works.

Is this a clean way?

its a patch so....

Seems to be a nice workaround. Maybe it would be better if we change the weight of the read-tab to a different value so the tab is at the end of the list (maybe before tags)

Small changes to above patch, otherwise looks good. Marking as rtbc.

ah ;). i knew one patch will slip up.

- $items['messages/inbox'] = array(
+ $items['messages/list'] = array(

you are removing inbox in favor of list. but then every where else in code messages/inbox is left neglected.

observe:
line 270: case 'inbox':
line 829: url('messages/inbox')

overall i dont agree with the apprach. lets not be lazy, add a bunch of extra menu items that act specifically as a localtask.

I've double checked with menu masters. they said we need to create different menu_local_task for each menu depth (or generally whenever we need them)

oops at leaving inbox in there.

The reason for removing the menus is to fix the observation in the original post - to fix the "local tasks" and to leave/add the menus at the top of the pages.

Is there another way to approach this?

removed

- $items['messages/inbox'] = array(
+ $items['messages/list'] = array(

from mthe patch - I will pout that in with #303087: Move "Sent Messages" to privatemsg_filter, Add inbox, improve user filtering. to stop these two patches blocking on each other.

This patch is standalone and AFAIK is not needed for any others, nor does it depend on others.

@litwol

I'm not sure if I understood your comment about being lazy correctly. The problem with the current way is that it is not intuitive to go back to the thread list when we are viewing a thread or writing a new message to a specific user. The target of this issue/patch is to provide the same (or atleast, similiar) navigation/tasks for every privatemsg-related pages (except admin). Or, more explicitly, to get a mix of the menu types CALLBACK and LOCAL_TASK.

The send message page is imho quite unproblematic, because the user id is an optional parameter for the same page.

I am also not 100% happy with the proposed solution for the view message page, but I can't find a better solution and it is imho better than the solution that we currently have. There seems to be no easier way to embed CALLBACK-items into an existing page with local taks.

I found out something really interesting by looking at how Views has done this for the "Edit" local Tab.

There is a simple, not so obvious trick. When parameters are defined with a name, for example %node, the menu system does call the respective name_load functions and when these functions return FALSE, the tab is not displayed.

So, I changed messages/view/% to messages/view/%privatemsg_thread and added a (currently, very smple) privatemsg_thread_load function. All this function does at the moment is checking if the parameter is an int value bigger than 0. At a later point, this can be extended. I haven't done this yet, because I want to synchronise it with #288183: Provide api function for other modules to send messages.. If this patch gets commited, I will extend the load function there.

While this solution is a bit harder to understand than the access callback, it seems to be the "Drupal way" to do it, and does add something useful (atleast later).

Committed !!! :)

Thanks.