Problem/Motivation
According to the Form API Reference an element of #type value can only use the properties #type and #value.
LinkWidget::formElement however defines #tree and #attributes. Especially the CSS class does not make any sense, since according to the FAPI reference a value is:
A form value that is internal to the form and never displayed to the screen.
// Exposing the attributes array in the widget is left for alternate and more
// advanced field widgets.
$element['attributes'] = array(
'#type' => 'value',
'#tree' => TRUE,
'#value' => !empty($items[$delta]->options['attributes']) ? $items[$delta]->options['attributes'] : array(),
'#attributes' => array('class' => array('link-field-widget-attributes')),
);
Proposed resolution
Delete the unused keys:
// Exposing the attributes array in the widget is left for alternate and more
// advanced field widgets.
$element['attributes'] = array(
'#type' => 'value',
'#value' => !empty($items[$delta]->options['attributes']) ? $items[$delta]->options['attributes'] : array(),
);
Remaining tasks
Review, feedback, commit.
User interface changes
API changes
Data model changes
Comments
Comment #1
killerpoke commentedI also think, that #tree and #attributes shouldn't be defined here. They make no sense to me.
Comment #2
mac_weber commentedComment #3
mgiffordRe-uploading patch for the bots.
Comment #4
mac_weber commentedThis is more a 'task' than a 'bug'.
The patch was missing to get rid of the configuration.
Comment #6
mac_weber commentedSomehow we need the attributes to migrate data from Drupal 6. I don't know how these attributes are used in D6, but maybe they could be migrated as existing configurations we already have in D8.
Comment #8
mac_weber commentedSo, the schema is needed for migration purposes.
Retesting the latest patch.
Comment #9
mac_weber commentedComment #21
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge require as a guide.
Was going to tag for subsystem maintainer review but see Mac_Weber that's you!
Wondering if this is still relevant for D10? Seems like it could be but will need a D10 version.
Comment #22
prem suthar commentedRerool the Patch For the 10.1 Branch.
Comment #26
dcam commentedI converted #22 to an MR.
In the #4 patch
$element['attributes']got confused with$elements['attributes']['#attributes']. This issue is about removing the latter.But then the schema for
$element['attributes']got removed in #4. That caused D6 migration errors, which makes sense.The MR should only be about removing the unused array keys from the
valueelement.Comment #27
dcam commentedComment #28
dcam commentedComment #29
smustgrave commentedSeems straight forward. Thanks for picking up this super old ticket @dcam!
Comment #30
astonvictor commentedComment #31
astonvictor commentedFixed the link in the description
+1 RTBC
Comment #32
astonvictor commentedComment #33
astonvictor commentedComment #34
catchCommitted/pushed to 11.x, thanks!