Fields are editable regardless of whether an bundle instance exists, missing menu titles, etc.

just subscribe for today :-(

See #614030-24: Cannot edit fields for taxonomy terms for an explanation on why $bundle arg in menu loaders are not usable :-(.

+++ modules/field_ui/field_ui.module	15 Mar 2010 00:34:01 -0000
@@ -86,33 +86,45 @@ function field_ui_menu() {
           $items["$path/fields/%field_ui_menu"] = array(
+            'load arguments' => array($entity_type, $bundle_name),

This is the crux of the issue :
You cannot use $bundle_name, because the same menu path will possibly be redefined for several bundles, thus only the $bundle_name for the last iteration of the 'foreach bundles' loop will be considered. You need to use $bundle_arg (the numeric position of the %bundle placeholder in the base $path).

Problem : in menu 'loader' callbacks, placeholders come as raw elements from the URL, not as 'elements as translated by the previous loader callbacks that have run for the path'. See #614030-3: Cannot edit fields for taxonomy terms
In short : field_ui_menu_load() won't receive a 'bundle name' it can understand, only the string that is in the path.

Powered by Dreditor.

Put a dsm($bundle_name) at the top of your field_ui_menu_load() func.
Then visit admin/structure/types/manage/article/fields/body (*article*)
You'll see that the func never receives 'article', but rather 'page'.

Node 'article' and 'page' bundles both set 'admin/structure/types/manage/%node_type' as their $entity_info['bundles'][$bundle_name]['admin']['path'].

Side note : Field API allows each bundle to specify its own admin path, even though in most cases it will be the same path (with a placeholder) for all bundles in a given entity type, because IIRC, when this code was written, node-type admin paths were precisely not using a placeholder for the node type, but separate menu items with hardcoded strings.

So the foreach ($info['bundles'] as $bundle_name => $bundle_info) { loop in field_ui_menu() in fact redefines the *same* $items["admin/structure/types/manage/%node_type/fields/%field_ui_menu/..."] menu items several times for node bundles. So 'load arguments' => array($entity_type, $bundle_name), in your patch just keeps the $bundle_name of the last loop iteration.

The 'load argument' needs to be the index of the placeholder ($bundle_arg), so that the menu loader receives the actual string in the requested path.

But :
field_ui_menu_load() will then receive $bundle_name as "the string present in the path", not as "the $node_type object loaded by the node_type_load() menu loader that has previously run for the %node_type placeholder" : menu loaders don't learn from each other - sad. Field API doesn't know how to turn this string into a 'bundle name' it understands. Meaning : field_ui_menu_loader() currently cannot deal with bundles and instances.
After replacing $bundle_arg with $bundle_arg in your patch, visiting the edit page of a field on a taxonomy term brings a 404, because field_ui_menu_load() returns FALSE.

Bottomline is : having a module handle menu items built by appending to paths handled by a different module is a *pain*.

It's all a question of what constraints we deem reasonable to put on fieldable entities :
- your admin paths need to be the same menu item for all of your bundles, using a placeholder
- the actual menu path needs to contain the exact bundle name understood by Field API - problem : we already know this won't be the case for comments.

Yes, sorry, my time in front of the keyboard is really limited currently :-/.

Hard to follow, but #7 should work indeed. But then :

$bundle_object = $map[$bundle_pos];

should be

$bundle_object = $map[$bundle_pos];
$bundle_name = field_extract_bundle($entity_type, $bundle_object);

[edited the snippet in my previous comment]

W00t ! So much love :-)

Great comments. Will try to actually test later today (er, tonight).

+++ modules/field_ui/field_ui.module	18 Mar 2010 11:50:34 -0000
@@ -86,33 +100,45 @@ function field_ui_menu() {
+            'weight' => 10,

Why ? Is this for admin_menu ? Is this the only item that needs an explicit weight ?

147 critical left. Go review some!

re #14: Those local tasks below $path/fields/%field_ui_menu (/edit, /field-settings, /widget-type, /delete) are in fact not displayed as 'local tabs'. They use MENU_LOCAL_TASK because it looks like the only way that those pages (e.g admin/structure/types/manage/article/fields/body/widget-type) keep the breadcrumb and primary tabs as 'context' - cf screenshots in #614030-12: Cannot edit fields for taxonomy terms to see what I mean.
I don't even understand how or why this use of MENU_LOCAL_TASK does the trick (I think it was quicksketch who wrote it that way when he worked on #516138: Move CCK HEAD in core), but I do know that any other way I tried fails...

So, as far as the current core Field UI is concerned, weights don't matter on those - could be different with admin_menu, though.

As for the patch :
- if $path/fields/%field_ui_menu uses a 'title callback', we can remove the manual drupal_set_title() in the page callback (field_ui_field_settings_form())
- why don't the other paths use a 'title callback' ? (the corresponding page callbacks also have a manual drupal_set_title() that can be removed if moving to a callback)

Other than that, RTBC for me.

Additional thoughts :
- Since you proved that field_ui_menu_load() can in fact receive and understand $bundles, then IMO it makes more sense to have it return an $instance rather than a $field. Those menu paths identify an (entity type + bundle + field name) triple, meaning : an instance. Plus getting a $field if you have an $instance is easy, but not the other way around.
- Then we don't need the $entity_type and $bundle_arg in 'title arguments' and 'page arguments', only $field_position (renamed $instance_position, I guess). The corresponding callbacks just receive an $instance instead of ($entity_type, $bundle, $field)

Yes, I changed from returning an $instance to returning a $field in #614030: Cannot edit fields for taxonomy terms just because I couldn't find a way to have the menu loader deal with bundles. You just made the loader smarter, so we can go back to the $instance.

Hence, all callbacks that specify $bundle_arg as callback argument will receive $instance in that place

Or rather : callback just need to specify $index_of_the_field_placeholder argument, and will receive an $instance, that contains all the other info (entitty type, bundle, field name). They don't need $bundle_arg and $entity_type as arguments.

Localisation : no, labels are not translated. Grey area, there's an issue about this somewhere :-(.

!empty($instance['label']) ? t($instance['label']) : $instance['field_name'];
Actually, I don't think I see a case where $instance['label'] would be empty. That's a remnant from a previous state of the code, possibly D6, dunno for sure. I'd say we can ditch that fallback and fix later if it breaks.

As per #16-#18, updated patch builds on #14
- reverts the menu loader to return an $instance,
- adjusts 'page arguments' and 'title arguments', as well as the corresponding callbacks, accordingly.

Rerolled after #707724: Call them entities instead of objects ('object_type' -> 'entity_type')

Committed to CVS HEAD. Thanks.