Croppie icon indicating copy to clipboard operation
Croppie copied to clipboard

destroy() method throws error upon second call. this.elements undefined

Open justsilencia opened this issue 5 years ago • 7 comments

PLEASE BE AWARE Issue #411 does not solve this issue. My version already contains patch for #411 .

After initializing Croppie, then clicking cancel (calling .destroy()), all seems well and no errors in console. However, if you create a second Croppie instance after this, then call .destroy() again, you get:

TypeError: Cannot read property 'boundary' of undefined.

It also errors out if you try saving the crop rather than clicking cancel.

I’ve been pouring through the croppie file, and the error is perplexing. Error occurs on _destroy when it tries to access the self.elements object:

function _destroy() { var self = this; self.element.removeChild(self.elements.boundary); removeClass(self.element, 'croppie-container'); if (self.options.enableZoom) { self.element.removeChild(self.elements.zoomerWrap); } delete self.elements; }

For some reason self.elements is undefined, hence the type error. Although self.elements is deleted on destroy, it is re-created upon creation of new instance, so it shouldn’t evaluate to undefined. I verified the re-creation of self.elements in the console, which re-populates the object just fine:

    boundary = self.elements.boundary = document.createElement('div');
    viewport = self.elements.viewport = document.createElement('div');
    console.log(self.elements);

Expected Behavior

Call the destroy method then re-instantiate the croppie object multiple times without issue.

Actual Behavior

After the first call to destroy, this.elements loses its state somewhere along the line, leading to a failure of the croppie instance.

Steps to Reproduce the Problem

  1. Instantiate the croppie object.
  2. Call destroy method
  3. Re-instantiate the croppie object (without refreshing the browser).
  4. Call destroy again, or try cropping the image.

justsilencia avatar Mar 06 '19 18:03 justsilencia

Hello! I was experiencing this issue and just wanted to share what fixed it. I was binding the 'destroy' call to a cancel button as an event listener. When I canceled, I was not removing that event listener correctly. So it was firing twice and causing the error.

ghost avatar Apr 30 '19 14:04 ghost

@floifyed AAAhhhh, this is probably the answer! Total deficiency on my part for not catching that. For my solution, I ended up just toggling the visibility of a single croppie instance. So cancel doesn't destroy, it just hides the croppie view. It works great but I'll be switching this asap now that you pointed this out. Thanks for sharing!

justsilencia avatar May 01 '19 13:05 justsilencia

To follow up with this, it seems the evenListener on the zoomerElement is not being destroyed when destroy() is called. I handled this by replacing the zoom slider element after calling destroy():

  var oldCrop = document.getElementById("crop-slider")
    var newCrop = oldCrop.cloneNode(true)
    oldCrop.parentNode.replaceChild(newCrop, oldCrop)

ojack avatar Sep 24 '20 09:09 ojack

To follow up with this, it seems the evenListener on the zoomerElement is not being destroyed when destroy() is called. I handled this by replacing the zoom slider element after calling destroy():

  var oldCrop = document.getElementById("crop-slider")
    var newCrop = oldCrop.cloneNode(true)
    oldCrop.parentNode.replaceChild(newCrop, oldCrop)

Where did you set the 'crop-slider' id? If I try your code, the program doesn't find any element with this id.

Euxiniar avatar Sep 30 '20 06:09 Euxiniar

Oh oops, I realized that I had modified my local version of Croppie in order to create a custom zoomer element, so this code is irrelevant.....sorry!

ojack avatar Oct 01 '20 10:10 ojack

Oh oops, I realized that I had modified my local version of Croppie in order to create a custom zoomer element, so this code is irrelevant.....sorry!

Ok no problem. I did something a bit dirtier but at least it works haha

Euxiniar avatar Oct 01 '20 11:10 Euxiniar

Any news on that?

guilhermebentomarques avatar Sep 30 '21 12:09 guilhermebentomarques