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')),
);
According to the patch #4 the attributes
are need to migrate from Drupal 6. Then, we need to check how these attributes are used in D6 in order to check if they can be migrated as configuration data we already have in D8.
Proposed resolution
Remaining tasks
Check how attributes
are used in D6 in order to check if they can be migrated as configuration data we already have in D8.
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#22 | 2354807-22.patch | 791 bytes | Prem Suthar |
#3 | link-remove_inappropriate_properties-2354807-2.patch | 1.21 KB | mgifford |
#2 | link-remove_inappropriate_properties-2354807-2.patch | 1.21 KB | Mac_Weber |
Comments
Comment #1
killerpoke CreditAttribution: killerpoke commentedI also think, that #tree and #attributes shouldn't be defined here. They make no sense to me.
Comment #2
Mac_Weber CreditAttribution: Mac_Weber commentedComment #3
mgiffordRe-uploading patch for the bots.
Comment #4
Mac_Weber CreditAttribution: Mac_Weber commentedThis is more a 'task' than a 'bug'.
The patch was missing to get rid of the configuration.
Comment #6
Mac_Weber CreditAttribution: 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 CreditAttribution: Mac_Weber commentedSo, the schema is needed for migration purposes.
Retesting the latest patch.
Comment #9
Mac_Weber CreditAttribution: Mac_Weber commentedComment #21
smustgrave CreditAttribution: smustgrave at Mobomo 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 CreditAttribution: Prem Suthar at Srijan | A Material+ Company for Drupal India Association commentedRerool the Patch For the 10.1 Branch.