Problem/Motivation

In Drupal\file\Plugin\Field\FieldWidget\FileWidget::value(), $input argument as default value. $input is the second argument and the third $form_state is required.

FileWidget::value() is called thanks to #value_callback in FormBuilder::handleInputElement() and $input is always setted. In every cases, not settings $input will cause error since the third argument is required.
Default value for $input is useless and should be removed.

Proposed resolution

Remove default value from $input argument.

CommentFileSizeAuthor
#7 interdiff-2-7.txt796 bytesrosinegrean
#7 default_value_in-2810903-7.patch1.58 KBrosinegrean
#2 default_value_in-2810903-2.patch1.16 KBGoZ
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

GoZ created an issue. See original summary.

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented
Status: Active » Needs review
FileSize
default_value_in-2810903-2.patch1.16 KB

Status: Needs review » Needs work

The last submitted patch, 2: default_value_in-2810903-2.patch, failed testing.

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented
Status: Needs work » Needs review
B31’s picture

B31 CreditAttribution: B31 commented
Status: Needs review » Reviewed & tested by the community
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Title: default value in FileWidget::value() » Second parameter in FileWidget::value() cannot be optional when the third parameter is required
Status: Reviewed & tested by the community » Needs work

Thanks @GoZ, good find!

This exact bug is also found in color.module:

[mandelbrot:drupal | Fri 07:28:15] $ grep -r "\$element, \$input = FALSE" *
core/modules/color/color.module:function color_palette_color_value($element, $input = FALSE, FormStateInterface $form_state) {
core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php:  public static function value($element, $input = FALSE, FormStateInterface $form_state) {

Let's fix that too. I'd also suggest blaming the declaration to see how and when this bug was introduced, and make sure there is not another bug because of it. Thanks!

rosinegrean’s picture

rosinegrean CreditAttribution: rosinegrean at PitechPlus commented
Status: Needs work » Needs review
FileSize
default_value_in-2810903-7.patch1.58 KB
interdiff-2-7.txt796 bytes

Hello,

Looks like this was done here Move FAPI callbacks for file/image widgets in classes so a long time ago.

I haven't seen in this patch something else buggy.

Here is the updated patch.

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Quickfix, -Quick fix

  • catch committed 41f00e0 on 8.3.x 
    Issue #2810903 by prics, GoZ: Second parameter in FileWidget::value()...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

I had a bit of a look further back with git log -S

It was added when file module was originally added to core:

commit ac30bf8a82adf6a5c5b76e18ab46edf7f376ce40
Author: Dries Buytaert <dries@buytaert.net>
Date:   Sat Aug 29 12:52:32 2009 +0000

    - Patch #391330 by quicksketch, drewish et al: file module to core. Yaaa-a-a-y!


+function file_field_widget_value($element, $input = FALSE, $form_state) {

My guess would be the callback originally didn't take $form_state, then it was added on, but it's been that way (albeit moved around over time with different naming) for at least seven years now.

Committed/pushed to 8.3.x, thanks!

  • catch committed 41f00e0 on 8.4.x 
    Issue #2810903 by prics, GoZ: Second parameter in FileWidget::value()...

  • catch committed 41f00e0 on 8.4.x 
    Issue #2810903 by prics, GoZ: Second parameter in FileWidget::value()...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.