Dynamic (i.e. not cached) alter hook to instance definitions

A generic alter hook on $instances is a tricky pattern : how do you do an alter hook on something that's stored in the db without having the alters saved on the next field_update_instance() (either programmatically or through the UI) ?
To my knowledge, this pattern doesn't exist anywhere in core. Alter hooks are for stuff defined in code (info hooks, menus, etc...), not in the db. I don't think we'll go there at this point in time.

This being said, #553298: Redesign the 'Manage Display' screen introduced hook_field_display_alter() on the 'display' side : alter display settings just in time before a field is rendered.

For the sake of symmetry, I guess a similar hook_field_widget_alter() or something could be added. Patch welcome :-).

Thks for being so quick on your feet, Amitaibu. However, I have big concerns with the approach :-/.

+++ modules/field/field.api.php
@@ -1770,6 +1770,78 @@ function hook_field_extra_fields_display_alter(&$displays, $context) {
+ * Note that instead of hook_field_widget_alter(), which is called for all
+ * fields on all entity types, hook_field_display_ENTITY_TYPE_alter() may be
+ * used to alter display settings for fields on a specific entity type only.

Incomplete edit after the copy / paste ;-)

+++ modules/field/field.api.php
@@ -1770,6 +1770,78 @@ function hook_field_extra_fields_display_alter(&$displays, $context) {
+ * This hook is called once per field per displayed widget entity. If the result

a 'widget entity' is not a known animal

+++ modules/field/field.form.inc
@@ -25,6 +25,13 @@ function field_default_form($entity_type, $entity, $field, $instance, $langcode,
+  // Let modules alter the widget instance.

'widget instance' doesn't exist either :-). 'Widget properties' ?

+++ modules/field/field.api.php
@@ -1770,6 +1770,78 @@ function hook_field_extra_fields_display_alter(&$displays, $context) {
+ * @param $context
+ *   An associative array containing:
+ *   - entity_type: The entity type; e.g. 'node' or 'user'.
+ *   - entity_type: The entity object.

2 x 'entity_type'

+++ modules/field/field.api.php
@@ -1770,6 +1770,78 @@ function hook_field_extra_fields_display_alter(&$displays, $context) {
+function hook_field_widget_alter(&$instance, &$field, $context) {
...
+    $field['cardinality'] = FIELD_CARDINALITY_UNLIMITED;

+++ modules/field/field.info.inc
@@ -378,16 +378,20 @@ function _field_info_prepare_instance_widget($field, $widget) {
- *   The field instance array.
+ *   The field instance array. If the instance has the 'behaviors' key
+ *   populated, then that key will be used to determine the behavior of the
+ *   widget. This allows implementing modules to change an instance on the fly.
  *
  * @return
  *   One of these values:
  *   - FIELD_BEHAVIOR_NONE: Do nothing for this operation.
  *   - FIELD_BEHAVIOR_CUSTOM: Use the widget's callback function.
  *   - FIELD_BEHAVIOR_DEFAULT: Use field.module default behavior.
+ *
+ * @see hook_field_widget_alter()
  */
 function field_behaviors_widget($op, $instance) {
-  $info = field_info_widget_types($instance['widget']['type']);
+  $info = empty($instance['behaviors']) ? field_info_widget_types($instance['widget']['type']) : $instance;
   return isset($info['behaviors'][$op]) ? $info['behaviors'][$op] : FIELD_BEHAVIOR_DEFAULT;
 }

Er ? $instance['behaviors'] to override the ones from the $instance['widget']['type'] widget type ? Ugh, that looks... odd (euphemism).

More generally - I really don't support the idea to let this hook alter $instance, not to mention $field.
forcing $field['cardinality'] = UNLIMITED sounds like a very strange and hackish idea.

My suggestion for a hook_field_widget_alter(), similar to hook_field_display_alter(), was only to alter the $instance['widget'] part of $instance JIT before it gets used to build a widget $element, not more.

74 critical left. Go review some!

I'd say you need 3 widgets :
- select ('multiple' => FIELD_BEHAVIOR_CUSTOM)
- autocomplete ('multiple' => FIELD_BEHAVIOR_DEFAULT)
- smart ('multiple' => whatever, FIELD_BEHAVIOR_DEFAULT, but won't be used actually)

function mymodule_field_widget_alter(&$widget, $context) {
  $field = $context['field'];
  $instance = $context['instance'];
  $limit = 10: // could even be a widget setting.

  if ($field['type'] == 'my_field_type' && $widget['type'] == 'smart') {
    $count = _my_module_count_options($instance);
    $widget['type'] = ($count > $limit ? 'autocomplete' : 'select');
  }
}

And hook_field_widget_form() never gets actually called for widget 'smart' - always defer to either 'select' or 'autocomplete'

LOL @ widget_am / widget_pm

Much saner :-).

The only thing that bugs me is the hook name, a bit imprecise / misleading - you're not altering the computed widget $element, nor the hook_widget_info() data. No real suggestion for now.

Yup, sounds better.

+  drupal_alter(array('field_widget', 'field_widget_' . $entity_type), $instance['widget'], $context);

does this work ? or is it just altering a local copy of $instance['widget'] that doesn't live up to the next line ?

$widget = $instance['widget'];
drupal_alter(array('field_widget', 'field_widget_' . $entity_type), $widget, $context);
$instance['widget'] = $widget;

?

Looks good to me.

+++ modules/field/field.api.php
@@ -1770,6 +1770,71 @@ function hook_field_extra_fields_display_alter(&$displays, $context) {
+ * Alters the widget properties of a field before it gets displayed.

Fields do not have widget properties; instances do. In fact drupal_alter is called on $instance['widget'].

+++ modules/field/field.api.php
@@ -1770,6 +1770,71 @@ function hook_field_extra_fields_display_alter(&$displays, $context) {
+ * hook_field_widget_properties_ENTITY_TYPE_alter() may be used to alter widget

I've been out of core development for a while. Has standard practice become for core hooks, or Field API hooks, to be defined separately per entity type like this?

We originally considered having many Field Attach API hooks called separately based on field type, entity type, phase of moon, etc. We eventually decided it was too much and went for just the base hooks. Unless that has been changed...

70 critical left. Go review some!

re bjaspan #17

Has standard practice become for core hooks, or Field API hooks, to be defined separately per entity type like this?

Not really. In this case, this mirrors the hook_field_display_alter() / hook_field_display_ENTITY_TYPE_alter() hooks introduced in #553298: Redesign the 'Manage Display' screen, and which do the same thing on the display side (alter display properties just before a field is displayed)
The granularity was needed over there because hook_field_display_alter() is called once per displayed field, and we want to avoid calling no-op implementations 100 times on a listing pages with 10 nodes x 10 fields. It also makes sense there because the main use case for this hook is to operate on a given view mode, and view modes semantics are per entity type.

Those two arguments in fact don't apply on the widget side :
- no view mode
- involves less fields in a page (no 10 nodes form),and performance is less critical.

So it's just a matter of symmetry between hook_field_display_alter() and hook_field_widget_properties_alter().
I could go both ways.

This being said -

We originally considered having many Field Attach API hooks called separately based on field type, entity type, phase of moon, etc. We eventually decided it was too much and went for just the base hooks. Unless that has been changed...

Having 'alternate' hooks got much easier and acceptable performance-wise since #765860: drupal_alter() fails to order modules correctly in some cases (was: Make drupal_alter() support multiple alter hooks executed by module weight'). This wasn't the case when we initially designed the Field API hooks.

I opened #785104: enrich hook_field_attach_X() with per-entity-type variants when the patch got in to try to figure if there could be other uses in Field API - although I was in a semi off-core period myself back then, and didn't really look back since.

The patch looks very good to me, and it is indeed important for modules that want to provide dynamic widgets (like the select box / autocomplete widget that Groups wants to provide).

Hmmmmm. This one I am less sold on than #629484: Add entity 'label' key info (e.g. title on node) since this seems more feature-ish, where the other one was more fixing an API oversight-ish. I am trying really hard to stamp the crap down on features wherever I find them, since we need to get D7 the (*@# out the door. :P~

This is apparently the last of three patches listed at http://drupal.org/project/og to get OG to work with D7, which is why I'm vaguely entertaining the idea. But it's not clear to me why you can't use similar form_alter() tricks to what OG D6 was using to achieve the same effect. I mean, sure, it would be nicer if Field API provided a hook for this. But unlike the other issue, I can't quite grok why this is necessary to achieve your aims, since the same widget-swapping trick is working just peachy in D6 without it.

So, in the meantime, here's some PHPDoc nit-picking.

+++ modules/field/field.api.php
@@ -1770,6 +1770,71 @@ function hook_field_extra_fields_display_alter(&$displays, $context) {
+     - field: The field that the widget belongs to.
...
+     - field: The field that the widget belongs to.

Missing *

+++ modules/field/field.api.php
@@ -1770,6 +1770,71 @@ function hook_field_extra_fields_display_alter(&$displays, $context) {
+ *  Alters the widget properties of a field instance on a given entity type
+ *  before it gets displayed.
+ *

Indented one too many times.

Powered by Dreditor.

Sorry. I meant that you are literally missing the '*' character at the start of those lines. :) PHPDoc stuff.

And yeah, let's hear what yched has to say.

re webchick #28 :

But it's not clear to me why you can't use similar form_alter() tricks to what OG D6 was using to achieve the same effect. I mean, sure, it would be nicer if Field API provided a hook for this

The purpose of hook_field_attach_form() is to react after Field elements have been added to an entity form. It was not formulated as an alter hook for inner consistency of the field_attach API (each field_attach_X() comes with its hook_field_attach_X() hook), and that's probably unfortunate.

So yes, altering FAPI properties for the widget can be done there, looking at the whole form - it's unfriendly and error prone, but can be done :
- detect if the form is about an entity that contains a field and widget type we're interested in (beware that the widget might not be there if field_access('edit') is denied)
- alter the FAPI $form[$field_name][$language] element in place
This won't work for widgets added outside of a regular entity form, though : widgets in Views exposed filters...

The hook added in the patch lets you act on the widget FAPI element individually, before it's drowned in the whole $form.
- It enhances "widgets as standalone, modular objects". It's a design flaw that you currently can't alter a widget on and by itself, but only as part of a larger form : detecting all forms where the widget can appear is impossible (think Panels, Views).
- It also restores the symmetry between formatters and widgets APIs, since a hook to alter the render array of an individual field exists on the display side : hook_field_display_alter(). This latter hook allows for consistency wherever the field gets displayed : regular [entity]_view(), Panels component, field in a table View...

I still stand for D7 inclusion :-(

This issue seems to have lost traction for the last few weeks. From yched's description this patch looks like it would be very useful for a number of modules. However beta2 is out already so its getting late. Do we know if this is going to get committed? If not can OG work using the approach outlined in #29?

yched is on board. Let's do this.

Note: I had to do this recently in CCK for Drupal 6, and it is a real pain. You have to resort building your own widget whenever you need to dynamically change the widget behavior.

Ok, then.

Committed to HEAD.