Skip to content

Commit

Permalink
SAK-41442 - Fix ContentItem insertion in Assignments (sakaiproject#6619)
Browse files Browse the repository at this point in the history
This is the minimal fix for the problem, but the whole plugin should be
cleaned up. The problem was with multiple editor instances on the page
and some incidental shared state.

The init function registers the ContentItemDialog with the top level
CKEDITOR object for each editor instance. What was happening in this
particular case was that the dialog was overwritten with the binding to
the second instance, for peer assessment, which is hidden. The editor
reference in the onOk callback was not for the actual editor that
invoked the dialog, but the last one initialized.

The error thrown was for the first selection range (range.checkReadOnly)
from insertHtml/insert in editable.js in CKEditor. With the second
editor hidden, the selection range array is empty, so the first item was
undefined. The shared/overwritten reference would have been more obvious
if the second editor was visible -- the item would have been inserted in
the wrong text area.

A more thorough fix would examine the init vs. onLoad details and make
sure that addIframe is called the appropriate number of times and that
only initialization that must run for each editor instance does. I
believe that would be the addCommand and addButton pieces. The
ContentItemIFrameWindow is also global. It is incidentally safe to use
because only one dialog can be in process at a time, but this should
reside within the instance of the dialog itself.
  • Loading branch information
botimer authored and csev committed Mar 8, 2019
1 parent 871002e commit 5ed6eae
Showing 1 changed file with 1 addition and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ CKEDITOR.plugins.add( 'contentitem',
{
// Dialog onOk callback.
// console.log(ContentItemIFrameWindow.returned_content_item);
var editor = this._.editor;
var items = ContentItemIFrameWindow.returned_content_item;
if ( items ) for(var i=0; i < items.length; i++) {
var item = items[i];
Expand Down

0 comments on commit 5ed6eae

Please sign in to comment.