michaelpapworth / tinymce-knockout-binding

A KnockoutJS custom binding that applies a TinyMCE Editor to the bound HTML element
MIT License
39 stars 19 forks source link

Error occurs when binding if a DOM element is removed before tinyMCE has been initialized #23

Open ben-m-lucas opened 8 years ago

ben-m-lucas commented 8 years ago

I have a page that makes part of its view not visible immediately. This is causing the DOM Node to be disposed. Because this happens so quickly, it sometimes occurs before tinyMCE is initialized. When this happens, the following error occurs:

Unhandled exception at line 41, column 5 in https://localhost:44302/Scripts/lib/wysiwyg.js

0x800a138f - JavaScript runtime error: Unable to get property 'remove' of undefined or null reference.

To continue development, I've been able to resolve this by assigning tinymce to a variable that I then check for null before calling the remove method:

        // To prevent a memory leak, ensure that the underlying element's disposal destroys it's associated editor.
        ko.utils['domNodeDisposal'].addDisposeCallback(element, function () {
            var tinymce = $(element).tinymce();
            if (tinymce) tinymce.remove();
        });

Ultimately, I would expect this would still leak memory whenever it is unable to call the remove method. It would be nice to ensure that the tinymce has finished and then still go ahead and properly dispose it in this case.

jlaustill commented 8 years ago

I'm running into this same issue because I am loading data from ajax. So my observable is initialized with ko.observable(new Post());, then the ajax call loads up real data and populates it a bit later and it's before tinymce is fully initialized. I'm not sure how to get around this, so I'm open to any suggestions :)

jlaustill commented 8 years ago

After stepping through this today, the issue actually seems to be something a bit different. It's not that the binding is getting disposed, it's that it's calling init twice for some reason. The second time through is causing the issues. I stepped through other binding that I've written and this isn't the behavior, so I'm a bit stumped. I'm not sure exactly WHY it's getting called twice...

jlaustill commented 8 years ago

I figured this out I think, here's a fiddle https://jsfiddle.net/LkqTU/29770/

So what's happening is that the update after the binding is happening before the binding is complete. This causes the underlying value to be updated underneath it from what I can tell. This causes it to get confused and trigger the dispose call, and then do updates back and forth between the original value and the new value. stepping through it makes my brain hurt. But I don't think this is an issue with the binding as much as an issue with knockoutjs. If you comment out the bottom line with the setTimeout and use the other one you will see the issue. The one with the timeout gives the bindings enough time to finish then updates and doesn't cause this issue...

jlaustill commented 8 years ago

Ok, so here's the fix. The issue is that the update is being done in the wrong place. comment out this line in the init function $( element ).text( valueAccessor()() );

and add the update to the update so it looks like this, mind you I've edited mine so it's a bit different, but the else statement is the key.

` 'update': function (element, valueAccessor) { var $el = $(element); var tinymce = $el.tinymce(), value = valueAccessor()();

        if (tinymce) {
            if (tinymce.getContent() !== value) {
                tinymce.setContent(value);
            } 
        }
        else {
            $el.val(value);
        }
    }

`

I think you will find that this issue is resolved. This was not an easy one to figure out, in fact I didn't, the guys in the knockoutjs forum helped me with it...

MattFromGer commented 8 years ago

Thank you jlaustill, your fix is working perfectly!