Problem/Motivation

When the Anchor button is enabled on our CKEditor5 editor, IDs disappear from links when editing a node.

Steps to reproduce

Note that at no point in discovering and reproducing this issue were we actually *using* the anchor button -- it was simply enabled on the editor's toolbar.

I created a basic page node with simple text reading "This is a test." I tried two different ways to create the desired link -- using the source editor, and using the Advanced Link functionality with the link button to add the URL and an ID. Both ways, I ended up with simply:

<a href="https://drupal.org" id="link-id-test">test</a>

I saved the link and confirmed that it was working and had the ID on it.

I then edited the node again. Checking either the source editor or the link settings in the link dialog both revealed that the ID was gone -- not just empty, the attribute had been removed entirely. The src attribute was still present.

The problem occurs while loading the editor -- if I don't save the node, the ID attribute is still present. If I re-add it and save, it's present until I edit again.

If I remove the Anchor icon from the toolbar, this problem stops happening. (Doesn't matter if I actually uninstall the module.)

Proposed resolution

Update the CKEditor5 Anchor integration to preserve id attributes on links when the Anchor button is enabled.

  • Support anchors with hrefs #16
  • Added id attribute to the allowed model schema and conversion handling.
  • Ensured id persists across editing and saving.

Remaining tasks

  • ✅ File an issue
  • ➖ Addition/Change/Update/Fix
  • ➖ Testing to ensure no regression
  • ➖ Automated unit testing coverage
  • ➖ Automated functional testing coverage
  • ➖ UX/UI designer responsibilities
  • ➖ Readability
  • ➖ Accessibility
  • ➖ Performance
  • ➖ Security
  • ➖ Documentation
  • ➖ Code review by maintainers
  • ➖ Full testing and approval
  • ➖ Credit contributors
  • ➖ Review with the product owner
  • ➖ Release notes snippet
  • ✅ Release

API changes

  • N/A

Data model changes

  • N/A

Release notes snippet

  • N/A
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

wrd-oaitsd created an issue. See original summary.

justcaldwell’s picture

I can confirm this behavior. We use Editor Advanced Link (2.2.4) to allow editors to add ARIA-label, classes and ID to links.

After Anchor Link is added to the toolbar, any previously-added ID attributes are removed from link tags when the entity is next edited. As noted, no actual use of Anchor Link is necessary. ARIA-label and class attributes are not affected.

justcaldwell’s picture

This would need to be resolved upstream on northernco/ckeditor5-anchor-drupal, but there's nowhere to open an issue there.

justcaldwell’s picture

StatusFileSize
new20.51 KB
new21.76 KB

Here's an example where I created a link (with an href), then selected the entire link and made it into an anchor, which sets the ID attribute and adds the ck-anchor class. You can see below that the UI indicates that both are in effect (both the link and anchor button are "active").

Anchor link in effect.

On initial save, the ID remains intact in the rendered page. If I make any subsequent change to the page and save it, the ID is stripped and the anchor is silently broken.

To add to potential confusion, if I edit the page again, the anchor flag icon still appears (because the ck-anchor class was not stripped) so the anchor still appears to be intact.

Anchor icon, but no ID.

justcaldwell’s picture

Priority: Normal » Major

I think this is at least major, as we're losing user input/data.

jwilson3’s picture

Priority: Major » Critical

I would say this is actually more like Critical since it results in data loss of existing data.

https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-...

tagpy’s picture

Hi,
Below lines of code($plugin_definition['ckeditor5']['config']['htmlSupport']['disallow'][] ) stripping the link and other attributes.

function anchor_link_ckeditor5_plugin_info_alter(array &$plugin_definitions): void {
  $plugins_to_override = [
    'ckeditor5_arbitraryHtmlSupport',
  ];
  foreach ($plugins_to_override as $plugin_id) {
    if (!isset($plugin_definitions[$plugin_id])) {
      return;
    }
    $plugin_definition = $plugin_definitions[$plugin_id]->toArray();
    // Make plugin-specific alterations. Disallow the General HTML Support
    // plugin from controlling links with the attributes handled by the
    // Anchor plugin.
    $plugin_definition['ckeditor5']['config']['htmlSupport']['disallow'][] = [
      'name' => 'a',
      'attributes' => [
        'id',
        'name',
      ],
      'classes' => [
        'ck-anchor',
      ],
    ];
    // Update plugin definitions.
    $plugin_definitions[$plugin_id] = new CKEditor5PluginDefinition($plugin_definition);
  }
}
tagpy’s picture

Assigned: Unassigned » tagpy

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

jwilson3’s picture

I'm confused whether @tagpy should still be assigned given @bedlam created the issue fork.

Am anxious to review this if possible, I see a first commit, but don't see any MR yet.

@bedlam what is the status here?

justcaldwell’s picture

FWIW, I don't think the code referenced in #7 plays a role in this issue. Per the comment in that code...

    // Make plugin-specific alterations. Disallow the General HTML Support
    // plugin from controlling links with the attributes handled by the
    // Anchor plugin.

...it's just preventing ckeditor's own General HTML Support plugin from acting on links that anchor_link wants to handle. That is, <a> elements with id and/or name attributes and the ck-anchor class. It's not actually stripping any attributes.

justcaldwell’s picture

StatusFileSize
new21.34 KB

Okay, I think this is where at least part of the problem begins in the ckeditor5-anchor-drupal plugin. In that upcast function, if the <a> tag has an href, the upcast bails out early (lines 134-136):

if (viewElement.hasAttribute('href')) {
    return;
}

(The code above was explicitly added to the drupal version of the anchor plugin in this commit.)

When I comment out those lines and rebuild the plugin, IDs are no longer stripped from existing links.

BUT, that introduces a new issue. Since anchor_link is handling any <a> with the ck-anchor class, when you place the cursor in the link text, the anchor_text UI popup appears — the link UI popup is no longer available.

Link with anchor only has anchor_link UI popup

So editors would no longer be able to edit the href in the usual way, only the anchor (i.e. the ID).

At this point I'm not sure if anchor link can be made to work on links with hrefs, but it seems like it needs some logic to prevent editors from turning said links into anchors.

And, obviously, it shouldn't strip IDs from links in exisiting content.

justcaldwell’s picture

Last observation for today: I noticed IDs were striped in my clean install (Drupal 10.2.6) even after I removed anchor_link from my text format/editor. So, I tried to add ID support by adding <a hreflang id> to Source Editing, and Drupal gave me an error telling me to enable anchor link to support IDs!

Error Message: The following attribute(s) are already supported by available plugins and should not be added to the Source Editing "Manually editable HTML tags" field. Instead, enable the following plugins to support these attributes: Anchor link (<a id>).

So this likely also related to #3410100: [10.2 regression] CKEditor 5 breaks when "Source"/Source editing button is added and "Manually editable HTML tags" are specified.

Basically, anchor_link is "adding support" for IDs in <a> tags (IDs aren't allowed by default), but it bails out on processing when it finds an href (see above), so the ID never makes it back into the content. AND, as long as anchor_link is installed/enabled in the project at all, you can't add support for IDs via Source Editing. Yeesh. 🤦🏻‍♂️

sacevedo87’s picture

Following up in regards to the ID stripping issue. Will there be a patch soon? The issue did not seem to exist in the prior version (2.7) but this version is not compatible with CKEditor 5 only with CKEditor 4. Hoping there is a fix soon.

justcaldwell’s picture

Assigned: tagpy » justcaldwell

Taking another crack at this...

rajab natshah’s picture

Facing similar issues with other modules and CKEditor 5 under Drupal 10.3

Do you have Limit allowed HTML tags and correct faulty HTML enabled in the text format?

And Allowed HTML tags

With CKEditor 5 this is a read-only field. The allowed HTML tags and attributes are determined by the CKEditor 5 configuration. Manually removing tags would break enabled functionality, and any manually added tags would be removed by CKEditor 5 on render.

I see <a id target rel class="ck-anchor" href title data-entity-type data-entity-uuid data-entity-substitution> in my Allowed HTML tags

NOTE: I do not use the Limit allowed HTML tags and correct faulty HTML in my case.

justcaldwell’s picture

Hi @rajab natshah. I've been testing with CKEditor 5 and a clean instance of Drupal 10.3.2, with the default text formats Basic HTML and Full HTML. The only change I've made to either is to enable anchor_link (i.e., add the anchor button to the toolbar).

Basic HTML has Limit allowed HTML tags and correct faulty HTML enabled, and has <a id target rel class="ck-anchor" href title data-entity-type data-entity-uuid data-entity-substitution> in Allowed HTML tags (same as yours). Limit allowed HTML is not enabled for Full HTML.

In both cases, IDs survive the initial node creation, but are stripped on subsequent edits.

justcaldwell’s picture

StatusFileSize
new731.63 KB

The Problem

I think I fully understand what's happening now:

1) anchor_link adds support for anchor id attributes (among others) to Allowed HTML tags in anchor_link.ckeditor5.yml.

2) anchor_link_ckeditor5_plugin_info_alter() tells CKEditor's core HTMLSupport to NOT handle any <a> with an id or name attribute, because anchor_link will act on these elements. (See @tagpy's comment #7)

3) BUT the anchor_link plugin was modified with the code below to stop processing if the <a> has an href attribute. (See comment #12)

if (viewElement.hasAttribute('href')) {
    return;
}

If you remove that code, id attributes are no longer stripped, but you get the suboptimal UI that doesn't allow for editing the link in context (see screenshot from #12).

That's where I gave up before.


Proposed Solution

We should remove the code above to re-enable processing of anchors with href attributes - AND - add the linkUI button to the anchor_link balloon toolbar - IF - the anchor also has an href.

Here's a quick demo of what that looks like, with a standard link and a plain anchor (no href) for comparison:

Screencast of linked anchor UI proposed in this issue.

I forked @northernco/ckeditor5-anchor-drupal, and the implementation for this is in the link-id-support branch.


Review and Testing

There won't be an MR or patch here, as all the changes need to occur in the @northernco/ckeditor5-anchor-drupal plugin. I'll submit a pull request there soon, and I'll set this issue to Needs Review when that's done.

If you're in a position to build/compile ckeditor 5 plugins (with npm/yarn/webpack etc.), then feel free to clone my issue fork to your /libraries directory and give it a try.

I may publish a package to make this easier to test.

justcaldwell’s picture

Version: 3.0.0-alpha2 » 3.0.x-dev
Status: Active » Needs review

Here's the pull request.

justcaldwell’s picture

Assigned: justcaldwell » Unassigned
justcaldwell’s picture

A package with the changes described in #18 has been published to https://asset-packagist.org to make it easier to test.

Just follow the OPTION #2 instructions for installing the library on the project page, being sure to replace npm-asset/northernco--ckeditor5-anchor-drupal anywhere you see it with npm-asset/justcaldwell--ckeditor5-anchor-drupal.

rajab natshah’s picture

Nice work, Michael.

Thank you :)
You have full credit for this fix!

rajab natshah’s picture

This is a good way of not attaching a static library to a module
Allowing developers to bring any library or any forked improved library, allows the module to do more.

justcaldwell’s picture

Thanks, Rajab! And agreed -- I really like the asset packagist approach. Your edits to the project page make the process much more reliable.

justcaldwell’s picture

Status: Needs review » Needs work

I discovered a minor (IMO) bug in this implementation. If you follow these steps:

  1. Select some text
  2. Click the anchor button and add the anchor id
  3. Click the link button and add a link href
  4. Click in the text and click the 'unanchor' button

The anchor/id is removed as expected, but subsequent clicks on the link text will continue to show the anchor actions balloon toolbar. It should display the link toolbar, since the href is still present.

The order matters. If you add the link/href before you make it an anchor, everything performs as expected. This only occurs before the content is saved. Editing existing content doesn't trigger the issue, unless you're adding a new linked anchor.

I'll try to dig into this next week.

masipila’s picture

Thank you so much @justcaldwell, you made my day!

Here are some remarks for other community members who have previously used the npm-asset method to manage their dependencies and are using `npm-asset/northernco--ckeditor5-anchor-drupal`

1. composer.json has two references that need to be updated

  • `require` section
  • `extra / installer paths` section

2. Note that the versions of `npm-asset/justcaldwell--ckeditor5-anchor-drupal` are not matching to the version numbers of `npm-asset/northernco--ckeditor5-anchor-drupal`. So make sure to check the latest version from https://www.npmjs.com/package/@justcaldwell/ckeditor5-anchor-drupal

3. If you modify composer.json manually to swap `npm-asset/northernco--ckeditor5-anchor-drupal` to `npm-asset/justcaldwell--ckeditor5-anchor-drupal`, you need to run `composer update drupal/anchor_link --with-all-dependencies` because your composer.lock still has reference to `npm-asset/northernco--ckeditor5-anchor-drupal`. The composer update command will remove the `npm-asset/northernco--ckeditor5-anchor-drupal` and download `npm-asset/justcaldwell--ckeditor5-anchor-drupal`

Cheers,
Markus

justcaldwell’s picture

Glad to help @masipila, and thanks for the notes!

I haven't been able to work on the bug I identified in #25, but I wanted to mention to everyone that I just found out that anchor support is finally coming to core CKEditor.

Per this comment on the github issue, work began a couple weeks ago. You can cast a vote on the UI by adding an emoji reaction to the comment. While you're there add a 👍 to the issue summary to express your support for the feature request.

justcaldwell’s picture

I've had no luck addressing #25. Hopefully this won't be an issue when "anchors" land in a ckeditor 5 core package. Here's my best guess at what is happening should someone else take this up.

The element created by createAnchorElement() is assigned a priority of 5 — the same priority used by CKEditor5's link plugin. Since both plugins operate on <a> elements, I assume the priority must match to prevent creation of additional elements. Indeed, if you change the priority value used in createAnchorElement to e.g. 4, the resulting markup will have two <a> tags when both id and href are added in the editor — one with the id, one with the href.

My guess is since both plugins share the same priority, the first plugin invoked wins when setting a custom property on the element ('link' or 'anchor'). These custom properties play a role in deciding what toolbar/UI to display. So, we get different behavior depending on whether a range of text was made into a link or an anchor first.

justcaldwell’s picture

Just noting that the CK Editor developers have opened an issue to track initial development of native anchor support — called "Bookmarks" — at Bookmarks: part 1 · Issue #17063 · ckeditor/ckeditor5.

Sounds like, at least initially, the UI will only create what they call "point-only-bookmarks", e.g. <a id="xyz"></a>. So the native plugin may not allow for actual links that have an href and link text to be edited using the new Bookmark UI.

brockfanning’s picture

We're suffering from this exact problem, and I'm hoping to get in a quick fix to stem the bleeding of the data loss. Does anyone have any guidance on how to apply justcaldwell's fix (thank you!!) for existing sites that went with "Option 1" on the install instructions for this project?

justcaldwell’s picture

I haven't actually tried this, but I think a "quick-and-dirty" approach would be to just manually replace the northernco version of the plugin in your /libraries folder:

- Visit https://asset-packagist.org/package/npm-asset/justcaldwell--ckeditor5-an..., and click 'Get Zip'
- Decompress the zip file, and rename the resulting 'package' directory to 'ckeditor5-anchor-drupal'
- Replace /libraries/ckeditor5-anchor-drupal with the directory above

This could get overwritten next time you update anchor_link with composer, so you'd probably want to re-install anchor_link using the modified "Option 2" outlined in #21 and #26 above when possible.

justcaldwell’s picture

Status: Needs work » Needs review

Just wanted to note that, when last I checked, ckeditor's upcoming 'Bookmarks' plugin will not support editing/creating bookmarks (anchors) on links.

I also don't see a way to resolve the edge case in #25. I think the changes so far are an improvement on stripping ids from links, so setting back to NR.

justcaldwell’s picture

Confirming that the CKEditor 5 Bookmark plugin does not support bookmarks/anchors on links. It my testing it also appears to strip id attributes from existing links.

If you're on Drupal 10.4 or 11.1, you can test for yourself with CKEditor 5 Bookmark.

rajab natshah’s picture

Status: Needs review » Needs work

Let us have an MR in the module
or a PR in the vardot/ckeditor5-anchor-drupal

mstrelan’s picture

Status: Needs work » Needs review

@rajab natshah I created a PR in vardot repo - https://github.com/Vardot/ckeditor5-anchor-drupal/pull/16

This is a direct copy of the PR from @justcaldwell in #18

rajab natshah’s picture

Thanks, Michael
To review, test, then merge.

rajab natshah’s picture

Title: ID's are stripped from links » Fix IDs being stripped from links when Anchor button is enabled in CKEditor5
Issue summary: View changes
cherrypj’s picture

I'd love to see this work merged soon. I'm not able to apply @justcaldwell's workaround. Thank you!

jastraat’s picture

Just checking, what specifically needs to be done to get https://github.com/Vardot/ckeditor5-anchor-drupal/pull/16 merged into the recommended module package and a new release of the package? There's no code for within the Drupal module, correct?

cherrypj’s picture

StatusFileSize
new38.02 KB

This was hard (for me)!

I generated a patch against vardot/ckeditor5-anchor-drupal with the changes @justcaldwell made, ran the build step, and then generated another patch to get that build file into drupal/anchor_link. There's probably a better way, but I have this working on a pre-production system with plans to go live if QA continues to go well (knock on wood).

Screenshot of a working ck editor 5 anchor link with ID

Here's my PR: https://github.com/USEPA/webcms/pull/1694/files

* The patch with the updates to vardot/ckeditor5-anchor-drupal is ckeditor5-anchor-drupal-justcaldwell-3443785-26.patch
* The patch with the build file generated from the first patch and applied to drupal/anchor_link is anchor-link-justcaldwell-3443785-26.patch

I put the build file at anchor_link/js/anchor-drupal.js and changed the location in anchor_link.libraries.yml.

I don't know if this all makes sense! It was three weeks ago, and I barely remember what I did :(

Definitely can't wait to see a proper MR.

jwilson3’s picture

Re: #33 from @justcaldwell

Confirming that the CKEditor 5 Bookmark plugin does not support bookmarks/anchors on links. It my testing it also appears to strip id attributes from existing links.

Interesting that both modules are affected. The Bookmark feature was added to CKEditor 5 in release 44.0.0 https://ckeditor.com/blog/ckeditor-44-0-0-release-highlights/#bookmarks

What is not clear from this is whether the bug is something that needs fixing in CKEditor itself, or is a generalized Drupal <> CKE5 integration issue.

This underlying issue seems pretty critical (due to data loss!) to demand a warning on both this module's project page, as well as on the ckeditor5_bookmark module page, until the issue is resolved.

jastraat’s picture

https://github.com/Vardot/ckeditor5-anchor-drupal/pull/16 merged, but in using the new release of ckeditor-anchor-drupal id attributes are still being stripped from links if the anchor link plugin is enabled in the WYSIWYG.

It looks like the PR did not include the code mentioned earlier:

if (viewElement.hasAttribute('href')) {
    return;
}
damienmckenna’s picture

Status: Needs review » Needs work
jastraat’s picture

This appears to no longer be an issue after upgrading to Drupal 10.6 which included ckeditor5 47.3.0

scontzen’s picture

Tested this on Drupal 10.6.5 with CKEditor5 47.6.0 and vardot/ckeditor5-anchor-drupal 2.0.4. The issue is still present.

Saved a node with <a href="https://example.com" id="test-link-id">Link with ID</a> in a full_html field. The id is stored correctly in the database, but when editing the node and switching to source view, it's already gone. No save needed - stripped on editor load.

The href bail-out in src/anchorediting.js (lines 132–134) identified in #3443785-12: Fix IDs being stripped from links when Anchor button is enabled in CKEditor5 is still there:

  if ( viewElement.hasAttribute( 'href' ) ) {
      return;
  }

The GitHub PR merged in #3443785-35: Fix IDs being stripped from links when Anchor button is enabled in CKEditor5 added the UI changes from @justcaldwell's fork but not the removal of this bail-out, as @jastraat also noted in #3443785-42: Fix IDs being stripped from links when Anchor button is enabled in CKEditor5. Also, build/anchor-drupal.js in 2.0.4 was not regenerated after the merge, so even the UI changes are not active. Drupal loads the build file, not the sources.

Removed the bail-out locally, rebuilt via npm run dll:build, and the fix works. IDs on links are preserved and the "Edit link" button shows up correctly. The complete fix would need to be submitted as a new PR to vardot/ckeditor5-anchor-drupal.

rajab natshah’s picture

I welcome any new PR to vardot/ckeditor5-anchor-drupal
Thank you for following up with this issue :)

scontzen’s picture

Status: Needs work » Needs review

Submitted the PR: vardot/ckeditor5-anchor-drupal#19

Removes the href bail-out in the upcast converter and regenerates the build file.
Tested on Drupal 10.6.5 with anchor_link 3.0.4.

dpi’s picture

Status: Needs review » Reviewed & tested by the community

PR#19 seems to do the trick!

rajab natshah’s picture

Tested PR #19 on Drupal 11 with CKEditor 5 and the Anchor button in Full HTML: the id is kept on editor load and after saving, and build/anchor-drupal.js regenerates byte for byte from the source change.

Thank you, scontzen and dpi :)

rajab natshah’s picture

Great news, this is out :) PR 19 is merged and released in vardot/ckeditor5-anchor-drupal 2.0.5:
https://github.com/Vardot/ckeditor5-anchor-drupal/releases/tag/2.0.5

Verified with the released package on Drupal 11: the id survives the editor load and the save, and the build file matches what was tested byte for byte.

Huge thanks, scontzen for the fix and dpi for testing it, and thank you Michael and everyone who kept this issue moving :)

rajab natshah’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: +anchor_link-3.0.5

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

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

Maintainers, credit people who helped resolve this issue.

rajab natshah’s picture

Issue summary: View changes

✅ Released anchor_link-3.0.5

Status: Fixed » Closed (fixed)

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