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
| Comment | File | Size | Author |
|---|---|---|---|
| #40 | ck5-anchor-link-in-the-wild.jpg | 38.02 KB | cherrypj |
| #18 | linked-anchors.gif | 731.63 KB | justcaldwell |
| #12 | link-with-href-and-id.png | 21.34 KB | justcaldwell |
| #4 | anchor-icon-no-id.png | 21.76 KB | justcaldwell |
| #4 | anchor-link-with-href.png | 20.51 KB | justcaldwell |
Issue fork anchor_link-3443785
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
Comment #2
justcaldwellI 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.
Comment #3
justcaldwellThis would need to be resolved upstream on northernco/ckeditor5-anchor-drupal, but there's nowhere to open an issue there.
Comment #4
justcaldwellHere'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-anchorclass. You can see below that the UI indicates that both are in effect (both the link and anchor button are "active").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.
Comment #5
justcaldwellI think this is at least major, as we're losing user input/data.
Comment #6
jwilson3I 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-...
Comment #7
tagpy commentedHi,
Below lines of code(
$plugin_definition['ckeditor5']['config']['htmlSupport']['disallow'][]) stripping the link and other attributes.Comment #8
tagpy commentedComment #10
jwilson3I'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?
Comment #11
justcaldwellFWIW, I don't think the code referenced in #7 plays a role in this issue. Per the comment in that code...
...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 theck-anchorclass. It's not actually stripping any attributes.Comment #12
justcaldwellOkay, 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):(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 theck-anchorclass, when you place the cursor in the link text, the anchor_text UI popup appears — the link UI popup is no longer available.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.
Comment #13
justcaldwellLast 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. 🤦🏻♂️Comment #14
sacevedo87 commentedFollowing 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.
Comment #15
justcaldwellTaking another crack at this...
Comment #16
rajab natshahFacing 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
I see
<a id target rel class="ck-anchor" href title data-entity-type data-entity-uuid data-entity-substitution>in my Allowed HTML tagsComment #17
justcaldwellHi @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.
Comment #18
justcaldwellThe 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 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:
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.
Comment #19
justcaldwellHere's the pull request.
Comment #20
justcaldwellComment #21
justcaldwellA 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-drupalanywhere you see it withnpm-asset/justcaldwell--ckeditor5-anchor-drupal.Comment #22
rajab natshahNice work, Michael.
Thank you :)
You have full credit for this fix!
Comment #23
rajab natshahThis 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.
Comment #24
justcaldwellThanks, Rajab! And agreed -- I really like the asset packagist approach. Your edits to the project page make the process much more reliable.
Comment #25
justcaldwellI discovered a minor (IMO) bug in this implementation. If you follow these steps:
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.
Comment #26
masipila commentedThank 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
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
Comment #27
justcaldwellGlad 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.
Comment #28
justcaldwellI'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.
Comment #29
justcaldwellJust 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.Comment #30
brockfanning commentedWe'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?
Comment #31
justcaldwellI 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.
Comment #32
justcaldwellJust 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.
Comment #33
justcaldwellConfirming 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.
Comment #34
rajab natshahLet us have an MR in the module
or a PR in the vardot/ckeditor5-anchor-drupal
Comment #35
mstrelan commented@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
Comment #36
rajab natshahThanks, Michael
To review, test, then merge.
Comment #37
rajab natshahComment #38
cherrypj commentedI'd love to see this work merged soon. I'm not able to apply @justcaldwell's workaround. Thank you!
Comment #39
jastraat commentedJust 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?
Comment #40
cherrypj commentedThis 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).
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.
Comment #41
jwilson3Re: #33 from @justcaldwell
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.
Comment #42
jastraat commentedhttps://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:
Comment #43
damienmckennaComment #44
jastraat commentedThis appears to no longer be an issue after upgrading to Drupal 10.6 which included ckeditor5 47.3.0
Comment #45
scontzen commentedTested this on Drupal 10.6.5 with CKEditor5 47.6.0 and
vardot/ckeditor5-anchor-drupal2.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. Theidis 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
hrefbail-out insrc/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: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.jsin 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 tovardot/ckeditor5-anchor-drupal.Comment #46
rajab natshahI welcome any new PR to vardot/ckeditor5-anchor-drupal
Thank you for following up with this issue :)
Comment #47
scontzen commentedSubmitted the PR: vardot/ckeditor5-anchor-drupal#19
Removes the
hrefbail-out in the upcast converter and regenerates the build file.Tested on Drupal 10.6.5 with anchor_link 3.0.4.
Comment #48
dpiPR#19 seems to do the trick!
Comment #49
rajab natshahTested 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 :)
Comment #50
rajab natshahGreat 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 :)
Comment #51
rajab natshahComment #53
rajab natshah✅ Released anchor_link-3.0.5