Attachment size issues - minor but annoying #3772

Closed
sbulen opened this Issue Jan 2, 2017 · 12 comments

Projects

None yet

4 participants

@sbulen
Contributor
sbulen commented Jan 2, 2017 edited

Two issues, minor but annoying. Trying to attach photos to a message, I see:

(1) What is displayed as a maximum is incorrect. If an image is rejected, the message displayed by SMF says I have surpassed a 2K limit, not a 2M limit. My setting in Attachment Settings for max attachment is 2000 KB.

(2) Attachment size is calculated differently vs displayed. This is just the normal 1000 vs 1024 issue. I have a 1.9M file that is rejected on a 2M limit. The file size is actually 1.91 MB (2,009,563 bytes). SMF says it is a 1.9MB file...

So... SMF says I have a 2K limit when I have a 2M limit, and shows me it is rejecting a 1.9MB file when it tells me it is rejecting this limit. It should be displaying a 2M limit, and that it is rejecting a 2M+ file...

I'd also look at avatars, I'm assuming they behave similarly.

@d3vcho
Contributor
d3vcho commented Jan 2, 2017
  1. Nice catch!

  2. Yeah, seems like we're working with MiB (1024) instead of MB (1000), but we're confusing them both... Possible solutions are to change all limits from 1024 to 1000, or to change all places were MB is used to MiB...

@d3vcho d3vcho added a commit to d3vcho/SMF2.1 that referenced this issue Jan 2, 2017
@d3vcho d3vcho #3772
Signed-off by: Frankie "d3vcho" <d3vcho@gmail.com>
464a2a0
@sbulen
Contributor
sbulen commented Jan 2, 2017

Screenshots attached... Forgot to attach them earlier....
attach_rejected_size
attach_size

@Antes
Member
Antes commented Jan 2, 2017

Try to increase max attachment size per post as well, I think that's a bug related to "0" value.

@live627 live627 added a commit that referenced this issue Jan 3, 2017
@d3vcho @live627 d3vcho + live627 #3772
Signed-off by: Frankie "d3vcho" <d3vcho@gmail.com>
d3cb5b2
@sbulen
Contributor
sbulen commented Jan 7, 2017

D3vcho's fix addressed the first issue above, where it stated something flat out inaccurate. Thanks!

The 2nd part is still outstanding. It now says "max attachment size is 2000 KB", and still displays 1.9MB as the file size as pictured above.

Max attachment size doesn't change this behavior.

It would be more friendly & understandable if it displayed the attachment size in K also.

FYI, it's not using true K (1024), it's using 1000's. This file's size is 2,009,563 bytes, or 1.91 MB, or 1962 K.

To be most clear it should display 2009 K as this pic's size, which clearly exceeds the 2000 K limit set by the admin.

@sbulen
Contributor
sbulen commented Jan 7, 2017

As an FYI, following up on my note above, avatars behave completely differently & don't appear to be an issue. (If the file is too big, it shrinks it!)

@sbulen
Contributor
sbulen commented Jan 8, 2017

The problem code here is in c.prototype.filesize in dropzone.min.js. Its job is to build the string
"1.9 MB" used above. It always assumes pure powers of 2 when displaying files in MB/GB/TB. SMF does attachment math relative to KB everywhere I could find, hence the inconsistency visible in the screenshot above.

Easily fixed. Just gotta go figure out how to re-minimize it when done...

@tinoest
Contributor
tinoest commented Jan 8, 2017

I use this quite a lot http://www.minifier.org/ if that's any help.

@sbulen
Contributor
sbulen commented Jan 8, 2017

Thanks!

I worry about checking in pre-minimized code.

Appears to lose the purpose of using git to track the changes.

@sbulen
Contributor
sbulen commented Jan 9, 2017

Testing deeper, there are some other issues here. I don't think the simple math checks are working properly - file size & total size. They are calc'd in different places, and when they are out of whack, strange things happen.

E.g., setting the max attachments slightly higher than your uploaded file(s), you get no more error messages but the attachments won't upload. They're stuck in limbo. Needs more work.

@sbulen
Contributor
sbulen commented Jan 9, 2017

dropzone.js has some initialization passed to it in smf_fileUpload.js, including a setting for filebaseSize.
Setting this to 1000 instead of 1024 brings all behavior consistent & crisp throughout the upload process.
Bonus: No need to putz with the minimized code in dropzone.min.js... This was anticipated & configurable.

There are a couple of other places here & there on the SMF side that still display 1962K vs 2009K for the test image I've been using. I'll be looking at those next...

@sbulen
Contributor
sbulen commented Jan 10, 2017

After all that... filebaseSize was a red herring... Pretty much everywhere we use real K (1024). Left that alone; it really should be 1024.

The REAL issues: There were errors in the calculation for the file size check. Made further confusing by issues populating figures in the error messages (wrong field used). All fixed & test out OK so far locally. I'll test it a bit more & submit a PR.

There are other issues here with dropzone - several formatting issues upon successful attachment & I think that when errors are encountered, temp files are left in the attachment directory. I also suspect issues with attachment file maintenance. I will do more testing & write this up later.

Sorry for the lengthy blog... Learning as I go...

@sbulen
Contributor
sbulen commented Jan 10, 2017 edited

Before & after attached. PR coming.

BEFORE:
before-bad

AFTER:
after-good

@sbulen sbulen added a commit to sbulen/SMF2.1 that referenced this issue Jan 10, 2017
@sbulen sbulen Attachment size issues - minor but annoying #3772
Signed by Shawn Bulen, bulens@pacbell.net
15d49c4
@live627 live627 added a commit that referenced this issue Jan 11, 2017
@sbulen @live627 sbulen + live627 Attachment size issues - minor but annoying #3772
Signed by Shawn Bulen, bulens@pacbell.net
0406939
@sbulen sbulen closed this Jan 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment