Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
editor.module
Priority:
Major
Category:
Bug report
Assigned:
Issue tags:
Reporter:
Created:
22 Aug 2013 at 16:41 UTC
Updated:
29 Jul 2014 at 22:48 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
wim leersThis also applies to the link dialog. This is both sad and fortunate at the same time: sad because it's the same problem, fortunate because the code uses the same API calls, so if it worked there, it'd be more mysterious.
Comment #2
catchThis is major at best.
Comment #3
wim leersApparently there is a magical, undocumented
data-cke-saved-ATTRIBUTENAMEthing you have to do. I found this only by talking to the CKEditor developers. They are going to document this.Comment #4
wim leersCrosspost.
Comment #5
wim leersThe CKEditor developers reviewed the code in #3 and found a problem with it.
The whole reason they are using this
data-cke-saved-ATTRIBUTENAMEthing is because browsers sometimes modify certain attribute values. For example the anchor'shrefand the image'ssrcattributes contain URLs, and browsers may expand relative URLs to full URLs. IE has been known to modify an anchor'snameattribute as well apparently.Please verify that this works when reviewing (citing Wiktor Walc, CKEditor CTO):
(In other words, ensure that the drupallink and drupalimage plugins ignore any
fooattribute for which there is an equivalentdata-cke-saved-fooattribute.)Also: you needn't worry, no
data-cke-saved-SOMETHINGattribute is ever allowed to end up in the resulting HTML.Comment #6
wim leersNote that all this was reviewed by the CKEditor developers for correctness. So I think it should be fine if somebody can just confirm that this indeed fixes the problems :)
Comment #7
Bojhan commentedTested on simplytest.me can confirm it worked
Comment #8
Bojhan commentedComment #9
jessebeach commentedI tested changing an image on a node form. That works now.
I also tested in-place editing. Changing an image through the CKEditor on the front-end results in no change to the image on the entity. After talking with Wim, we believe this is due to the CKEditor not firing a change event when the image attributes are altered; or, it is firing an event and we're just not listening for it in the Edit code.
The code Wim wrote solves the underlying issue in this thread. The problem I uncovered is one of bubbling the change up to the code introduced in this issue.
Comment #10
wim leersThe root cause was that no
changeevent was being triggered. At first I thought CKEditor itself was at fault, but then I realized this might have something to do with our custom link/image plugins.So I started looking at CKEditor's own plugins. I noticed that all of them had two
calls: one before making the change (because the user might have typed changed) and one after making the change (because the plugin has modified some of the HTML)
Our custom plugins were missing the second call to
editor.fire('saveSnapshot');. This must've slipped through the cracks when quicksketch originally wrote this.Note: the reason it worked fine on the back-end form is that there, the end result is saved; we don't listen to CKEditor's change event there, because there is no need to.
Comment #11
wim leersAlso: great find Jesse :)
Comment #12
Bojhan commentedAh, doh - didn't check inline editing. Still has to become part of my mental model to check that too :)
Comment #13
jessebeach commentedCode changes look good. Awesome find Wim! I tested in-place editing and node form editing. Both are working beautifully now.
Comment #14
webchickAwesome, that works!
Committed and pushed to 8.x. Thanks!
Comment #15
wim leers.