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.
Problem/Motivation
Admin menu was recently updated with the issue #2084217: Make field_ui_admin_menu_map() actually work for all entity types. The Field UI mapping is now included by default and does not have to be added in the Node registration hook_admin_menu_map()
.
Currently this results in a notice:
Proposed resolution
Remove the mapping of fields from hook_admin_menu_map()
Remaining tasks
- Write a patch
- Review
User interface changes
None
API changes
None
Comment | File | Size | Author |
---|---|---|---|
#16 | 2407359-remove-field-mapping-from-hook-admin-menu-map-2.patch | 1.59 KB | dpfitzsi |
#1 | node_registration-remove_field_mapping-2407127-1.patch | 1.1 KB | idebr |
Comments
Comment #1
idebr CreditAttribution: idebr commentedAttached patch removes the duplicate Field mapping from hook_admin_menu_map().
Comment #2
rudiedirkx CreditAttribution: rudiedirkx commentedWow, that is seriously bad from admin_menu. Obviously all kinds of modules will have implemented this hook. And now it'll be doubled, because suddenly they do it automatically!? But with another function name!?
What about all the nice people that don't have admin_menu-edge?
Comment #3
rudiedirkx CreditAttribution: rudiedirkx commentedIs there a way we can 'ask' admin_menu if they already provide the relevant info? Apparently it's not the same function name. I don't want to check version. Other ideas? If not, is there an info alter hook, so we can check if it exists and maybe add it there?
Comment #4
rudiedirkx CreditAttribution: rudiedirkx commentedLooks like there is an alter: http://cgit.drupalcode.org/admin_menu/tree/admin_menu.inc
Whoever makes a patch to still support old admin_menu users, gets a cookie.
Comment #5
idebr CreditAttribution: idebr commentedYes, admin_menu should have checked for duplicate values. The issue was included in the latest stable release 7.x-3.0-rc5. I would argue the amount of people running the latest Node registration but not admin_menu is not worth the hook. They will only miss a small feature. Either way, I guess I won't be getting that cookie :(
Comment #6
idebr CreditAttribution: idebr commentedFieldable Panels Panes solved this issue by checking the admin map before adding their own items. This seems like a decent workaround. #2146479: Conflict with Administration menu
Comment #8
rudiedirkx CreditAttribution: rudiedirkx commentedI agree. This hook for NR isn't even in a stable release, so nobody'll miss it. Just me =(
Comment #9
rudiedirkx CreditAttribution: rudiedirkx commentedNR dev + admin_menu rc5 doesn't produce a nice admin menu for me though... The generated mapping is quite different from the custom.
Ide, you have commit access =) Fix it.
Comment #10
dpfitzsi CreditAttribution: dpfitzsi commentedI've worked on a patch that appears to work (I tested on latest dev and moving back and forth between admin_menu-7.x-3.0-rc4 and rc5. I have added map_alter for older releases so they will still get the menu under structure.
Comment #11
dpfitzsi CreditAttribution: dpfitzsi commentedComment #12
rudiedirkx CreditAttribution: rudiedirkx commentedI think the patch failed... Lots of white space flaws and where does
$admin_path
come from? Also, why a hook map AND a hook map alter? Just the alter will do to add stuff if it doesn't exist yet. Or is that the funky patch too?Why doesn't admin_menu rc5 work with NR items?? Isn't that what they've added supposedly!?
Comment #13
dpfitzsi CreditAttribution: dpfitzsi commentedOk, I'll get those issues fixed and update the patch. I was following the example from davereid to use both, but the alter may do on its own.
Comment #14
dpfitzsi CreditAttribution: dpfitzsi commentedNew patch attached.
Comment #15
rudiedirkx CreditAttribution: rudiedirkx commentedWhere does
$path
come from? I get a bunch of these:No rush. I'm creating a new release without admin menu mapping.
Comment #16
dpfitzsi CreditAttribution: dpfitzsi commentedWoops, I had missed readding some the code I originally had under the map() in my first patch. The variable $path is now defined. I have retested on rc4 and rc5 and the menu renders on both versions without an error.
Comment #17
dpfitzsi CreditAttribution: dpfitzsi commentedComment #18
rudiedirkx CreditAttribution: rudiedirkx commentedI'm sorry, that's just not good enough. Admin Menu adds a map for
admin/structure/node_registration/manage/%node_registration_type/fields/%field_ui_menu
, which apparently doen't do anything, and then NR adds a map foradmin/structure/node_registration/manage/%node_registration_type
, which does half (it adds only 1 level, no fields/display).Plus, the code is weird. What's up with this:
Of course
$arguments
is not empty...Some version soon Admin Menu will 'fix' this weirdness, and NR has to do another update to facilitate. I won't. Not to add 1 level of admin menu.
You can keep working on it until you're satisfied, but I won't be spending much more time on it. I'm sorry you did and now I'm not doing anything with it. We'll blame @idebr for bringing it up =)