- Issue created by @wrd-oaitsd
- 🇺🇸United States justcaldwell Austin, Texas
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.
- 🇺🇸United States justcaldwell Austin, Texas
This would need to be resolved upstream on northernco/ckeditor5-anchor-drupal, but there's nowhere to open an issue there.
- 🇺🇸United States justcaldwell Austin, Texas
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").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 in tact.
- 🇺🇸United States justcaldwell Austin, Texas
I think this is at least major, as we're losing user input/data.
- 🇪🇨Ecuador jwilson3
I would say this is actually more like Critical since it results in data loss.
https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... →
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); } }
- Assigned to tagpy
- First commit to issue fork.
- 🇪🇨Ecuador jwilson3
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?
- 🇺🇸United States justcaldwell Austin, Texas
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 theck-anchor
class. It's not actually stripping any attributes. - 🇺🇸United States justcaldwell Austin, Texas
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 theck-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.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.
- 🇺🇸United States justcaldwell Austin, Texas
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 🐛 [10.2 regression] CKEditor 5 breaks when "Source"/Source editing button is added and "Manually editable HTML tags" are specified Needs review .
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. 🤦🏻♂️ - 🇺🇸United States sacevedo87
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.
- Assigned to justcaldwell
- 🇯🇴Jordan Rajab Natshah Jordan
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 tagsNOTE: I do not use the Limit allowed HTML tags and correct faulty HTML in my case.
- 🇺🇸United States justcaldwell Austin, Texas
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.
- 🇺🇸United States justcaldwell Austin, Texas
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:
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.
- Issue was unassigned.
- Status changed to Needs review
3 months ago 7:01pm 14 August 2024 - 🇺🇸United States justcaldwell Austin, Texas
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 withnpm-asset/justcaldwell--ckeditor5-anchor-drupal
. - 🇯🇴Jordan Rajab Natshah Jordan
Nice work, Michael.
Thank you :)
You have full credit for this fix! - 🇯🇴Jordan Rajab Natshah Jordan
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. - 🇺🇸United States justcaldwell Austin, Texas
Thanks, Rajab! And agreed -- I really like the asset packagist approach. Your edits to the project page make the process much more reliable.
- Status changed to Needs work
3 months ago 6:39pm 16 August 2024 - 🇺🇸United States justcaldwell Austin, Texas
I discovered a minor (IMO) bug in this implementation. If you follow these steps:
- Select some text
- Click the anchor button and add the anchor id
- Click the link button and add a link href
- 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.
- 🇫🇮Finland masipila
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 - 🇺🇸United States justcaldwell Austin, Texas
Glad to help @masiplia, and thanks for the notes!
I haven't been able to work on the bug I identified in #25 🐛 ID's are stripped from links Needs work , 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 comment on the issue to cast a vote on the UI. While you're there add a 👍 to the issue summary to express your support for the feature request.
- 🇺🇸United States justcaldwell Austin, Texas
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.
- 🇺🇸United States justcaldwell Austin, Texas
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. - 🇺🇸United States brockfanning
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?
- 🇺🇸United States justcaldwell Austin, Texas
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 aboveI assume this would 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.
- 🇺🇸United States justcaldwell Austin, Texas
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.