Incorrect breadcrumbs when using menu contexts with the admin menu module

Coming from #909148: Arbitrary menu selection in menu condition with an alternative approach: the attached patch introduces an 'ORDER BY mlid ASC' clause to the query to give priority to menus having the older items.

After installing Context and using a menu condition, not using breadcrumb (disabled or not in zen theme settings) I had the above as the h1 title (instead of "Users"), and plach's context-909148-1.patch resolved this. I haven't found any issues with it.

Perhaps we should merge the two patches: having an explicit choice as proposed in #2 seems to be the only way to get the needed results for every use case, while #5 provides a good way to have a consistent behavior when the result would be ambiguous.

I know this is still in progress, but wanted to report that while the patch from #5, context-909148-1.patch resolved the h1 on admin/user/user (as I reported in #6), the same problem appears for the h1 on admin/user/user/create.

The patch behaves well and fixes the issue. We should use the configured menus also when selecting the condition values, otherwise one could select a menu item that is not allowed by the current configuration. Moreover there were a couple of minor string issues. Here is a reroll.

Subscribe

Using Context 6.x-3.0, applied patch #10, it solved /user but as reported above admin/user/user/create has still the problem. There is also another problem, the context menu condition works when it is in the main language (fr in this case) but it does not work for the menus in English. Any ideas?

Drupal 6.19, Context 6.x-3.0, Admin Menu 6.x-1.6

Same behavior. Using a context with a menu condition, /user, /admin/user/user and /admin/user/user/create all had the title set to the admin_menu image markup.

Applied patch in #10, it failed to apply from the module root and from the plugins directory, but I was probably doing something wrong. Applied it manually and the titles are back to normal on those user pages.

Have not found any residual issues from the patch.

Thanks!

Applied patch from #10 manually against 3.0 and I still get the issue

Did you esclude "admin_menu" in the context general settings page?

Update to #13

I don't know if this is intended behavior or not for this condition, but before the patch, if a parent menu item was selected as the menu condition, all child menu items for that parent also triggered the condition. In my case this was desired behavior. If the client added child menu items to that parent the context persists. That way the context did not have to be edited to add that new child menu item after it was added to the menu tree.

After the patch this stopped happening. In order to get all child menu items to also trigger the context, we had to select the parent and all children in the condition.

It seems that there might be a typo in the patch.

Here is the patch addition to the execute function:

<?php
        $menus = variable_get('context_condition_menu_selections', menu_get_names());
        
        $params = array_values($menus);
        $params[] = $items['href'];

        $in_clause = implode(', ', array_fill(0, count($menus), "'%s'"));
        $query = "SELECT menu_name FROM {menu_links} WHERE menu_name IN ($in_clause) AND link_path = '%s' ORDER BY mlid ASC";

        if ($menu_name = db_result(db_query($query, $params))) {
?>

What I think was supposed to be there was:

        $menus = variable_get('context_condition_menu_selections', menu_get_names());
        
        $params = array_values($menus);
        $params[] = $item['href'];

        $in_clause = implode(', ', array_fill(0, count($menus), "'%s'"));
        $query = "SELECT menu_name FROM {menu_links} WHERE menu_name IN ($in_clause) AND link_path = '%s' ORDER BY mlid ASC";

        if ($menu_name = db_result(db_query($query, $params))) {

I changed the variable for $items['href'] to $item['href'].

I would attach a new patch, but I still don't have that figured out yet.

Can anyone else confirm?

Re #15 thank you, I excluded the menu and it works fine.

#949994: Title corrupted on login page marked as duplicate of this issue.

Rerolled patch from #10 as there is an error in the 3rd hunk so the patch from #10 can't be applied automatically.

patch #19 good over here!

Patch in #19 did the trick.

then

Patch #19 works on /user and most other pages having the issue but I still get it on admin/user/user/list

By simply applying the patch at #19, user and admin/user/user worked fine.

But admin/user/user/create and admin/user/user/list didn't work until I saved my Menus setting at admin/build/context/settings. Specifically, I had to exclude the "admin_menu", and then it worked.

Does that mean this patch needs more work? Should the default value for "context_condition_menu_selections" exclude "admin_menu"?

@joelstein it is working as desired imo, it is not automatical fix

Read comment #2: "The attached patch adds a new section to the Context UI Settings screen that allows the administrator to select which menu(s) to use when checking a menu context condition."

Do you think the same settings should be applied to the context_reaction_menu.inc when selecting the menu item or breadcrumb? I think if your going to limit your menus through the settings, it should reflect visually when setting up your context.

Subscribing

My 2 cents - I think that it should be left on the settings page and not part of setting up your context. Especially for the amount of times it's going to be used it hardly warrants adding to the main bulk of context, unless I'm missing something fundamental.

The patch in #19 works for me.

Attached is the same patch, but I altered the description on the settings form to reflect that if "none are selected, all of the menus are used" as this better describes the implemented behaviour.

Setting the status of this issue to "needs review".

I can confirm that #30 fixes the issue when you go to /user. If then you also deselect "admin menu" in the context settings it also fixes it for /admin/user/user/list and /admin/user/user/create. I think this needs to be documented somewhere, perhaps when you go to the context settings page it could say something like "admin menu has been detected, in most cases we recommend deselecting admin menu".

As for #26 perhaps when you add a menu reaction and you have admin menu installed it but not deselected it could show a message there too saying that this may interfere on the user page.

Patch no longer applies; re-rolled.

For other users wanting a clean workaround while this is fixed, I have put this into my theme_preprocess_page function, something similar may sort it for you:

    if ($GLOBALS['user']->uid == 0 && $_GET['q'] == 'user') {
      $variables['title'] = 'Login';
    }

Subscribing

Thanks, #33 works for me.

I wonder if #23 and #34 (and maybe #37) in this thread would be of help? http://drupal.org/node/279767

Seems that if you don't correctly doctor your menu query, you could end up with one of several entries for the same path. One needs to be careful that a) we have permission for the path and b) the path is created by a specific module (to ensure it's unique).

SELECT mlid FROM {menu_links} WHERE link_path = 'admin/user/user'; in my site gives me THREE mlid's.

SELECT mlid FROM {menu_links} WHERE module = 'system' AND link_path = 'admin/user/user'; gives me only one.

This method would work exclusively(? I think), if it were not for the fact(? I think) that even the original system module entry for that path could be modified, although it's pretty unlikely. Excuse the doubts, but that's what info I have anyway! :)

#33 working on latest dev as of today.

# cd .......modules/contrib
# drush dl context-6.x-3.x-dev
Project context (6.x-3.x-dev) downloaded to ...../sites/all/modules/contrib/context.  [success]
# cd context/
# wget http://drupal.org/files/issues/modules_context_plugins_context_condition_menu.inc_.901606.patch 
--2011-06-17 12:15:25--  http://drupal.org/files/issues/modules_context_plugins_context_condition_menu.inc_.901606.patch 
Resolving drupal.org... 
Connecting to drupal.org... connected.
HTTP request sent, awaiting response... 200 OK
Length: 2441 (2.4K) [text/plain]
Saving to: `modules_context_plugins_context_condition_menu.inc_.901606.patch'

100%[=================>] 2,441       --.-K/s   in 0s      

2011-06-17 12:15:26 (214 MB/s) - `modules_context_plugins_context_condition_menu.inc_.901606.patch' saved [2441/2441]
# patch -p0 < modules_context_plugins_context_condition_menu.inc_.901606.patch
patching file plugins/context_condition_menu.inc
#

#33 working on latest dev as of today for me too.

Thanks!
-Tim

then

I tried #33 with the current dev and it didn't fix the problem.

Patch at #33 on current dev did work after all.

Note that it is necessary to clear cache to see the result. I apologize for being either hasty or sloppy before. Thanks for the fix.

@bib you're going to have to leave more info than that if you want to help fix the issue - do you have any feedback?

#33 worked for me too on last dev. Thanks !

Is it just me or does this seem a very "hard work" approach to fixing the issue? Is the information in #37 of no use...? Seems like altering the query in the first place would fix this issue, or does that have a level of ambiguity that can only be circumvented by the technique in patch @ #33?

FYI patch works as advertised from #33. Gonna just leave it at that for now! :p

http://drupalcode.org/project/context.git/commit/3695e17