Support the "name" attribute for backwards compatibility

I can confirm, <a name="anchor></a> gets removed while <a id="anchor></a> doesn't. Unfortunately the patch in #2 did not resolve the issue.

Worked here as expected

A GitHub PR was opened for this at https://github.com/northernco/ckeditor5-anchor-drupal/pull/3. However, I'm not sure it's working. See the PR for details.

I've opened a new Github PR that I think addresses this issue:

https://github.com/northernco/ckeditor5-anchor-drupal/pull/9

Not sure if this implementation is the best way to go, but it seems to be working for me.

I've merged in mkimmet's fix and it seems to be working for me. It discovers anchor links with the name attribute set and converts them to id anchor links, but never saves links with the deprecated attribute. Please let me know if that's not suitable for your use case/if it's not working for you. It's on the 3.0.x branch currently, but drupal.org hasn't generated the composer release yet.

This does not seem to require the patch from @DamienMcKenna, I think because it's not actually saving the attribute.

Thanks Dylan! Could we get a bump to 3.0.0-alpha2 (or some variation) on the list of releases on the project page with the new reference to the registry.npmjs.org ckeditor5-anchor-drupal-0.5.0.tgz? It seems when I reinstall the module I'm still getting 0.4.0 right now. Thanks!

Or maybe a bump to the dev version would be more appropriate, either way dev or alpha would work.

Moving to needs work, as I think think we need a new release on the for 3.0.x-dev (or alpha) which points to ckeditor5-anchor-drupal-0.5.0.tgz.

Thanks for updating the dev release Dylan! I did a test and it looks good to me. Moving back to Needs Review.

alpha2 is not working for us.
The name attribute will be removed as soon as the Anchor Link Module is activated :-(

Hi, @dbielke1986. Unfortunately, the updated library is only in 3.0.x-dev. It hasn't made it to a tagged release yet. Back to NR.

On that note, we've been using dev for a couple months with no issues. I'd vote for RTBC.

@justcaldwell

Unfortunately, I cannot confirm this. I used the dev version of “Development version: 3.0.x-dev updated 20 Dec 2023 at 17:25 UTC” and the name=" tag is removed.

The PR mentioned in #8 is fixing the issue in the library and for new content the name attribute should not be allowed and if you edit the old content in the form it converts them to id attributes. It works correctly! Name attribute should be removed because that is how HTML5 is defining it for anchor links.

The fact that it is not rendered for legacy content which was never touched after update is a different thing!
If you have existing content with name attribute in the text, and you want to avoid big update path processing these texts, that needs to be converted to id attribute. You can do that by creating your own filter plugin to ensure name attribute is copied to an id attribute, like I pasted below.

class FilterAnchorLink extends FilterBase {

  /**
   * {@inheritdoc}
   */
  public function process($text, $langcode) {
    $result = new FilterProcessResult($text);

    $dom = Html::load($text);
    $xpath = new \DOMXPath($dom);
    foreach ($xpath->query('//a') as $node) {
      $href = $node->getAttribute('href');
      if (!empty($href)) {
        // If there is an "href" defined, we should not do anything.
        continue;
      }
      $name = $node->getAttribute('name');
      if (empty($name)) {
        // If there is no deprecated "name" defined, we should not do anything.
        continue;
      }
      $id = $node->getAttribute('id');
      if (!empty($id)) {
        // If there is an "id" defined, we should not do anything.
        continue;
      }
      $node->setAttribute('id', $name);
    }
    $result->setProcessedText(Html::serialize($dom));

    return $result;
  }

}

Thanks for clarifying, @joevagyok!

That the name attribute doesn't survive the upcast is mentioned in a couple comments, so I updated the IS to clarify the proposed solution based on the current implementation.

Also hiding the patch from #2, as it's not necessary given the current approach.

This seems to be working well — setting to RTBC. The library change for this issue actually shipped in the latest tagged release (3.0.0-beta1). So should it actually be marked as Fixed?

I also tweaked the issue summary to further clarify that name attributes are supported, but won't appear in the markup for new or edited content.

If you have existing content with name attribute in the text, and you want to avoid big update path processing these texts, that needs to be converted to id attribute.

Though name is deprecated, browsers tend to support obsolete markup for a very long time. I suspect that browsers will support "named anchors" into the foreseeable future. If that's the case, I'm not sure there's any requirement to update older content to convert names to ids (at least not for the purposes of this issue).

I agree with @justcaldwell comment to move it into RTBC!

How do I get this change into the module? I don't understand what the Github repo in PR #8 is for.

@rob230 - this fix is actually included in the current release (3.0.0-beta1). The maintainers just haven't updated the issue status to Fixed.

Here to eat my words 😬.

Given that there's still a large number of sites that haven't moved from CKEditor 4 (1.x and 2.x branch usage is still at ~18K), there may be a case for supporting an improved upgrade path that allows testing both CKE4 and CKE5 concurrently (see #3484756: Support concurrent use of CKEditor 4 and 5).

This would require allowing the name to continue to appear in the final source code. As I wrote in that issue:

The plugin currently upcasts anchors with name attributes, but does not include the name attribute in the resulting page source. As a result, anchor links that have been edited in CKEditor 5 will no longer be present if the text format is subsequently switched back to one using CKEditor 4.

Even though name is deprecated, it might be preferable to continue full support for now, and remove it in a subsequent release or branch.

As I indicate in the MR, this still requires minimal changes in northernco/ckeditor5-anchor-drupal plugin. I can open a PR there if there's any interest in this.

At this point, we're just using a custom build of the plugin that resolves this and a few other issues in the queue to try to ensure a smooth transition from cke4 -> cke5.

I opened PR 14 on northernco/ckeditor5-anchor-drupal in support of the MR here.

Static patch of the current MR.

Merged into 3.0.x as b680f26.

One change from the merge request: the helper is named _anchor_link_append_to_filter_html_settings(). Drupal 10 core defines a function with the original name in ckeditor5.post_update.php, so the update run would have stopped there with a redeclaration.

Thank you, justcaldwell and everyone on this issue :)

Please do not forget about giving credits for the participants who helped resolve the issue. :)

Give credit to everyone, for sure.
Thanks for reminding me :)

✅ Released anchor_link-3.0.5