As reported over a year ago (https://www.drupal.org/node/2146481), Flag produces an error within the admin menu:

Warning: strtr() expects parameter 1 to be string, array given in admin_menu_tree() (line 27 of /var/www/mysite/sites/all/modules/contrib/admin_menu/admin_menu.inc).

With the latest version of admin menu, it is no longer necessary to add mapping for the individual fields so the whole if (module_exists('field_ui')) statement can now be removed from function flag_admin_menu_map() within the flag.module file.

Dave Reid has produced a change record that exemplifies how the changes were implemented in Fieldable Panel Panes (FPP) module (https://www.drupal.org/node/2397007). For reference, the error in FPP was also reported in https://www.drupal.org/node/2146479. In his example, an alter can implemented if we want to support older versions of admin menu as well.

Comments

cmonnow’s picture

Title: Update Flag module to support new hook_admin_menu_map() changes in 7.x-3.0-rc5 onwards » Update Flag module to support new hook_admin_menu_map() changes in Admin Menu 7.x-3.0-rc5 onwards
joachim’s picture

Assigned: Unassigned » joachim

Well, that issue you refer to wasn't reported against Flag module, and module maintainers can't be expected to keep up to date with changes in development versions of all the modules they interact with!

But yes, I'm aware of this and working on this at this very moment :)

joachim’s picture

Assigned: joachim » Unassigned
Status: Active » Needs work
StatusFileSize
new1.37 KB

Argh, Admin Menu doesn't like Flag being at the secondary tab level :(

Here's how far I got.

mcfilms’s picture

Any status update on this? Did it get rolled into the 7.x-3.x-dev version of December 22, 2014?

ed523’s picture

So this patch doesn't fix it?

joachim’s picture

Could you both please read up on the meaning of the issue status field, so you don't need to ask these questions? There is a link to documentation right here below the form element.

mcfilms’s picture

ed523, I have reopened the issue on the admin menu issue cue: https://www.drupal.org/node/2371389. Maybe there will be some help there.

joachim’s picture

Status: Needs work » Needs review
StatusFileSize
new1.37 KB

I've had another look at the work I had in progress, and it seems to do the trick. Not sure why I was having so much trouble with this when I last looked it.

Could someone try this out please?

danchadwick’s picture

Status: Needs review » Reviewed & tested by the community

#8 Works great.

I updated to admin_menu 7.x-3.0-rc5. The described PHP warning reappeared.
I updated to today's 7.x-3.x-dev. The described PHP warning continued.
I applied #8 and flushed the cache. The PHP warning disappeared. The Struture/Flags menu contains the expect sub-menus, and these menus work.
I reversed the patch and the warnings re-appeared.

Thank you joachim for your work on this issue and Flag in general. It is appreciated.

jgullstr’s picture

I confirm #8 works. Tested on a fresh Standard install. Notices are gone, menus expand correctly.

cmonnow’s picture

Just a note that this patch assumes you have updated to at least Admin Menu 7.x-3.0-rc5 (the latest recommended release). The fallback alter can be added though it will bloat the code. Should this requirement be noted somewhere?

danchadwick’s picture

I suggest just putting that in the release notes for Flag 7.x-3.6, when it is released. Which I hope is soon. :)

  • joachim committed a273aa2 on 7.x-3.x
    Issue #2397047 by joachim: Updated hook_admin_menu_map() implementation...
joachim’s picture

Status: Reviewed & tested by the community » Fixed

Hmph. D.org just ate my reply, which was long and I can't be bothered to type it all over again :(

Briefly:

- new release just rolled!
- thanks everyone who tested the patch so promptly!
- I don't think requiring people to update Admin Menu is a big deal, as the latest release fixes a critical bug anyway, and it's not like Admin Menu engenders other dependencies.

Status: Fixed » Closed (fixed)

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

ed523’s picture

Thanks