Followup of : #2545360: CKEditor 4.5.2 seems to not work

Since that issue, we always create a widget when we start dragging, to be more compatible with changes in latest ckeditor. However if the drop fails we have to do some more cleanup.

Its not critical, as it works fine without this, but its a nice to have.

Attached patch from the other issue.

Currently the patch has issue on IE 9 SCRIPT5007: Unable to get value of the property 'find': object is null or undefined
ckeditor.js: line 1129 character 190

CommentFileSizeAuthor
#10 widget-cleanup-2546712-10.patch2.08 KBgifad
#7 widget-cleanup-2546712-7.patch2.3 KBgifad
#4 widget-cleanup-2546712-4.patch1.11 KBgifad
widget-cleanup-2545360-16.patch1.09 KBnagy.balint

Comments

nagy.balint created an issue. See original summary.

nagy.balint’s picture

nagy.balint commented
Issue summary: View changes
nagy.balint’s picture

nagy.balint commented
Status: Active » Needs work
gifad’s picture

gifad commented
StatusFileSize
new widget-cleanup-2546712-4.patch1.11 KB

Only checked with firefox 40
with widget-cleanup-2545360-16_0.patch, the widget is not properly deleted : you can find a reference at :
CKEDITOR.instances["edit-body-und-0-summary"]._.editable.editor.widgets.instances[0]
with attached patch, CKEDITOR.instances["edit-body-und-0-summary"]._.editable.editor.widgets.instances is empty...

don't know if this is related to IE9 error, but anyway, this is the recommended method, per http://docs.ckeditor.com/#!/api/CKEDITOR.plugins.widget-method-destroy

steps to reproduce :
node/add/article
drop an atom in body "value" text area
check firebug DOM tab of the "summary" text area

gifad’s picture

gifad commented
Status: Needs work » Needs review
nagy.balint’s picture

nagy.balint commented

Yes, i tried that line in comment #18, but it did not make any difference in IE9, so the new patch has the same issue in IE9.

Likely we need a different approach.

gifad’s picture

gifad commented
StatusFileSize
new widget-cleanup-2546712-7.patch2.3 KB

The attached patch does two optimisations :
- The drag process is initiated only for visible ckeditor instance(s)
- The widget refresh (with the associated ajax call) is deferred until the atom is actually dropped

about the IE9 error :
- does it occur when the summary textarea is hidden, visible, or both ?
- does it occur in iframe mode, divarea mode, or both ?
finally, could you provide the call stack to the error ?

nagy.balint’s picture

nagy.balint commented

About your questions:
It's just two separate text area. Not using summary.
It occurs in iframe mode. Did not test the divarea plugin.
Unfortunately ie9 does not provide a call stack. So I would need to set up the dev version of ckeditor and put in a trace call. But I cannot do that right now. Maybe I can set it up next week.

nagy.balint’s picture

nagy.balint commented

Thanks!

patch #4 had the IE 9 error, but patch #7 works without error, so probably one of the additions fixed it.

Can we move widget.data.drag = false; into cleanupDrag?

Otherwise it seems nice.

gifad’s picture

gifad commented
StatusFileSize
new widget-cleanup-2546712-10.patch2.08 KB

Can we move widget.data.drag = false; into cleanupDrag?

Yes we can !
We can even get rid of this ugly widget.data.drag, by dropping the atom in its downcast state, and call refreshAtom() after drop;
now, refreshAtom() is disabled if and only if the atom wrapper has no parent.

Limited test, with firefox 40 only...

  • nagy.balint committed 65c3510 on 7.x-1.x authored by gifad 
    Issue #2546712 by gifad, nagy.balint: ckeditor 4.5.2 compatibility -...
nagy.balint’s picture

nagy.balint commented
Status: Needs review » Fixed

Thanks, Committed.

(Passes my tests)

Status: Fixed » Closed (fixed)

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