Reinitializing editor duplicates event listeners on element

FWennerdahl commented 6 years ago

Expected behavior.

When destroying and reinitializing an editor the event listeners set up in ctrl.initListeners should not be added again to the element.

Actual behavior.

The event listeners are added again, causing issues for example when the editor is located within an ng-repeat.

Steps to reproduce the problem.

Open JSFiddle at: https://jsfiddle.net/yax8pLp3/
Focus the editor (make sure that the editor has gotten focus, see #159).
Blur the editor.
Click the "Update model externally" button.

When the model is updated, the ng-repeat will re-render, causing $destroy to be called on the angular-froala directive. Since the editor had been reinitialized, two listeners are now registered for the $destroy event and since the listener sets element to null, the second listener will cause the following error:

TypeError: Cannot read property 'off' of null
    at HTMLTextAreaElement.<anonymous> (angular-froala.js:149)
    at HTMLTextAreaElement.dispatch (jquery-2.1.3.min.js:3)
    at HTMLTextAreaElement.r.handle (jquery-2.1.3.min.js:3)
    at Object.trigger (jquery-2.1.3.min.js:3)
    at n.fn.init.triggerHandler (jquery-2.1.3.min.js:3)
    at Function.qa.fn.on.qa.cleanData (angular.js:1872)
    at n.fn.init.remove (jquery-2.1.3.min.js:3)
    at angular.js:5513
    at Object.push (angular.js:5094)
    at Object.leave (angular.js:5512)

From reading the issue tracker on the Froala WYSIWYG editor, the intended way to bring an editor back to non-initialized state (when using initOnClick) is to reinitialize it, see https://github.com/froala/wysiwyg-editor/issues/1822. As such, it's expected that this will work even if the editor is located in an ng-repeat and the model updates externally.

OS.

macOS 10.12.6

Browser.

Chrome 63.0.3239.132

stefanneculai commented 6 years ago

@FWennerdahl unfortunately, we do not support using the editor inside ng-repeat. The problem is coming from the fact that when the render action happens, the old editor instance is not being destroyed but just removed from the DOM.

FWennerdahl commented 6 years ago

I'm sorry, but this doesn't seem to be accurate. At https://github.com/froala/angular-froala/blob/master/src/angular-froala.js#L150 in the $destroy-handler the editor is destroyed. Placing a breakpoint directly after it in the JSFiddle above shows that the editor has been destroyed and the underlying textarea is shown instead.

The issue is seems rather that event listeners are being added anew to the same element when the editor is destroyed and then reinitialized on blur. Would a viable solution be to prevent the same angular-froala directive instance from binding event listeners more than once?

stefanneculai commented 6 years ago

@FWennerdahl sorry, I was wrong. What it appears to be happening is that the destroy event is triggered twice: once internally by the ng-repeat and then again by the blur event. I think there should be something like this: https://jsfiddle.net/yax8pLp3/2/.

FWennerdahl commented 6 years ago

@stefanneculai There's a difference between the $destroy event (called by AngularJS when the Angular $scope for the directive is destroyed) and the call to initControls.destroy (which is called in the blur event handler and destroys the Froala editor instance but not the directive).

It's not the initControls.destroy that is called twice, rather the angular $destroy handler, since when reinitializing the editor the same $destroy-handler has been registered twice on the element. I think something like directiveInitialized would be more accurate:

ctrl.initListeners = function() {
    // Still want this to be set up again after re-initialization.
    if (ctrl.options.immediateAngularModelUpdate) {
        ctrl.froalaElement.on('keyup', function() {
            scope.$evalAsync(ctrl.updateModelView);
        });
    }

    if (!ctrl.directiveInitialized) {
        element.on('froalaEditor.contentChanged', function() {
            scope.$evalAsync(ctrl.updateModelView);
        });

        element.bind('$destroy', function() {
            element.off(ctrl.listeningEvents.join(" "));
            element.froalaEditor('destroy');
            element = null;
        });

        ctrl.directiveInitialized = true;
    }
};

What do you think?

froala / angular-froala