Closed (fixed)
Project:
Entity Embed
Version:
8.x-1.x-dev
Component:
CKEditor integration
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
8 Jun 2019 at 23:52 UTC
Updated:
10 Jul 2019 at 22:09 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
oknateComment #3
oknateComment #4
oknateI have test coverage demonstrating this bug in this patch:
https://www.drupal.org/files/issues/2019-06-09/entity-embed-3060396-11.p...
Comment #5
oknateI suspect the cause is this:
From https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_plugins_widget.h...
I suspect it's that when you delete the property and add it back, it doesn't detect that a property has changed. Not confirmed, just a hunch. But this seems to fix it:
My reasoning is that by keep the attribute set to a blank string, when it's updated, it detects that it has changed.
I didn't want to write a separate test for this issue, so I'm posting an updated patch to #3060396: Add CKEditor Widget JS test coverage.
The test only patch should fail and demonstrate the bug.
Comment #7
wim leersNice find!
I landed #3060396: Add CKEditor Widget JS test coverage. Rebased this patch.
Comment #8
oknateWe might want to just reinitialize the widget. That would probably also fix #3060634: Editing link text in captions via EntityEmbedDialog fails to update link text in CKEditor Widget
Although I'm sure there are advantages with undo/redo not to reinitialize unnecessarily.
Can we just reinitialize the editable caption, when it changes?
Comment #10
oknateWe could also put this in as a stop-gap, since it doesn't seem to hurt anything, until we find a fix that encompasses both bugs (and perhaps others).
Comment #11
wim leersAdding
data.hasCaptioninstead, just like CKEditor'simage2plugin does. This means we don't have to pretend adata-captionattribute should still exist when it doesn't need to exist (which is a negative consequence of the current patch).This also sets us up to add the same checkbox-triggered caption enabling that
EditorImageDialoganddrupalimagecaptionhave in core.Comment #12
wim leersThat was the wrong interdiff, sorry!
Comment #14
wim leersThat came back green, pushed. 🚢
Comment #15
wim leersComment #16
rosinegrean commentedComment #17
wim leers