pqina / filepond-plugin-image-preview

🖼 Show a preview for images dropped on FilePond
https://pqina.nl/filepond
MIT License
46 stars 26 forks source link

Missing null ref check #59

Closed PinkDuck closed 3 years ago

PinkDuck commented 3 years ago

TypeError: s is undefined at DID_UPDATE_ITEM_METADATA@filepond-plugin-image-preview.min.js:9:19658

Where s is an expected crop object, but if there is no crop object the following conditional test fails with null ref.

rikschennink commented 3 years ago

The issue description lacks detail, please provide a concise test case on codesandbox.io to reproduce and leave a comment, will close till updated.

PinkDuck commented 3 years ago

Within the didUpdateItemMetadata function beginning at line 3142 there is a regex test for crop/markup/resize action change then on line 3169 a use of crop variable without null ref check.

A call to setMetadata("resize") on an FilePond image rendered with preview plugin results in the error above, where object supplied is {"mode":"contain","upscale":false,"size":{"width:732,"height:670}}.

The object returned by file.getMetadata() after image drop (and after upload transform) is:

{
  "resize": {
    "mode": "contain",
    "upscale": false,
    "size": {
      "width": 732,
      "height": 670
    }
  },
  "size": {
    "width": 1478,
    "height": 1108
  },
  "color": null,
  "output": {
    "type": null,
    "quality": null,
    "client": [
      "crop",
      "resize",
      "filter",
      "markup",
      "output"
    ]
  }
}

Evidently no crop property key at the root, so probably a relative referencing issue in the change handler.

rikschennink commented 3 years ago

Alright, can you try 4.6.9, just published it.

PinkDuck commented 3 years ago

Thanks, that resolved the reported glitch.

However, revealed another of similar type for var aspectRatio = crop.aspectRatio || imageAspectRatio; on line 2663.

Plus a fair batch at 2540 onwards.

rikschennink commented 3 years ago

Have you registered the crop plugin I wonder?

PinkDuck commented 3 years ago

I haven't done so as I don't have need to make use of cropping and didn't think there was a dependency on it.

I'll try it out to see… ah, fixative impact seen.

Might wish to revise the documentation to mention this (if not wishing to make preview independent of crop).

rikschennink commented 3 years ago

@PinkDuck okay, glad to hear that fixes it. I will probably fix the plugin later as it should work without the crop plugin.