Problem/Motivation

An extra attribute is being created by CKEditor, "data-cke-saved-href", and this carries through into the embed code when you edit a caption with a link. The drupalink plugin has code to clear this out, but it doesn't seem to work with within the caption.

<drupal-entity data-align="left" data-caption="&lt;a data-cke-saved-href=&quot;http://www.drupal.org/&quot; href=&quot;http://www.drupal.org/&quot;&gt;changed caption test&lt;/a&gt;" data-embed-button="media" data-entity-embed-display="view_mode:media.full" data-entity-embed-display-settings="" data-entity-type="media" data-entity-uuid="f12a679b-dcc8-414a-a914-bdb1c44f1422" data-langcode="en"></drupal-entity>
  • Expected: the property data-cke-saved-href should not perpetuate back into EntityEmbedDialog nor into the drupal-entity embed code.
  • Actual: extra property is displaying in both the drupal form EntityEmbedDialog and in the wysiwyg text in the element.

here's a video demonstrating the bug.
https://www.drupal.org/files/issues/2019-06-11/extra_attribute.mov

Steps to reproduce:
Edit the href of a link within the caption of an entity embed within the CKEditor.

Proposed resolution

We can use javascript to traverse the DOM and clean this up before saving the attribute:

+              captionDomNode.querySelectorAll('a').forEach((link) => {
+                link.removeAttribute('data-cke-saved-href');
+              });

Remaining tasks

Review and Commit

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

CommentFileSizeAuthor
#25 3061895-24.patch2.92 KBWim Leers
#25 3061895-24-test_only_FAIL.patch1.24 KBWim Leers
#21 3061895-21.patch3.81 KBWim Leers
#21 interdiff.txt1.89 KBWim Leers
#19 3061895-19.patch3.3 KBWim Leers
#19 3061895-19-better_fix-do-not-test.patch1.36 KBWim Leers
#14 3061895-12.patch2.55 KBoknate
#14 3061895-12--TEST-ONLY.patch1.84 KBoknate
#10 3061895--TEST-ONLY.patch1.17 KBoknate
#10 3061895-10.patch1.88 KBoknate
#9 entity-embed-extra-attribute-3061895-9.patch725 bytesoknate
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

oknate created an issue. See original summary.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Issue summary: View changes
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Component: Code » CKEditor integration
Issue tags: +Media Initiative

Reproduced.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Related issues: +#3060634: Editing link text in captions via EntityEmbedDialog fails to update link text in CKEditor Widget

Tagging the original issue as related. It will help us add steps to reproduce.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
FileSize
entity-embed-extra-attribute-3061895-9.patch725 bytes

Here's an initial patch. It seems to do the trick.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
FileSize
3061895-10.patch1.88 KB
3061895--TEST-ONLY.patch1.17 KB
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Active » Needs review

The last submitted patch, 10: 3061895-10.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 10: 3061895--TEST-ONLY.patch, failed testing. View results

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Needs work » Needs review
FileSize
3061895-12--TEST-ONLY.patch1.84 KB
3061895-12.patch2.55 KB

Here's two updated patches, a fix only patch demonstrating the bug and a patch with the test coverage and fix.

The last submitted patch, 14: 3061895-12--TEST-ONLY.patch, failed testing. View results

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Issue summary: View changes
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
FileSize
3061895-19-better_fix-do-not-test.patch1.36 KB
3061895-19.patch3.3 KB

Here is a different solution, that doesn't require hardcoding any of this logic; it relies on CKEditor's Advanced Content Filter (rather than reading the DOM, it now reads the post-ACF filtering representation).

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Needs review » Reviewed & tested by the community

I tested manually and it works, and it's more elegant. Plus it passes the functional javascript test. RTBC.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
FileSize
interdiff.txt1.89 KB
3061895-21.patch3.81 KB

Clean-up.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: Extra attribute is created on a tag when moving between caption in CKEditor vs drupal form » [PP-1] `data-cke-saved-href` attribute is created on `<a>` tag when moving between caption in CKEditor Widget vs Drupal form
Status: Reviewed & tested by the community » Postponed 
+++ b/tests/src/FunctionalJavascript/MediaImageTest.php
@@ -496,13 +496,26 @@ class MediaImageTest extends EntityEmbedTestBase {
+    $this->setCaption('<a href="https://www.drupal.org/project/issues/entity_embed">Gotta leave you all behind and face the truth</a>');

Rather than doing this, we should be using the DrupalLink button but we can't due to #3062034: `href` in caption editable not updating when using DrupalLink. So postponing this on that.

Also improving title.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented

I was just going to suggest you look at that one before committing. The test cases conflict.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented

Now with #3062034: `href` in caption editable not updating when using DrupalLink fixed, the test case here needs to be reworked.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: [PP-1] `data-cke-saved-href` attribute is created on `<a>` tag when moving between caption in CKEditor Widget vs Drupal form » `data-cke-saved-href` attribute is created on `<a>` tag when moving between caption in CKEditor Widget vs Drupal form
Status: Postponed » Reviewed & tested by the community
FileSize
3061895-24-test_only_FAIL.patch1.24 KB
3061895-24.patch2.92 KB

Was already doing exactly that :)

#3062034: `href` in caption editable not updating when using DrupalLink just got committed. Rerolling this.

The last submitted patch, 25: 3061895-24-test_only_FAIL.patch, failed testing. View results

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented

Great, looks perfect.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Reviewed & tested by the community » Fixed

Agreed! 🚢

  • Wim Leers committed 8bcda24 on 8.x-1.x 
    Issue #3061895 by Wim Leers, oknate: `data-cke-saved-href` attribute is...

Status: Fixed » Closed (fixed)

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