Problem/Motivation

When another CKEditor plugin provides a widget with certain attributes, the drupal-entity handling incorrectly claims it as its own widget during upcast. This results in the other widget's handling no longer working, e.g. doubleclick or right click behaviors.

Proposed resolution

Prevent upcast if the element is not actually a drupal-entity tag.

Remaining tasks

Write patch & review

CommentFileSizeAuthor
#5 2956745-5.patch844 byteswim leers
#2 entity_embed-js_plugin_upcast-2956745-2.patch989 bytesckaotik

Comments

ckaotik created an issue. See original summary.

ckaotik’s picture

ckaotik
Primary language German
Location Berlin
commented
Assigned: ckaotik » Unassigned
Status: Active » Needs review
StatusFileSize
new entity_embed-js_plugin_upcast-2956745-2.patch989 bytes

I've attached a minimal patch, please review.
We might want to include a test for this, dunno.

Status: Needs review » Needs work

The last submitted patch, 2: entity_embed-js_plugin_upcast-2956745-2.patch, failed testing. View results

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

I would need to check if this would break the Lightning distro's CKEditor integration.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
Issue tags: +JavaScript
StatusFileSize
new 2956745-5.patch844 bytes

I cannot reproduce this with just core + entity_embed, but I can see how another CKEditor widget with extremely similar markup (i.e. the same data- attributes) could run into this.

core/modules/ckeditor/js/plugins/drupalimage/plugin.js in its upcast method does something similar:

        widgetDefinition.upcast = function (element, data) {
          if (element.name !== 'img') {
            return;
          }

So I'm +1 to this. But I'd keep this much simpler.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2544020: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin

Note: if #2544020: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin happens before this, this will become obsolete.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Closed (outdated)

Actually, I think it doesn't make sense to keep this open, we should do #2544020: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin anyway, so let's just make a note there to absolutely certainly do this as part of that.