If you are attaching some option attributes to a link field (using a custom module for example), those attributes are removed on node save.
One solution is to use link_attributes, but you might not want to allow users to edit those attributes from node edit.

CommentFileSizeAuthor
#25 drupal-link-related-module-breakage-3056652-25.patch3.38 KBmaxstarkenburg
#19 interdiff-3056652-17-19.txt1.38 KByogeshmpawar
#19 3056652-19.patch3.38 KByogeshmpawar
#17 interdiff-3056652-15-17.txt870 bytesyogeshmpawar
#17 3056652-17.patch3.36 KByogeshmpawar
#15 3056652-15.patch3.34 KBranjith_kumar_k_u
#8 link-option-attributes-3056652-2.patch3.34 KBmashermike
#6 link-option-attributes-3056652-tests-only.patch2.59 KBmashermike
#2 link-option-attributes-3056652-1.patch766 bytesaalin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aalin created an issue. See original summary.

aalin’s picture

aalin CreditAttribution: aalin as a volunteer commented
Version: 8.7.x-dev » 8.8.x-dev
FileSize
link-option-attributes-3056652-1.patch766 bytes

changing $element['attributes'] = .. to $element['options']['attributes'] is fixing this

aalin’s picture

aalin CreditAttribution: aalin as a volunteer commented
Status: Active » Needs review
idebr’s picture

idebr CreditAttribution: idebr at iO commented
Issue tags: +Needs tests

@aalin Can you add a test case showcasing the current failing behavior?

aalin’s picture

aalin CreditAttribution: aalin as a volunteer commented

1. Set some custom option attributes to a field (consider that the field is not empty):

  $node = Node::load(1); // an existing node
  $field = $node->get('field_link')->first();
  // Set options attributes
  $fieldOptions = $field->get('options')->toArray();
  if (!isset($fieldOptions['attributes'])) {
    $fieldOptions = ['attributes' => []];
  }
  $fieldOptions['attributes'] += ['data-ga-event'=>'my-event'];
  $field->set('options', $fieldOptions);
  $node->save(); // save the node, that will update the link field

2. check that the custom options exists:

 $node = Node::load(1);
  kint( $node->get('field_link')->first()->getUrl()->getOption('attributes') );

this will output: [data-ga-event] => my-event

3. go to: /node/1/edit and save

4. check again custom options on field_link:

$node = Node::load(1);
  kint( $node->get('field_link')->first()->getUrl()->getOption('attributes') );

this will output NULL

mashermike’s picture

mashermike CreditAttribution: mashermike commented
FileSize
link-option-attributes-3056652-tests-only.patch2.59 KB

Status: Needs review » Needs work

The last submitted patch, 6: link-option-attributes-3056652-tests-only.patch, failed testing. View results

mashermike’s picture

mashermike CreditAttribution: mashermike commented
Status: Needs work » Needs review
FileSize
link-option-attributes-3056652-2.patch3.34 KB
mashermike’s picture

mashermike CreditAttribution: mashermike commented
Issue tags: -Needs tests

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u at Zyxware Technologies commented
FileSize
3056652-15.patch3.34 KB

Re-rolled #8 for 9.4.

Status: Needs review » Needs work

The last submitted patch, 15: 3056652-15.patch, failed testing. View results

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location 🇮🇳 Pune(MH)
CreditAttribution: yogeshmpawar at Portage CyberTech commented
Status: Needs work » Needs review
FileSize
3056652-17.patch3.36 KB
interdiff-3056652-15-17.txt870 bytes

Hope updated patch will fix test failures.

Status: Needs review » Needs work

The last submitted patch, 17: 3056652-17.patch, failed testing. View results

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location 🇮🇳 Pune(MH)
CreditAttribution: yogeshmpawar at Portage CyberTech commented
Status: Needs work » Needs review
FileSize
3056652-19.patch3.38 KB
interdiff-3056652-17-19.txt1.38 KB

Another try to fix test failures.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Reviewed & tested by the community

This looks valid. and the test cases seem to pass.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Issue tags: +Bug Smash Initiative
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.5.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.1.x, cherry-picked to 10.0.x, 9.5.x, 9.4.x, thanks!

  • catch committed 02cc55d on 10.0.x 
    Issue #3056652 by yogeshmpawar, mashermike, aalin, ranjith_kumar_k_u:...
  • catch committed 8eea239 on 10.1.x
    Issue #3056652 by yogeshmpawar, mashermike, aalin, ranjith_kumar_k_u:...
  • catch committed b9942ab on 9.4.x
    Issue #3056652 by yogeshmpawar, mashermike, aalin, ranjith_kumar_k_u:...
  • catch committed 2d7b035 on 9.5.x
    Issue #3056652 by yogeshmpawar, mashermike, aalin, ranjith_kumar_k_u:...
maxstarkenburg’s picture

maxstarkenburg
he/him
CreditAttribution: maxstarkenburg at TEN7 commented
FileSize
drupal-link-related-module-breakage-3056652-25.patch3.38 KB

I'm finding that this change is breaking some link-related contrib. For example, link_class and link_target no longer allow edits to their respective attribute values once a field has been saved the first time (#3301937: Link classes can't be edited/updated after core 9.4.5 update and #3302123: Targets can't be updated after core 9.4.5 update (ok as of 9.4.6)), and menu_link_attributes has a more "additive" problem now, with replacement values just being concatenated onto the previous one (#3302105: As of core 9.4.5, can't remove link attribute values once applied). Haven't (yet) tested link_attributes and other modules I might not be aware of that might be affected.

I don't know if the onus is on core to make a further change for this issue to work better with these modules, or whether it's on contrib to "get with the [new] program", so to speak, but in any case, since I have some affected sites where I can't just remove these modules, and also because I'm not sure about those modules' maintenance statuses, I've made a patch for core that basically reverts the change made here, to use in the meanwhile.

maxstarkenburg’s picture

maxstarkenburg
he/him
CreditAttribution: maxstarkenburg at TEN7 commented

Also, please let me know if best practice in a case like (especially where the change was already released) is to file a new issue instead of tacking on comments to the originating one?

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

Think it's best to open a new issue referencing this one.

maxstarkenburg’s picture

maxstarkenburg
he/him
CreditAttribution: maxstarkenburg at TEN7 commented

Thanks @smustgrave, I've created #3302472: Multiple link-related contrib modules broken by #3056652 to address the breakage (should probably port the patch over there too, though am already using the above one on a few of my composer.json files, heh).

idebr’s picture

idebr CreditAttribution: idebr at iO commented
Status: Reviewed & tested by the community » Fixed

Restoring the issue status to 'Fixed' as a follow-up issue is available at #3302472: Multiple link-related contrib modules broken by #3056652

idebr’s picture

idebr CreditAttribution: idebr at iO commented
Version: 9.5.x-dev » 9.4.x-dev

  • catch committed a75c1ca on 9.4.x 
    Revert "Issue #3056652 by yogeshmpawar, mashermike, aalin,...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.4.x-dev » 9.5.x-dev

I've just reverted this from the 9.4.x branch, so those modules will work again with no changes in the next patch release of 9.4.x. If the contrib modules need adapting to work with both versions of the form structure, then they should end up working with the (old) 9.4.x and (new) 9.5.x versions anyway.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Fixed » Needs work
Issue tags: +Needs change record, +Needs release note

fwiw if an issue has very recently been committed and introduced an obvious regression, it's OK to re-open it for a revert. For every other case, it's better to open a follow-up.

I thought about this some more and went-ahead and reverted from all branches.

If the contrib module just need to adapt to the new form structure, we can possibly re-commit this to 10.1.x-9.5.x with a release note and change record.

If this was a form element I probably would have thought about this before commit, but I wouldn't have expected contrib modules to be relying on a #type => value that wasn't working in core, so it'd be good to understand why the contrib modules broke due to this.

  • catch committed 4be368f on 10.0.x 
    Revert "Issue #3056652 by yogeshmpawar, mashermike, aalin,...
  • catch committed 4ab6486 on 10.1.x
    Revert "Issue #3056652 by yogeshmpawar, mashermike, aalin,...
  • catch committed 3ba7edd on 9.5.x
    Revert "Issue #3056652 by yogeshmpawar, mashermike, aalin,...
segx’s picture

segx CreditAttribution: segx commented

Patch #25 seems to be implemented and the problem is fixed in the 9.4.6 update. Please verify ... thank you!

maxstarkenburg’s picture

maxstarkenburg
he/him
CreditAttribution: maxstarkenburg at TEN7 commented

@segx I did a quick test with menu_link_attributes on a site after updating to core 9.4.6 and can verify that the problem is now absent with regard to that module (I would assume the other modules affected are now fine again too). The original issue posted at top however is, I suppose, back to square one.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.