Problem/Motivation

This issue originally surfaced on a Drupal 10.2.4 site with the Media Library embed tool enabled in CKEditor and contrib module Editor Advanced Link installed and configured.

When a site user entered embedded an image media entity with the Media Library tool, then set the media to be a link to be opened in a new tab, but left the link URL empty, the page saved and displayed correct, with the media being wrapped in an <a href=""> tag. However, when going back to edit that content, the content becomes empty in the editor, and a JS error shows up in the console:

ckeditor5.js?schaem:468 TypeError: Cannot read properties of null (reading 'getAttribute')
    at or (ckeditor5-dll.js?v=41.2.0:5:420624)
    at er.setAttribute (ckeditor5-dll.js?v=41.2.0:5:412304)
    at e.on.priority (drupalMedia.js?schaem:1:20214)
    at Es.fire (ckeditor5-dll.js?v=41.2.0:5:634006)
    at Es._convertItem (ckeditor5-dll.js?v=41.2.0:5:336068)
    at Es._convertChildren (ckeditor5-dll.js?v=41.2.0:5:336528)
    at Object.convertChildren (ckeditor5-dll.js?v=41.2.0:5:334642)
    at Ms.upcastDispatcher.on.priority (ckeditor5-dll.js?v=41.2.0:5:340669)
    at Es.fire (ckeditor5-dll.js?v=41.2.0:5:634006)
    at Es._convertItem (ckeditor5-dll.js?v=41.2.0:5:336173)
(anonymous) @ ckeditor5.js?schaem:468

Investigating further, if the content is entered similarly, with the link URL being left empty, but not set to open in a new tab, again the content displays correctly on view, but on edit, the media in the editor is removed. In this case, there is no JS error.

In either case, while entering links with empty links is likely user error, it would be ideal for the editor to handle this more gracefully and allow editors to more easily correct the issue.

After #3424644: Update CKEditor 5 to 41.2.0, this issue becomes harder to reproduce in Drupal 11.x or 10.3.x because the update to CKEditor5 41.0.0 prevents the entry of empty links by default, but it is still possible to reproduce with an implementation of hook_ckeditor5_plugin_info_alter() that changes the configuration to allow empty links to be created (and adds the ability to set links to open in a new window).

Steps to reproduce

In Drupal 10.2.x:

  1. Install Drupal with standard profile
  2. Enable Media Library
  3. Configure Basic HTML text format and add Drupal Media to Active toolbar
  4. Enable the Embed media filter on Basic HTML text format and save the format
  5. Enable CKEditor functionality for the Link tool to allow users to set to open links in new tab/window. Either:
    • Install Editor Advanced Link.Configure Basic HTML format and Enable all attributes for Advanced links. OR
    • Create and enable a custom module (e.g. mymodule) that implements hook_ckeditor5_plugin_info_alter():
      /**
       * Implements hook_ckeditor5_plugin_info_alter().
       */
      function mymodule_ckeditor5_ckeditor5_plugin_info_alter(array &$plugin_definitions) {
        assert($plugin_definitions['ckeditor5_link'] instanceof CKEditor5PluginDefinition);
        $link_plugin_definition = $plugin_definitions['ckeditor5_link']
          ->toArray();
        $link_plugin_definition['ckeditor5']['config']['link']['decorators'][] = [
          'mode' => 'manual',
          'label' => t('Open in new window'),
          'attributes' => [
            'target' => '_blank',
          ],
        ];
        $plugin_definitions['ckeditor5_link'] = new CKEditor5PluginDefinition($link_plugin_definition);
      }
      

For Drupal 10.3+, the additional step needed is to enable creating empty links. So in hook_ckeditor5_plugin_info_alter():

/**
 * Implements hook_ckeditor5_plugin_info_alter().
 */
function mymodule_ckeditor5_plugin_info_alter(array &$plugin_definitions) {
  assert($plugin_definitions['ckeditor5_link'] instanceof CKEditor5PluginDefinition);
  $link_plugin_definition = $plugin_definitions['ckeditor5_link']
    ->toArray();
  $link_plugin_definition['ckeditor5']['config']['link']['allowCreatingEmptyLinks'] = TRUE;
  $plugin_definitions['ckeditor5_link'] = new CKEditor5PluginDefinition($link_plugin_definition);
}

Proposed resolution

In core/modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupallinkmedia/drupallinkmediaediting.js, there is code that checks whether the href attribute is missing:

       const linkHref = viewLink.getAttribute('href');

        // Missing the `href` attribute.
        if (!linkHref) {
          return;
        }

Changing the condition to a null check addresses the error:

       const linkHref = viewLink.getAttribute('href');

        // Missing the `href` attribute.
        if (linkHref === null) {
          return;
        }

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3443527-10.2.x-2.patch77.88 KBgodotislate

Issue fork drupal-3443527

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

godotislate created an issue. See original summary.

godotislate’s picture

Status: Active » Needs work
StatusFileSize
new77.88 KB

Since the editor JS is minified, here is a fix-only patch for 10.2.x.

godotislate’s picture

Title: Setting empty URL when making embedded media a link in CKEditor5 causes JS erros » Setting empty URL when making embedded media a link in CKEditor5 causes JS errors

godotislate’s picture

Status: Needs work » Needs review

Created an MR against 11.x.

This is likely only a 10.2 problem, as mentioned before users will be prevented from entering empty hrefs via the link tool in 10.3+.

Putting this in Needs Review because it is a minimal diff (falsy check becomes a not null check), to see whether this qualifies as the type of issue that doesn't need a test. (Test would require a new test module and Functional JS test.)

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Following the steps I am able to reproduce the issue and the MR does resolve it.

I believe this does fall under #2972776: [policy, no patch] Better scoping for bug fix test coverage where the issue is minor enough that tests aren't needed.

  • nod_ committed e71f1e93 on 10.3.x
    Issue #3443527 by godotislate, smustgrave: Setting empty URL when making...

nod_’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 74f01af and pushed to 11.x. Thanks!

  • nod_ committed 74f01afa on 11.x
    Issue #3443527 by godotislate, smustgrave: Setting empty URL when making...
godotislate’s picture

Since this is primarily a 10.2 issue, could this get backported? The patch in #2 should still apply to 10.2.x, or I can make an MR.

  • longwave committed a708e542 on 10.2.x
    Issue #3443527 by godotislate, smustgrave: Setting empty URL when making...
longwave’s picture

Version: 10.3.x-dev » 10.2.x-dev

Let's squeeze this in before the patch release.

Committed a708e54 and pushed to 10.2.x. Thanks!

  • nod_ committed 74f01afa on 11.0.x
    Issue #3443527 by godotislate, smustgrave: Setting empty URL when making...

Status: Fixed » Closed (fixed)

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