Hi all,

I've just upgraded context from 7.x-3.1 to 7.x-3.3 and now all my "active" menu items are broken.

No css class as "active" or "active-trail" are printed in my menu items.

Context reaction "Menu" is still setted in my context settings but without effects.

What happens in latest version about this topic ?

Than you very much for resolving this.

Comments

fantasma’s picture

Same here!
Upgraded from 7.3.2 to 7.3.3

keynone’s picture

Same. It's not working at all anymore.

Seems to be especially broken on multilingual drupal installs.

  • colan committed f3e501b on 7.x-3.x
    Revert "Issue #835090 by Karsa, Deciphered, nedjo, fearlsgroove, Dane...
colan’s picture

Status: Active » Closed (duplicate)

The commit from #835090: Context Reaction: Set menu trail has been rolled back so this should no longer be an issue.

hass’s picture

Category: Bug report » Support request
Status: Closed (duplicate) » Fixed

Simply press save and you are done. The config cannot upgraded automatically. This looks like a design limitation. This problem occur only because we fixed serious bugs in #835090: Context Reaction: Set menu trail where the previous code saved a broken settings array.

colan’s picture

Status: Fixed » Active

Looks like this is back on the table now that the other patch is back in.

colan’s picture

Category: Support request » Bug report
hass’s picture

Do you have any concept of upgrading setting between versions if the structure of the settings array change? I thought i had a case opened for this, but cannot find it.

mxt’s picture

Simply press save and you are done.

Obviously I've done this. I've resaved all contexts and the main context settings page too.

But this does not resolve the issue.

I think that Keynone in #2:

Seems to be especially broken on multilingual drupal installs.

may be right: my site is multilingual (with entity_translation)

Thank you very much for working on this.

hass’s picture

I use the feature and it works very well and it was totally broken before 3.3. Please investigate the root cause of you issue so we can followup and may fix the root cause. It was for sure required to reconfigure the context after the upgrade, but now active trail really works.

irowboat’s picture

Seems that entity_translation and context -> menu do not work together. I can't see it being a site configuration problem, since there are no 'creative' solutions on my current site that exhibits the error.

Tried 7.x-3.6 and current DEV. Also reverted briefly back to 7.x-3.2 and the menu activation does work on that (after updating the settings) but that broke other things.

freeform.steph’s picture

This was working great for us with context version 7.x-3.2, however after an update to 7.x-3.6 last night, it is no longer producing the desired action. Have not been able to get this to work since the update.

We have internationalization enabled, but are not using entity_translation.

hass’s picture

Are you able to debug the issue, please? We have no idea how to repro as it started working very first with 3.3 with normal menus and was completly broken before.

Matt Habermehl’s picture

Can confirm this is an issue with 7.x-3.6 on non-multilingual site. Using "debug" reaction I have confirmed that the context is active, but that the "active" CSS class is not being added to the selected menu item.

Could this be the problem?
https://www.drupal.org/node/1683944

Matt Habermehl’s picture

I believe I have part of the problem figured out. It appears that in plugins/context_reaction_menu.inc the function $this->get_contexts() is not available. The actual get_contexts() function in context_reaction.inc is returning properly.
Why would the get_contexts() function not be available when it works as expected in context_reaction.inc and "class context_reaction_menu extends context_reaction"?

tuwebo’s picture

StatusFileSize
new1.93 KB

Hello,
I have some problems with localized menus and I've been debuggin a little.
I've found two things that might cause problems:

  1. context_reaction_menu.inc - options_form(): $options array for the (multi) select list is being overriten when we have same $identifier (link_path). This could happen for menus, where you have more than one link pointing to the same system route.
    This is happening because it's not being checked the menu name as part as the identifier as mentioned in #2407665: Configuration form for menu and breadcrumb reaction doesn't list duplicate options correctly.
    <?php
    while (isset($options[$root_menu][$identifier])) {
    ?>
    

    should be

    <?php
    while (isset($options[$root_menu][$menu_name . ':' .$identifier])) {
    ?>
    
  2. context.module - context_preprocess_menu_link(): Since it seems that we have a multiselect list, we have an array of identifiers to check against to and we also have stored identifiers as [menu_name:identifier], so we also need to strip out the "menu_name:" part:
    <?php
    if ($variables['element']['#href'] == $context->reactions['menu']) {
    ?>
    

    should be something like

    <?php
    // Get identifiers as link paths and then...
    if (in_array($variables['element']['#href'], $identifiers))
    ?>
    

I've attached a patch as a starting point, trying to solve this two issues, and nothing more. Do not use this patch in production even if it works for you (post here your review instead).

After applying the patch you want to double check all your menu reactions in your contexts, save them again (I think this will avoid warnings for stored values as a strings, now we use an array for the multi select) and clear the drupal cache.

tuwebo’s picture

Version: 7.x-3.3 » 7.x-3.x-dev
Status: Active » Needs review

Changed the status and version.

cfunken’s picture

Having the same issue, also on a multilanguage site.
Applying the Patch sadly didn't change anything that I could observe.

I tried it in two ways with a DB and code rollback in between.
First removing all menu reactions from contexts, then applying the patch, then adding the reactions back in.
Second time I left the reactions in there and just applied the patch as is.

Both had the same result in that the sites behavior didn't change. The menu still does not get any css classes.
Can't really add anything more than that, but I thought I'd at least report my results with the patch.
Was anyone else able to solve the problem with this patch so far?

tuwebo’s picture

@cfunken thanks for the review.

It looks like you did it as I did, but different results for me, maybe menu configurations, and/or links in those menus... I don't know.

Doing this test works for you?

1. Create a new context with xxx condition and one menu reaction, selecting two items from same menu that are pointing to different routes (i.e. node/10 and node/20). It should be better same language elements (just for this test)

2. If you have access to the program code and devel module enabled or you can debug, take a look and see if the context worked and no other modules are processing same thing after us:

<?php
function context_preprocess_menu_link(&$variables) {
       // .....
       if (in_array($variables['element']['#href'], $identifiers)) {
           $variables['element']['#localized_options']['attributes']['class'][] = "active";
           // Test - this items bellow should have active class
           dpm($variables['element']['#href'], 'Active')
         }
?>

Those items should have "active" class in your menu, if your menu is visible, as far as I know. The code is pretty odd but it should work as a proof that elements where getting that class once.

Also notice that the active trail item could not be the expected. Since we can select more than one item we should be able to select a preffered menu, just in case we have selected links that are in different menus. But I think this could be a feature request if there is none.

cfunken’s picture

Hello.

It seems that the context_preprocess_menu_link isn't firing in the first place. As nothing I put in there for debugging purposes showed up anywhere (even a die(); was completely ignored).
So I suspect the problem I'm having is less about this specific issue with the Context module (thought it might also be that) and more about the messy installation I have to maintain right now. If the problems remains after I get the preprocess_menu_link hook sorted out I'll try that debugging and let you know the results.
Thanks for the effort anyway.

cfunken’s picture

@TuWebO
Took some tinkering, but the problem seems to be fixed now. I tried to make sure whether or not the patch solved the issue or my cleaning up the messy drupal install did the trick, but without the patch even the cleaned up version fails. So I take it the patch did its work and the cleanup merely allowed it to do so. At leasts thats the best I can offer in terms of assuredness on whether or not the patch fixed the issue.
Thanks you for the patch btw,.. helped out a lot...

tuwebo’s picture

I'm glad to hear that @cfunken, and thanks again for the review.

Probably we will need to change this code (and make it lot better), let's see if someone with more experience with this module could help us here a little.

jessica.k’s picture

Ran into this issue trying to add some extra visual wayfinding in a new site's theme. The patch in #16 seemed to work perfectly until I went to edit a page and got a giant wall of error messages. It appears to be two errors looping repeatedly.

  • Warning: array_values() expects parameter 1 to be array, string given in context_preprocess_menu_link() (line 198 of /foo/sites/all/modules/context/context.module).
  • Warning: Invalid argument supplied for foreach() in context_preprocess_menu_link() (line 204 of /foo/sites/all/modules/context/context.module).

The errors then appeared regularly across the whole site after editing. This was after resaving menu reactions and clearing the cache as instructed. Definitely needs some more review, and unfortunately I'm not knowledgeable enough to contribute much more. I've rolled back to the bugged official release for now but I would love a proper fix in the near future. :)

tuwebo’s picture

Hi jessica.K thanks for the review.
You are right the patch won't work, it is just a starting point but it looks like there is a lot more work to do. I will try to get some time to take a look at it and post it here.

mxt’s picture

I'm now in this bad situation: I cannot upgrade Context for last security issue due to this bug.

Please resolve it!

Thank you very much

torotil’s picture

Status: Needs review » Needs work

Set to "needs work" as per #24.

adriaav’s picture

Same problem here with context 3.6 on a multilingual install and using tb mega menu (don't know if that really matters...)

Is there any solution to this issue?

Thank you!

flocondetoile’s picture

Hi,

Can't get the menu be fired because $context->reactions['menu'] is now an array. The values of this array contain too the menu machine name whereas the variable $variables['element']['#href'] contains only the path. So the menu item never match the menu reaction in the function context_preprocess_menu_link(). Applied the patch from #16 resolved this issue.

Thanks TuWebO. It seems that your strating point works (test with context 7.x-3.6). Maybe we should simply test if the $context->reactions['menu'] is an array to avoid notice mentionned in #23. I don't dig this point but may be in some case the variable $context->reactions['menu'] is not an array but a string.

$menu_reactions = array_values($context->reactions['menu']); 

could be

$menu_reactions = is_array($context->reactions['menu']) ? array_values($context->reactions['menu']) : array($context->reactions['menu']); 

to avoid this notice ?

rocket777’s picture

Just wondering where we are on this. I am also on 3.2 and cannot upgrade

floown’s picture

To set active menu with Context module does NOT work with the 3.6 version. Someone can update the bug issu?

The dev version correct this problem?

//EDIT : OMG, the dev version is the same that the last stable.

floown’s picture

Hello,

I have found a little CSS "hack".
I have notice that the menu herit only the class "active-trail" instead of "active active-trail". So I have add a class in my CSS :

#region-user-first .menu li a.active, #region-user-first .menu li a.active-trail {
  background: #64A7D1;
}

Hope it works for you too. Have a nice day.

mecano’s picture

Looks like it only uses menu_set_active_trail() and that leaves out 'active' class, would be great if Context would let us apply custom classes on the whole active path menu tree.

mecano’s picture

My bad it also uses menu_tree_set_path() but no luck with 'active' class there as well? https://api.drupal.org/comment/60193#comment-60193

  • colan committed f3e501b on 8.x-4.x
    Revert "Issue #835090 by Karsa, Deciphered, nedjo, fearlsgroove, Dane...

  • colan committed f3e501b on 8.x-0.x
    Revert "Issue #835090 by Karsa, Deciphered, nedjo, fearlsgroove, Dane...

  • colan committed f3e501b on 8.x-1.x
    Revert "Issue #835090 by Karsa, Deciphered, nedjo, fearlsgroove, Dane...
hass’s picture

Assigned: Unassigned »

Working on a patch based on #16 that is really the source of this bug. I fixed all the errors. Need to cleanup the code a bit and than I will post the patch.

Has someone an idea why the options field was changed from single item to multi item? It is not really useful with 3 lines only.

hass’s picture

Status: Needs work » Needs review
StatusFileSize
new4.11 KB

Somehow based on the #16 idea it helped me understanding the root cause. Refactored the patch from scratch.

- A path is now per menu (bug)
- If more than one menu path is configured, both are 'active' (bug)
- The options dialog is now properly sized like it is done in context_condition_menu.inc (usability bug/feature)

Please try this patch out.

I will ask for co-maintainer now as activity on this module is really low.

tuwebo’s picture

Hi hass,
Thank you very much for your work, I will try to test it (on friday) and post the result, it looks good!

hass’s picture

@TuWebO: have you been able to try it out?

  • hass committed 2737ccd on 7.x-3.x
    Issue #2374445 by hass, TuWebO: Reaction MENU to set active menu item...
hass’s picture

Status: Needs review » Fixed

Code is not yet in D8.

  • hass committed 60cb7d8 on 7.x-3.x
    Issue #2374445 by hass: Undefined index - menu in context.module line...

  • hass committed 5f3114e on 7.x-3.x
    Issue #2374445 by hass: Failed to set field reactions[plugins][menu] to...
hass’s picture

All tests are back to green.

Status: Fixed » Needs work

The last submitted patch, 38: Issue-2374445-by-hass-Reaction-MENU-to-set-active-me.patch, failed testing.

The last submitted patch, 38: Issue-2374445-by-hass-Reaction-MENU-to-set-active-me.patch, failed testing.

The last submitted patch, 38: Issue-2374445-by-hass-Reaction-MENU-to-set-active-me.patch, failed testing.

The last submitted patch, 38: Issue-2374445-by-hass-Reaction-MENU-to-set-active-me.patch, failed testing.

hass’s picture

Status: Needs work » Fixed
tuwebo’s picture

Hass,
Sorry, I have fridays for contributing, but last friday I was with another issues and this one I'm traveling.
I'll try as soon as possible because is really interesting this patch

hass’s picture

I released a new version v3.7 that fixes all these issues.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.