field_behaviors_widget() is a thing from CCK D6, it is mostly stale now.
Code currently calling it to check the 'default_value' or 'multiple_values' properties of the widgets need to
- call $options = entity_get_form_display()->getComponent($field_name)
- then call WidgetPluginManager::getDefinition($options['type'])

This should probably move to a getRendererDefinition() method on the EntityDisplay class ?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Issue tags: +Field API

tagging

yched’s picture

Issue tags: +API change

and tagging

jlindsey15’s picture

Assigned: Unassigned » jlindsey15

I'll take this.

jlindsey15’s picture

Status: Active » Needs review
FileSize
4.85 KB

Here's my first try. I put it in the EntityFormDisplay class instead - that seemed to make more sense.

Status: Needs review » Needs work

The last submitted patch, field-behaviors-widget-2047997-4.patch, failed testing.

yched’s picture

+++ b/core/modules/entity/lib/Drupal/entity/EntityFormDisplayInterface.phpundefined
@@ -14,4 +14,17 @@
+  /**
+   * Returns the definition of a renderer plugin for the given field (e.g.
+   * widget, formatter).
+   *
+   * @param string $field_name
+   *   The name of the field.
+   *
+   * @return array|null
+   *   The widget or formatter plugin, or NULL if the field does not exist.

The first line of functions/methods phpdoc should fit in 80 chars if possible.

Maybe "Returns the definition of the renderer plugin used for a component" ?
Then the @return section could be:
"The definition of the renderer plugin (e.g. widget, formatter), or NULL if the component is not set."

+++ b/core/modules/entity/lib/Drupal/entity/EntityFormDisplayInterface.phpundefined
@@ -14,4 +14,17 @@
+  public function getRendererDefinition($field_name);

As a replacement of field_behaviors_widget(), we need the method to exist on EntityFormDisplay, true.
But generally speaking, the method would also make sense on EntityDisplay objects.
So I'd suggest adding it to EntityDisplayBaseInterface & EntityDisplayBase

+++ b/core/modules/entity/lib/Drupal/entity/Plugin/Core/Entity/EntityFormDisplay.phpundefined
@@ -45,6 +45,14 @@ public function __construct(array $values, $entity_type) {
+    $options = getComponent($field_name);

Should be $this->getComponent($field_name) :-)
(probably explains that patch fails to install)

Also, it should be wrapped in an if() because the field might be hidden, in which case getComponent() will return NULL:

if ($options = $this->getComponent($field_name)) {
  return $this->pluginManager->...
}
jlindsey15’s picture

Thanks for the comments - good point about putting it in EntityDisplayBase. When I'm flip-flopping between coding in PHP and java, I always forget to do this->function() instead of just function()... I'll have the reworked patch in a bit.

jlindsey15’s picture

Status: Needs work » Needs review
FileSize
4.77 KB

Status: Needs review » Needs work

The last submitted patch, field-behaviors-widget-2047997-8.patch, failed testing.

webchick’s picture

So granted I've been out of the loop awhile, but I'm a bit confused why we are ripping functions out in
"normal" tasks when we're past API freeze..?

At the very least, we should keep the old function there with a @deprecated thing, no?

yched’s picture

I'm fine with keeping the function around as @deprecated, but:

- This function makes very very little sense in D8. "Give me this property from the plugin definition of the widget used by this field".
But now that we have "form modes", there is no such thing as "the (one and only) widget used by this field". Just like "what's the formatter used by this field" makes no sense - in which view mode ?
The function currently uses the widget used in the 'default' form mode, but this makes no special sense.
- It's really a remnant from CCK D6 where widget info definitions had a 'behaviors' entry, which doesn't exist anymore. Its only possible remaining use is for "default values", and it's a sick use, that we need to get rid of anyway : #2056405: Let field types provide their support for "default values" and associated input UI

jlindsey15’s picture

Status: Needs work » Needs review
FileSize
4.18 KB

Added it back as @deprecated, still haven't fixed the bug in the patch though. @deprecated doc may change depending on how it gets fixed.

Status: Needs review » Needs work

The last submitted patch, field-behaviors-widget-2047997-12.patch, failed testing.

jlindsey15’s picture

Status: Needs work » Needs review
FileSize
4.2 KB

See comment 12 description

The last submitted patch, field-behaviors-widget-2047997-14.patch, failed testing.

yched’s picture

Status: Needs review » Needs work

Now that #2056405: Let field types provide their support for "default values" and associated input UI is in and the 'default_value' entry in widget plugin definitions is not supported anymore, we have no reason whatsoever to keep field_behaviors_widget().
We should really just remove it.

swentel’s picture

Status: Needs work » Needs review
FileSize
1.92 KB

Let's see

yched’s picture

 + if (!isset($instance['default_value'])) {
+  $instance['default_value'] = NULL;
+} 

This looks like this might be useless now ?

swentel’s picture

FileSize
670 bytes
1.97 KB

Yeah, probably, let's see.

yched’s picture

Status: Needs review » Reviewed & tested by the community

Looks good if green

catch’s picture

Title: Get rid of field_behaviors_widget() » Change notice: Get rid of field_behaviors_widget()
Priority: Normal » Major
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

Committed/pushed to 8.x.

Could use a short change notice - not much more than the image summary.

yched’s picture

Title: Change notice: Get rid of field_behaviors_widget() » Get rid of field_behaviors_widget()
Assigned: jlindsey15 » Unassigned
Priority: Major » Normal
Status: Active » Fixed
Issue tags: -Needs change record
asimmonds’s picture

Status: Fixed » Needs review
FileSize
1.06 KB

Should the three FIELD_BEHAVIOR_* constant definitions in field.module be removed as well?
I can't find anything in core that still uses them.

swentel’s picture

Status: Needs review » Reviewed & tested by the community

Right, duh, good catch, thanks! :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

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