U4-4428 - Upload field on members fails on resizing

Created by Sebastiaan Janssen 13 Mar 2014, 17:10:06 Updated by Shannon Deminick 17 Mar 2014, 07:38:47

In Umbraco.Core.IO.UmbracoMediaFile the call to DoResize was changed from: DoResize(GetDimensions().Width, GetDimensions().Height, maxWidthHeight, fileNameAddition); To: DoResize(-1, -1, maxWidthHeight, fileNameAddition);

Which makes trying to set a file to the upload field fail completely with the error below. @Per.Ploug it's been changed in your commit e825c089015f7c37e88173231df32665515e5867 but the commit is so incredibly huge that I can't figure out why this was done. Can we revert this?

====

Parameter is not valid.

Description: An unhandled exception occurred during the execution of the current web request. Please review the stack trace for more information about the error and where it originated in the code.

Exception Details: System.ArgumentException: Parameter is not valid.

Source Error:

Line 40: member.getProperty("bio").Value = tb_bio.Text; Line 41: var file = fu_avatar.PostedFile; Line 42: member.getProperty("avatar").Value = file; Line 43:
Line 44: //Assign a group, get the group by name, and assign its Id

Source File: d:\Level2MasterClassMarch2014\Attendee Site\Training.Site\usercontrols\RegisterMemberForm.ascx.cs Line: 42

Stack Trace:

[ArgumentException: Parameter is not valid.] System.Drawing.Bitmap..ctor(Int32 width, Int32 height, PixelFormat format) +1140345 System.Drawing.Bitmap..ctor(Int32 width, Int32 height) +13 Umbraco.Core.Media.ImageHelper.GenerateThumbnail(Image image, Int32 maxWidthHeight, Int32 fixedWidth, Int32 fixedHeight, String thumbnailFileName, String extension, IFileSystem fs) +280 Umbraco.Core.IO.UmbracoMediaFile.DoResize(Int32 width, Int32 height, Int32 maxWidthHeight, String fileNameAddition) +212 Umbraco.Core.IO.UmbracoMediaFile.Resize(Int32 maxWidthHeight, String fileNameAddition) +39 umbraco.cms.businesslogic.datatype.FileHandlerData.set_Value(Object value) +943 umbraco.cms.businesslogic.property.Property.set_Value(Object value) +11 Training.Site.usercontrols.RegisterMemberForm.SaveMember(Object sender, EventArgs e) in d:\Level2MasterClassMarch2014\Attendee Site\Training.Site\usercontrols\RegisterMemberForm.ascx.cs:42 System.Web.UI.WebControls.Button.OnClick(EventArgs e) +9615678 System.Web.UI.WebControls.Button.RaisePostBackEvent(String eventArgument) +103 System.Web.UI.WebControls.Button.System.Web.UI.IPostBackEventHandler.RaisePostBackEvent(String eventArgument) +10 System.Web.UI.Page.RaisePostBackEvent(IPostBackEventHandler sourceControl, String eventArgument) +13 System.Web.UI.Page.RaisePostBackEvent(NameValueCollection postData) +35 System.Web.UI.Page.ProcessRequestMain(Boolean includeStagesBeforeAsyncPoint, Boolean includeStagesAfterAsyncPoint) +1724

Comments

Shannon Deminick 17 Mar 2014, 05:05:24

@sebastiaan the -1, -1 was in this commit, not the one you've referenced:

394915fa05dfcf21e90910b4b682c85d60bd8376

And the reason is because that particular call is inside the method:

public string Resize(int maxWidthHeight, string fileNameAddition)

Which does not set specific dimensions, thus -1 is a default (ignored case). Specifying anything but -1, will now ignore the maxWidthHeight so you 'fix' in rev: f6267381742e31c494d6b26e539d9260198e856f is incorrect.

I'll need to review this error and what you've changed.


Shannon Deminick 17 Mar 2014, 05:21:14

The -1, -1 must remain there otherwise the image resizing will not work properly for any thumbnails.

In the above stack trace, here is what is happening:

  • FileHandlerData.Value = somepostfileobject;
  • That calls into: UmbracoMediaFile.Resize(Int32 maxWidthHeight, String fileNameAddition) which specifies ONLY a maxWidthHeight
  • That calls into: UmbracoMediaFile.DoResize(Int32 width, Int32 height, Int32 maxWidthHeight, String fileNameAddition) which is a generic private method which checks if the width is -1, if it is it will then use the maxWidthHeight to determine the thumbnail size whilst maintaining an aspect ratio
  • That then calls: ImageHelper.GenerateThumbnail(Image image, Int32 maxWidthHeight, Int32 fixedWidth, Int32 fixedHeight, String thumbnailFileName, String extension, IFileSystem fs) which as you can see is only getting a maxWidthHeight value passed in.

Somewhere in there is your problem, it is not the -1, -1 which must remain. I realize there's cleaner ways to do this but the method is private for a reason, we would expect people to pass in -1, -1 as parameters and I didn't feel like making overloaded methods for this at the time.

If you can please step into this code or give me the code to replicate that'd be appreciate.


Sebastiaan Janssen 17 Mar 2014, 06:29:51

I think that the problem then is this bit of code:

var thumbnail = maxWidthHeight == -1 ? ImageHelper.GenerateThumbnail(image, maxWidthHeight, fileNameThumb, Extension, _fs) : ImageHelper.GenerateThumbnail(image, width, height, fileNameThumb, Extension, _fs);

If maxWidthHeight is -1 then use that value to resize, great idea! So this if needs to be inverted. Agreed?


Shannon Deminick 17 Mar 2014, 07:38:19

Indeed, good spot


Priority: Major

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.0.4

Due in version: 7.1.0

Sprint:

Story Points:

Cycle: