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:
- Install Drupal with standard profile
- Enable Media Library
- Configure Basic HTML text format and add Drupal Media to Active toolbar
- Enable the Embed media filter on Basic HTML text format and save the format
- 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
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3443527-10.2.x-2.patch | 77.88 KB | godotislate |
Issue fork drupal-3443527
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:
- 3443527-setting-empty-url
changes, plain diff MR !7803
Comments
Comment #2
godotislateSince the editor JS is minified, here is a fix-only patch for 10.2.x.
Comment #3
godotislateComment #5
godotislateCreated 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.)
Comment #6
smustgrave commentedFollowing 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.
Comment #9
nod_Committed 74f01af and pushed to 11.x. Thanks!
Comment #11
godotislateSince 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.
Comment #13
longwaveLet's squeeze this in before the patch release.
Committed a708e54 and pushed to 10.2.x. Thanks!