This project is read-only.

Cropper doesn't store output selection

Nov 10, 2010 at 2:06 AM
Edited Nov 10, 2010 at 2:07 AM

Love the tool.  I act as a coordinator on Cropper.Plugins, have built a couple plugins myself.

I noticed a problem with Cropper and the way it stores the user's output selection.  Cropper stores this in the cropper.config file as the ImageFormat element.  Typical values are bmp, png, jpg. 

But, with the more exotic plugins - SendToXXX, where XXX is Flickr, Paint.NET, Imgur, and so on - the image format (jpg, png, bmp, etc) is distinct from the output selection.  It is possible to send a PNG to, or a bmp, or a jpg.  But regardless of the format of the image file, the output selection is "Send to Imgur".  Yet, this output selection is never stored in the cropper.config file. 

When a user selects "Send to XXX", apparently cropper stores the plugin's Extension property into the Configuration.Current.ImageFormat (which then gets serialized into the cropper.config file).   The plugin's Description property is the distinguishing property, I believe, but it is not stored, or serialized. 

To fix this:  The Description needs to be stored in the Cropper.config file upon Save/Exit, possibly in addition to the Extension (image format) property.  The settings class needs to be extended to also store this value.   Upon startup, loop through the output menu items, and select the menu item for which the Description matches the description stored in cropper.config (and thus Configuration.Current).

I am looking through the Cropper.UI code right now and how to make that happen hasn't popped out at me.

Nov 11, 2010 at 2:56 PM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.
Nov 14, 2010 at 2:29 PM

Hi Terry.  I just posted this to the fixed workitem.  Wanted to also post it here to insure that you saw it.

Terry I saw the fix and thanks for applying it. I have a comment. My view is that the code in changeset 63694 introduces a potential for confusion, because the name of the Settings.ImageFormat property/element no longer reflects its true meaning and purpose. I appreciate the back-compat issue of modifying the name of ImageFormat; existing cropper.config files would no longer "work". Users would get an exception from the XmlSerializer on de-serialization, if they used a new Cropper with ImageFormat removed from Settings class but an old cropper.config file, with ImageFormat still present. So I understand the reluctance to change or remove this property name. This is why, in my proposed change, I *added* a property to Settings ("Description" or "OutputDescription"), and just left the ImageFormat property in there. (Also I wasn't sure if ImageFormat was used for some other purpose elsewhere in the code). It doesn't feel quite perfect to alow ImageFormat to become a vestigial property in the Settings class, but it seems much preferrable to keeping ImageFormat and re-defining its meaning, without, I may add, even updating the xml code doc describing what ImageFormat does. I think using a new property/element with a name that accurately describes what is being stored, eg, OutputDescription, would reduce the potential for confusion.

Also ... I noticed that there was a cut/paste error in Settings.cs. The property OutputPath, has an incorrect XML code doc - it says "the users last used image format." This is the description from ImageFormat, of course, apparently pasted in when adding that property. My proposed change also fixed this minor problem.

I hope you'll reconsider the changes I proposed.