Problem/Motivation

By default module linkit returns the system path in this format: /node/xxx, and this value is used as href(correct), but also in text -> where we would appreciate the entity title.

Steps to reproduce

Create a link using linkit(with node list autocomplete) using ckeditor5 plugin when no text is selected.

Proposed resolution

Automatically use the entity label as text in A tag - e.g <a href="/node/XXX" data-entity-type="node" data-entity-uuid="..." data-entity-substitution="canonical">Lorem ipsum dolar</a> instead of <a href="/node/XXX" data-entity-type="node" data-entity-uuid="..." data-entity-substitution="canonical">/node/XXX</a>

thank you in advance!

Issue fork linkit-3388565

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

evonasek created an issue. See original summary.

mark_fullmer’s picture

Status: Active » Postponed (maintainer needs more info)

This sounds like a duplicate of #2966320: Show entity title after autocomplete selection instead of internal route (e.g., /node/123). If the original poster can confirm that, let's mark this as a duplicate.

gskharmujai’s picture

After trying out the patch that is available in Show entity title after autocomplete selection instead of internal route (e.g., /node/123), the patch applied successfully for D10 but it failed to address the required functionality which involves CKEditor 5.

In D9 this was implemented using a core patch Display node title by default when creating link with drupalink, allow configuration. but now with CKEditor5 as default in D10, a core patch dosent seem feasible at all.

I am also looking for a fix to this issue but no headway as yet considering my limited knowledge on the new features in CKEditor 5.

evonasek’s picture

This is not a duplicate issue. We have also tried to install the patch from https://www.drupal.org/project/linkit/issues/2966320, but this is not working in the ckeditor5 plugin. Please keep this issue open.

mark_fullmer’s picture

Thanks for the additional responses. If this is not a duplicate issue, please explain how the goal or purpose of this issue is different from the goal of #2966320: Show entity title after autocomplete selection instead of internal route (e.g., /node/123). If that issue is what you want but the current patch is not working, we typically don't open a new issue; we try to explain what is not working there and resolve it.

Based on a few responses, it sounds like you're saying that #2966320: Show entity title after autocomplete selection instead of internal route (e.g., /node/123) is not working with the CKEditor 5 plugin. My preference as a maintainer of this module is that this should be stated on that issue and work should continue there to make the patch compatible with either CKEditor 4 and CKEditor 5, rather than having one issue for CKEditor 4 and a different issue for CKEditor 5.

Thank you!

evonasek’s picture

Thank you Mark. I have commented https://www.drupal.org/project/linkit/issues/2966320#comment-15245574, so, please close this issue.

mark_fullmer’s picture

Status: Postponed (maintainer needs more info) » Closed (duplicate)

Thanks for the clarification. Closing this issue as a duplicate.

klidifia’s picture

This is specifically about what the default link text is when a link is inserted via the plugin button into CKE so I don't think this is a duplicate of 2966320 ((ref comment #3 paragraph 2 (Issue 2961554).

That issue no longer applies in D10 because the Drupal form used as an intermediary between Drupal/CKEditor no longer exists (well; it's deprecated - (core/modules/editor/src/Form/EditorLinkDialog.php).

I am not sure the issue outlined here can be solved within Linkit now -- I don't know how this can be achieved because CKEditor5 doesn't have a separate field for what the default text should be.

klidifia’s picture

Version: 6.0.0 » 7.x-dev
Priority: Major » Normal

I needed to solve this so have altered JS so when creating a new (internal) link in CKEditor5 using the Link function and locating the entity using LinkIt - the URL alias is inserted as the default link text.

There are a lot of other issues in Linkit around this or related to this:

#2961554: Display node title by default when creating link with drupalink, allow configuration. (Core) - This is where a solution had been made until Drupal 10 / CKEditor5
#2877535: Show URL alias after autocomplete selection instead of internal route (e.g., /node/123)
#2966320: Show entity title after autocomplete selection instead of internal route (e.g., /node/123)
#3014297: Use target name as anchor text
#2827151: Insert node title instead of # when no link text is selected
#2926898: Allow customization of default text when text not selected

nick hope’s picture

@klidifia Thanks for the heads-up on the related issue.

I made a patch out of #10 and patched 7.0.0-alpha1 with it.

In my testing (linking to taxonomy terms only, which is my use case), if a link to a taxonomy term is inserted in an article without first selecting text, the small link window does not dismiss when the green arrow is clicked. I can dismiss that window by clicking outside it, in the body of the article. Then the link is just shown as a vertical bar (like a pipe symbol). When I save the article, the taxonomy term name is shown, but followed by the term route. e.g. Thailand/taxonomy/term/683, where "Thailand" is the term name.

klidifia’s picture

@nick-hope thanks for trying it. I can't see what might be happening, I imagine it is producing some sort of JS error in console when not closing the Link window. I tried the same on a Fresh D10 install. Configured the default Linkit matcher to include taxonomy terms, made a term (Thailand) and tested on a new page in a few separate places. (Type 'Thai', select via Linkit, enters Link URL as /taxonomy/term/1, click green tick, entered as Thailand.)

<p>
    <a href="/taxonomy/term/1" data-entity-type="taxonomy_term" data-entity-uuid="c2415acd-08e0-494d-b51d-09acc1728f0d" data-entity-substitution="canonical" data-entity-title="Thailand">Thailand</a>
</p>
nick hope’s picture

@klidifia Thanks for testing it.

I'm testing with Drupal 10.2.6, Linkit 7.0.0-alpha1 with a patch downloaded from your last MR. I've tried on an old Drupal D10 project and a near-vanilla D10 project. Tried articles, basic pages, basic HTML, full HTML, default Linkit profile, new "Taxonomy Terms" Linkit profile.

I'm finding that the link window will close correctly for the first link I create in an article, but today I'm getting the route before the term name in the anchor text (reverse of yesterday), like this:

<a href="/taxonomy/term/9" data-entity-type="taxonomy_term" data-entity-uuid="2c20c5cb-dee2-4af4-9408-2394309a3c19" data-entity-substitution="canonical" data-entity-title="Thailand">/taxonomy/term/9Thailand</a>

When I try to create a 2nd link in the same article, I get this console error in Chrome (I'm afraid I'm not much use at interpreting it):

ckeditor5-dll.js?v=40.2.0:5 Uncaught TypeError: Cannot read properties of undefined (reading 'attributes')
    at Ar.getChanges (ckeditor5-dll.js?v=40.2.0:5:376656)
    at html-support.js?v=40.2.0:5:44080
    at $r._callPostFixers (ckeditor5-dll.js?v=40.2.0:5:390254)
    at $r._handleChangeBlock (ckeditor5-dll.js?v=40.2.0:5:389512)
    at Cn._runPendingChanges (ckeditor5-dll.js?v=40.2.0:5:424836)
    at Cn.change (ckeditor5-dll.js?v=40.2.0:5:421831)
    at r.on.priority (editorAdvancedLink.js?v=10.2.6:1:7035)
    at oe.fire (ckeditor5-dll.js?v=40.2.0:5:604093)
    at <computed> [as execute] (ckeditor5-dll.js?v=40.2.0:5:607777)
    at f.execute (ckeditor5-dll.js?v=40.2.0:5:112967)
rethrowUnexpectedError @ ckeditor5-dll.js?v=40.2.0:5
change @ ckeditor5-dll.js?v=40.2.0:5
r.on.priority @ editorAdvancedLink.js?v=10.2.6:1
fire @ ckeditor5-dll.js?v=40.2.0:5
<computed> @ ckeditor5-dll.js?v=40.2.0:5
execute @ ckeditor5-dll.js?v=40.2.0:5
execute @ ckeditor5-dll.js?v=40.2.0:5
(anonymous) @ link.js?v=40.2.0:5
fire @ ckeditor5-dll.js?v=40.2.0:5
e.listenTo.useCapture @ ckeditor5-dll.js?v=40.2.0:5
fire @ ckeditor5-dll.js?v=40.2.0:5
t @ ckeditor5-dll.js?v=40.2.0:5

I could do a screen recording with a completely fresh D10 project if that would help, so we can see if/where our steps differ.

ericgsmith’s picture

Issue summary: View changes
Status: Closed (duplicate) » Needs work
StatusFileSize
new1.49 MB

Nice work @klidifia!

I am reopening this issue since its hidden being closed when there is work continuing on it.

I agree that it sounds like it is not exactly a duplicate of #2966320: Show entity title after autocomplete selection instead of internal route (e.g., /node/123) although that issue would be useful alongside this one - but that is specifically talking about the UI shown after making a selection from the autocomplete - where as this is about the default text content of the link.

I have this a test on a clean install and it looks good to me - updated issue summary with a gif recording of it.

I think the existing LinkitDialogCKEditor5Test should be updated to show that the default text is the entity label. Setting to needs work for this change.

I also did not get the error reported in #11 - but I see in the js log it mentions editorAdvancedLink - I have not tested in combination with this module but could that be doing anything to interfere with the attributes?

nick hope’s picture

Good spot @ericgsmith. It does indeed seem to be an incompatibility with the Editor Advanced Link module. If I uninstall that, then this patch works correctly. Apologies for that oversight.

Editor Advanced Link is quite a widely adopted module (109k sites), so this incompatibility should probably be addressed before (if) this feature is added to Linkit. Not sure which side it would need addressing from.

There are also important views and discussion on the related issues (example) that should be considered.

klidifia’s picture

Thanks both -- Yeah I can replicate that with installing Editor Advanced Module.

I note the code block on both modules is basically the same around the _addExtraAttributeOnLinkCommandExecute

Both have same priority high, if I change editoradvancedlink to highest, then it works OK. I'm not sure yet of the best way to make them play nice together and if there are other issues with both modules in play.

jessmm made their first commit to this issue’s fork.

jessmm changed the visibility of the branch linkit-3388565-3388565-7 to hidden.

ericgsmith’s picture

Rebased MR after conflicts introduced when #3447669: No longer a way to post a link as-is if there is 1 pattern match was comitted.

ever-c0de’s picture

StatusFileSize
new68.12 KB

Ignore this one

ever-c0de’s picture

StatusFileSize
new19.18 KB

Latest diff/patch based on current MR.

ever-c0de’s picture

StatusFileSize
new19.18 KB
drasgardian’s picture

As outlined in #16, this patch does not play nicely if editor_advanced_link is also enabled.

The ckeditor5 plugin in editor_advanced_link borrows some code from this module, so the changes here will also be needed in that plugin to operate ok together.

I have opened a MR on the editor_advanced_link project with the changes needed.
https://www.drupal.org/project/editor_advanced_link/issues/3526372

I also found that the changes here also don't work nicely with linkit_media_library so submitted a patch there too:
https://www.drupal.org/project/linkit_media_library/issues/3526417

drasgardian’s picture

drasgardian’s picture

ericgsmith’s picture

Status: Needs work » Needs review

Thanks @drasgardian - that very good information to know, thank you for creating the relating issues.

From what I can see from your related is this code / change for Linkit itself is then ready to review as the issue is the responsibility of the other module?

I that case, I think I can move this back to needs review.

Have rebased against the latest 7.x changes

ericgsmith’s picture

Rebased against latest changes from 7.x - saw in #3535098: [CKEditor v45] Cannot replace 'Displayed text' of existing links changes from null to empty string so applied that to the new attribute we introduce here.

Since I have rebased this multiple times are we have lots of commits I've moved the build js to a separate commit. I think this makes it easier to review the commits and also will make it easier to resolve merge conflicts in future as we just need to build the assets once vs for each commit.

mark_fullmer’s picture

Title: Display node title (a text) in by default when creating link in ckeditor5 » Populate displayedText with entity title when left blank
mark_fullmer’s picture

Status: Needs review » Needs work

Note that Drupal 10.5/11.2 updated to CKEditor version 45, which has a redesigned link interface that includes a new input element for the displayed text: https://ckeditor.com/blog/ckeditor-45-0-0-release-highlights/#smarter-li...

The work here will need to be updated to accommodate this version of CKEditor's interface

ericgsmith’s picture

StatusFileSize
new15.03 KB

@mark_fullmer thanks Mark, I missed that change when rebasing.

Should be a simple enough change to set the value of "Displayed text" if its empty instead of waiting for the link to be inserted - I'll hopefully have some time to take a closer look soon.

ericgsmith’s picture

Status: Needs work » Needs review
StatusFileSize
new174.5 KB

Love to see this - not only is the UI better but the change is now super simple - can remove all the changes to the attributes that we first tried.

Back for review - example here showing the new behaviour when no text has been entered, and existing behaviour to keep text if already entered:

Given we are no longer messing with attributes - I wonder if the previous issues reported with editor advanced link would also be resolved with this change? Anybody that was previously hitting an issue with that able to verify if its still present?

@drasgardian re your related issues in other modules - from a quick look it seems they could also be simplified to interact with the formView itself and thus they would not be dependent in this change. Haven't looked too closely though.

mark_fullmer’s picture

Status: Needs review » Fixed

The narrow changes based on the refactor in #31 make good sense for the CKEditor version 45 API. I'm happy to merge this in, with the understanding that it will be applicable for sites running Drupal 10.5+ or 11.2+, and will not introduce backwards-compatibility-breaking problems for sites on older versions of Drupal. Thanks, everyone!

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

ericgsmith’s picture

Priority: Normal » Critical
Status: Fixed » Needs review

Hi @mark_fullmer - thank you for comitting - massively sorry about this but I did break backwards incompatibility for older CKEditor versions. - they get an error with object property undefined which breaks the select handler completely.

Have pushed a fix for this to https://git.drupalcode.org/project/linkit/-/merge_requests/145/diffs

jastraat’s picture

Cross posting that this commit in the new LinkIt release also results in this bug: https://www.drupal.org/project/linkit/issues/3552727 (in Drupal 10.5.4 so the current version of ckeditor)

ericgsmith’s picture

This issue was committed after 7.10.0 - it is not part of a release? https://git.drupalcode.org/project/linkit/-/commits/7.x

ericgsmith’s picture

In fact, completely unintentionally I think this issue resolves whatever the bug in 7.10.0 is. I'll move this discussion to your issue

jastraat’s picture

Interesting - yes sorry. There was a commit not in the release notes but it wasn't this one.

mark_fullmer’s picture

Status: Needs review » Fixed

Thanks, @ericgsmith , for the follow-up MR. This does resolve the breakage on older versions of Drupal (with CKEditor5 < v45) but the data attributes aren't populated on those versions. I've committed this as a partial fix, and have staged what I think completes the work -- ensuring those attributes are present -- in #3552727: [7.0.10 regression] Data attributes no longer added to new link when displayed text empty

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

mlncn’s picture

Just noting as this issue gets fixed that there is one more permutation of the basic problem:

  1. Get the link text from the title of the entity being linked to.
  2. When the entity's title changes, update the text of the link to match the title.

… and coincidentally there is now a module for that: Autoupdate Link Text.

mark_fullmer’s picture

… and coincidentally there is now a module for that: Autoupdate Link Text.

Thanks for sharing! Created just today, huh?! It looks like currently that module only supports this autoupdating for link _fields_, as opposed to the more common usage of Linkit for links within CKEditor rich text areas (developing a methodology for updating those would presumably be a big task!). That should probably be called out in that module's project page more clearly.

Status: Fixed » Closed (fixed)

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