Fix - Memory leak when ending editing by destroying the scope.

[kenfdev]

This merge request probably solves Issue #364 . Currently when entering the editable mode, the element is $compiled with the same $scope as before and is not being destroyed properly when exiting editable mode. This fix is the same fix made in issue #158 but the merge request hasn't been accepted due to other problems ( i'm not sure why ). I think this memory leak is fatal for pages which use many inputs on the same page (it'll create lots of $$watchers every time entering editable mode). I'm not quite sure about the side effects so it would be very helpful if somebody could point any problems about this change.

Thanks in advance.

Edited It seems this same fix has been removed due to the fix of issue #265 . I'll have to take a look at the issue a little more.

[kenfdev]

I've probably found the proper way to fix both #364 and #265. The problem was that directives like editable-bsdate, editable-radiolist, etc. had their own ng-model set manually and were removing the original ng-model with this.inputEl.removeAttr('ng-model'). Therefore, this commit (7e73a3b9d459cab3960455bede46af7c3f043e71) was not enough because it was being removed inside the render method of the directive.

Below is editableRadiolist for example.

// radiolist
angular.module('xeditable').directive('editableRadiolist', [
  'editableDirectiveFactory',
  'editableNgOptionsParser',
  function(editableDirectiveFactory, editableNgOptionsParser) {
    return editableDirectiveFactory({
      directiveName: 'editableRadiolist',
      inputTpl: '<span></span>',
      render: function() {
        this.parent.render.call(this);
        var parsed = editableNgOptionsParser(this.attrs.eNgOptions);
        var html = '<label ng-repeat="'+parsed.ngRepeat+'">'+
          '<input type="radio" ng-disabled="' + this.attrs.eNgDisabled + '" ng-model="$parent.$data" value="{{'+parsed.locals.valueFn+'}}">'+
          '<span ng-bind="'+parsed.locals.displayFn+'"></span></label>';

        this.inputEl.removeAttr('ng-model');   // the original `ng-model` is being removed here
        this.inputEl.removeAttr('ng-options');
        this.inputEl.html(html);
      },
      autosubmit: function() {
        var self = this;
        self.inputEl.bind('change', function() {
          setTimeout(function() {
            self.scope.$apply(function() {
              self.scope.$form.$submit();
            });
          }, 500);
        });
      }
    });
}]);

Therefore, I've added a $parent to the $datas to directives like editable-bsdate, editable-radiolist, etc. I haven't checked all the possibilities but at least the demo page seems to be working. $parent.$parent definitely isn't clean but was the simplest way I could of thought of for right now.

Can anybody confirm this change?? Or point me to a cleaner path??

Thanks in advance.

[ckosloski]

Do all the tests pass?

[kenfdev]

@ckosloski thank you for the reply. I've been wondering how I can execute the tests. I assume its inside test/e2e/dev-test.html but I think I'm missing something (i.e. ../../dev.html ) and all the tests fail. Are there any instructions on how I can execute the tests???

[ckosloski]

This is what I have done for tests (based on other comments I've seen).

• install grunt globally, if you haven't already: npm install -g grunt-cli
• Execute npm install in the project root
• Execute grunt jade to create the documentation
• Execute grunt build to build the project files
• Execute http-server to start the server.
• Navigate to the following url's for the tests:
  • http://localhost:8080/test/e2e/dev-test.html
  • http://localhost:8080/test/e2e/docs-test.html

[ckosloski]

You will need to squash your commits before they can be merged.

[kenfdev]

@ckosloski I appreciate your kind support. I was able to execute the tests. They had to be updated because the input and select element's attributes changed from $data to $parent.$data and the original tests would fail due to not being able to find the elements. After the update I've confirmed that all the tests have passed. I hope this fix could be merged and get released.

[ckosloski]

@eugef Merge this and then do a new release?

[eugef]

@ckosloski I'll try, unfortunately jsdoc command is not compatible with npm3 which i have on my machine right now, so i will install npm2 and do a release