The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Currently the context menu isn't shown in CKEditor at all, because of an exception which is caused by the drupalimage plugin.
Steps to reproduce:
1. Open node/add/page
2. Type some text and right click on this text
3. Result: the context menu isn't shown and an error is thrown.
This issue happens whenever the current selection is in a text node. This bug is problematic mainly inside tables, where working with rows/columns without context menu is quite problematic.
The source of the issues is in the isImageWidget() function (~line 237), which is passing a text node to editor.widgets.getByElement() instead of expected CKEDITOR.dom.element.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | ckeditor_contextmenu_broken-2342633-7.patch | 2.05 KB | wim leers |
Comments
Comment #1
wwalc commentedAfter some investigation it looks like the code in the drupalimage plugin is both not needed and invalid.
Why invalid:
1. The exception is thrown when the selection is in a text node
2. In all other situations where the exception isn't thrown, the code is not executed anyway.
3. In order to check if image widget is selected a code similar to the one in afterInit() could be used:
but then, we'd end up with two context menu items, one registered by core image2 plugin and another one registered by drupalimage, which does not make sense as well.
Since drupalimage requires image2 plugin anyway, the context menu item that comes from the image2 plugin is currently used anyway, the one defined by drupalimage is never used, I would simply go ahead with removing the dead/invalid part of the code.
The only reason against KISS approach is if there was some reasoning behind doing it this way that I'm not aware of. Like reusing Drupal.t for a label? If yes, then I guess that seems to have very little sense as the rest of CKEditor is using CKEditor translated strings anyway?
My guess is that this is a leftover after some refactoring, perhaps after integrating it with image2 etc.
Comment #4
wim leersExactly!
You fixed a bug by deleting code. AWESOME :D Can you send more of these patches? :D
Testbot must've been drunk, because a JS-only patch should never fail since testbot doesn't do JS testing.
Comment #5
wim leersManually tested and confirmed both the bug and that the patch in #1 fixes it. Thanks, Wiktor!
Comment #7
wim leersThe test failed thanks to our test coverage to verify Drupal's translation of strings in CKEditor plugins works — I thought that it was a random failure, apparently it wasn't :)
The
Image Propertiesstring no longer exists, so use a different translated string,Edit Link, from thedrupallinkCKEditor plugin.Comment #8
wim leersI only fixed the test, so moving back to RTBC now.
Comment #9
alexpottCommitted c9bd67a and pushed to 8.0.x. Thanks!