WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 23 months ago

#27224 accepted enhancement

Multisite upload settings are inconsistent

Reported by: danielbachhuber Owned by: jeremyfelt
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: dev-feedback has-patch needs-refresh
Focuses: multisite Cc:

Description

"Site upload space" is indicated in MB whilst "Max upload file size" is indicated in KB.

It would be useful to standardize on MB.

Attachments (3)

27224.diff (1.3 KB) - added by bansod_deven 4 years ago.
Changed the indicators to MB
27224.2.diff (1.3 KB) - added by bogdand 3 years ago.
fixed step incrementation
27224.3.diff (2.9 KB) - added by jeremyfelt 3 years ago.

Download all attachments as: .zip

Change History (13)

#1 @jeremyfelt
4 years ago

  • Keywords good-first-bug added

+1

The KB setting threw me for a second the other day. Especially when larger and larger uploads seem to be more common.

@bansod_deven
4 years ago

Changed the indicators to MB

#2 @bansod_deven
4 years ago

Added a Patch. Changed the max File Upload size 'Indicators on the Form' to MB.
Did not change it in the Database. As the Value in Database(which is in KB) may be used by other Functions.

P.S. : The Max Upload File size needs to be still less than the PHP.ini's Settings for that. For increasing that you may need to change the settings of php.ini only

#3 @samuelsidler
3 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

@bogdand
3 years ago

fixed step incrementation

#4 @bogdand
3 years ago

  • Keywords needs-testing removed
  • Resolution set to worksforme
  • Status changed from new to closed

Hi guys,

Tested the patch and everything seem to work ok. I changed the incrementation from 0.001 to 1

#5 @bogdand
3 years ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

This ticket was mentioned in IRC in #wordpress-dev by bogdand. View the logs.


3 years ago

@jeremyfelt
3 years ago

#7 @jeremyfelt
3 years ago

  • Milestone changed from Future Release to 4.1
  • Owner set to jeremyfelt
  • Status changed from reopened to accepted

Thanks for the patches @banson_deven and @bogdand. These initial changes get us close. I've expanded a bit in 27224.3.diff.

  • When storing fileupload_maxk, force absint() when converting POSTed MB to KB. I don't think we want to shift to float in the database after storing int for so long.
  • Adjust all 3 areas where fileupload_maxk is communicated throughout core. Use ceil() to round up to the nearest integer. The default has been 1500KB, which will calculate to 1.46MB. Using round() would round down to 1MB by default and could cause confusion.
  • There was one place - network settings - where the default site option for fileupload_maxk was 300, even though it is set to 1500 in the network schema. I changed this to be 1500 instead.

Note: This change does mean that a very precise DB setting such as 1234(KB) will now become a max size of 2MB and if re-saved from the settings screen will become a DB setting of 2048(KB).

I don't think this fits right now, but in a future network settings screen, it may be interesting to allow specification of unit in addition to the number— 1GB, 200MB, 20000KB, etc...

And for the sake of history. It appears the decision to use KB was made circa 2005 in or around mu:165

#8 @jeremyfelt
3 years ago

This feels strange. It would be nice to store the value as bytes and not have to do the math in multiple places. For that we'd have to deprecate the fileupload_maxk setting and go with fileupload_max or something.

#9 @jeremyfelt
3 years ago

  • Keywords dev-feedback added; good-first-bug removed
  • Milestone changed from 4.1 to Future Release

I think we need to transition to storing this data as number of bytes rather than kilobytes. This will allow us to be much more flexible. I'm going to push to future release, but would be happy to test and help get into 4.1 if input on approach and a patch can line up.

#10 @chriscct7
23 months ago

  • Keywords needs-refresh added
Note: See TracTickets for help on using tickets.