Add better support for linking to anchors

Reroll without changing anything

#22/27 works with 8.x-5.x
#26 works with 6.0.x

What about passing just query and fragment. This keeps the plugin interface much cleaner.

The last submitted patch, 29: 3022261-29.patch, failed testing. View results →

Patch does not apply anymore

Please review #36

patch does not apply

Rerolled patch #36, please review it.

Thanks!

Rerolled the patch for latest changes.

+1 - would be great to get this feature in a stable release :-)

I've taken the patch in #41 and put it into a merge request to demonstrate passing test coverage against the latest development in Linkit.

Data attributes are currently removed again when adding a URL fragment manually. It seems that this is caused by https://www.drupal.org/project/linkit/issues/3443845 🐛 When [Enter] key is used to replace a link, the previous link's attributes are not removed Fixed . Commenting out 3 lines in /js/ckeditor5_plugins/linkit/src/index.js and rebuilding /js/build/linkit.js fixes the issue:

const linkit = editor.plugins.get('Linkit');
linkFormView.urlInputView.fieldView.element.addEventListener('input', function (evt) {
  //linkit.set('entityType', null);
  //linkit.set('entityUuid', null);
  //linkit.set('entitySubstitution', null);
});

Steps to reproduce

1. Create a link by selecting a node from autocomplete: the data attributes are added correctly.
2. Edit the link by adding a URL fragment manually and hitting the enter key: the data attributes are removed.
3. After commenting out the above mentioned code and rebuilding the javascript file, the data attributes remain after step 2

Patch at #41 works perfectly for me with Linkit 6.1.4 and Drupal 10.3

Thanks

The problem is that when you append the fragment to the path, LinkIt will no longer return the data-entity-type and data-entity-uuid attribues for the

Testing this today with the 7.x branch, it appears that the data-entity-type and data-entity-uuid attributes are preserved just fine when an anchor/fragment is present:

Is there another scenario where this is still problematic?

Patch from #41 works and fixes the issue on 6.1.4. I also tested on 7.0.0-alpha1 version and there the issue still remains but on dev-7.x it is fixed and doesn't need patching.

Atleast for our projects, we'll continue using the 6.1.4 version with the patch until issue is fixed in a stable version.

Maybe I have a unique configuration, but the patch in #41 does not work for me.

Drupal 10.3.6
Linkit 6.1.4
CKEditor Anchor Link 3.0.0-beta1

When I go to save the link, either by selecting the matched node or tabbing out of the URL field, the anchor portion of the link (“#foo”) is stripped away.

Sorry, please disregard, Linkit wasn’t being patched properly in my local environment. Patch in #41 worked for me too.

I believe RTBC is the proper status for this…

Okay, thanks for the work and the information on which branch this is still needed for.

I also tested on 7.0.0-alpha1 version and there the issue still remains but on dev-7.x it is fixed and doesn't need patching.

I've released a new version of the 7.x branch, 7.0.0-alpha2.

Patch from #41 works and fixes the issue on 6.1.4.

I've merged this into the 6.1.x branch and released 6.1.5.

- Drupal 10.3.6
- Linkit 7.0.0-alpha2
- CKEditor Anchor Link 3.0.0-beta1

I have the same problem as #52: when I insert the anchor it is deleted.

This issue introduced a breaking change:

  public function getUrl(EntityInterface $entity, array $options = []);

As a result, media_entity_download for example has a fatal error again with this.

This was also proposed in another issue where I had already provided feedback about that.

IMHO this should be reverted, or a follow-up should be done to remove the argument from the interface.

Found the issue: ✨ Allow "Linkit URL converter" filter to generate links based on the language of the current page, rather than the language of the referenced entity Postponed: needs info .

IMHO this should be reverted, or a follow-up should be done to remove the argument from the interface.

Re-opening this issue, with the plan to revert the change so that the 6.1.x branch does not introduce BC breaking changes...

Okay, release 6.1.6, published on 2024-10-17, reverts this change: https://www.drupal.org/project/linkit/releases/6.1.6 →

Setting this back to "Needs work" for an implementation that avoids BC-breaking changes.

I think you can only do this in the 6.1.x to provide a warning that a new param will be added in the next major:

public function getUrl(EntityInterface $entity /*, array $options = [] */);

We already updated to 6.1.5 :(

Created a new MR containing the suggested change of kim.pepper

I think you can only do this in the 6.1.x to provide a warning that a new param will be added in the next major:

Based on my testing in comment #50 and confirmation from another in #51, this is effectively resolved in the next major version. The intent in the recent merge here was to provide equivalent functionality for folks who still cannot upgrade to Linkit 7.x due to API changes introduced there.

Thanks @casey for providing the MR that does not include the second parameter to getUrl(). Folks who are interested in this change in 6.1.x, please review!

> this is effectively resolved in the next major version

Do we know *why* it was resolved there? 7.x doesn't have $options, so apparently there are other solutions that don't require this.

Even if it's fixed there, the test coverage should be ported to 7.x to verify that there.

This issue and patches here likely predate the change to return URL objects. Just like ✨ Allow "Linkit URL converter" filter to generate links based on the language of the current page, rather than the language of the referenced entity Postponed: needs info , where I made the same arguments, why are we passing in $options, set it on the URL object that we then return again? Can't we just set it on the returned URL object (while keeping BC for 6.x where it might not be a URL object yet)

That other issue moved away from this approach too, didn't test it yet, but apparently that worked there too, with a very similar requirement.

The current MR produced a fatal error when using the LinkitFormatter:

ValueError: func_get_arg(): Argument #1 ($position) must be less than the number of the arguments passed to the currently executed function in func_get_arg() (line 24 of modules/contrib/linkit/src/Plugin/Linkit/Substitution/Canonical.php).

That's because the formatter does not pass $options, so I quickly added sanity checks to all occurrences of func_get_arg(1).

Apart from that, I agree with @berdir. I don't understand why we are passing the options to the substitution plugin that creates the URL, and still process the fragment and query parameters afterwards. I also think this processing code has a bug (left a comment on the MR).