Problem/Motivation
It is possible to configure PHP for no upload size limit, in which case the File widget says "Files must be less than 0 bytes", which is useless.
Old code was assumed there is always a limit, while the docs for file_upload_max_size explicity assume this case to be valid.
Attached patch only shows validator hint when a size limit is in effect.
Steps to reproduce
- Change the php ini settings as follows:
[PHP]
upload_max_filesize = 0;
post_max_size = 0;
- Add a file field to a content type
- Add content of that type and look at the file field
Proposed resolution
Update template_preprocess_file_upload_help() to only display the file size text if the limit value is greater than zero.
Remaining tasks
Address feedback in #7
Write test
review
commit
User interface changes
Before

After

API changes
Data model changes
Release notes snippet
Comments
Comment #2
cilefen commentedComment #3
cilefen commentedThe Drupal coding standard doesn't allow inline "if"s, so please convert to curly braces syntax.
Comment #4
cilefen commentedIt looks to me as if Drupal 8 has this bug. If I am correct, this must be fixed in 8.0.x first then backported to Drupal 7.
See \Drupal\file\Plugin\Field\FieldType\FileItem (and maybe others).
Comment #5
andyf commentedI've re-rolled the patch, thanks.
Comment #6
andyf commentedI noticed the same problem at
admin/config/regional/translate/importand have added a check totemplate_preprocess_file_upload_help()to only display the text if the limit's positive.Comment #7
swentel commentedIf we do a positive check here, we can remove the check in FileItem I guess ..
Comment #8
andyf commentedThanks, I did think about that but just thought it seemed a bit weird to knowingly return a no-op validator from
::getUploadValidator(). I'm happy to reroll tho (:Comment #17
quietone commentedProblem exists in 9.3.x.
I updated the patch, which works, added screenshots.
Todo:
Address feedback in #7
Add a test.
Comment #19
avpadernoThe indentation is wrong by two characters.
The short array syntax should be used.
Comment #20
avpadernoI am not changing the current status, as the patch is still missing tests.
Comment #28
dcam commentedThis test is bad as written because
upload_max_filesizeandpost_max_sizecan't be set withini_set().Comment #29
dcam commentedBig thanks to @acbramley for the test guidance.
Comment #30
smustgrave commentedis there an existing test we could extend or even turn into a non functional test? Since this test isn't doing a submission it's just showing help text seems overkill to do a full bootstrap for it.
Also proposed solution appears to be empty.
Comment #31
dcam commentedI converted the Functional test to a Kernel test and updated the proposed resolution.
Comment #32
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #33
dcam commentedRebased following the move of File module template preprocess functions to a Hook class.
Comment #34
smustgrave commentedThis seems more like a feature request vs an actual bug.
Removing the D7 backport since D7 is EOL.
Ran the test-only
As always thanks @dcam for keeping this as a kernel test!
Manually tested following the IS on a ddev environment.
Can see it's doing as advertised.
Comment #36
catchCommitted/pushed to 11.x, thanks!