Support from Acquia helps fund testing for Drupal Acquia logo

Comments

marcingy’s picture

Priority: Major » Normal

This is not major

Christopher Riley’s picture

Although it is not major it is a irritation and should be taken care of. Just like the silly Weight selector that I just noticed is now showing up below box with the images that have been uploaded.

Actually I just added another check and the box went away I changed the line from

if ($widget['#file'] == FALSE) {

to

if (isset($widget['#file']) && $widget['#file'] == FALSE) {

and the Notice vanishes as well as the stupid little weight box.

Only question is does this break anything? How I hate working on core.

almc’s picture

Version: 7.17 » 7.23

I've hit this issue when started using the module Multiupload Filefield Widget on D7.23, getting the following messages when add files to a node using the module feature:

Notice: Undefined index: #file in theme_file_widget_multiple() (line 846 of ...\modules\file\file.field.inc).
Notice: Undefined index: fid in file_field_widget_submit() (line 765 of ...\modules\file\file.field.inc).

Posting this issue here as it seems that it could be related to the core.

creeksideplayers’s picture

I'm seeing the same problem when I add a content type that has multiple files: Notice: Undefined index: #file in theme_file_widget_multiple() (line 846 of public_html/modules/file/file.field.inc).

Uv516’s picture

Version: 7.23 » 7.34
Issue summary: View changes

I've tried #2, but it isn't a good idea. it makes other troubles.
I have the same errors as #3 in Drupal 7.34
The error message does not appear to adversely affect the files are saved correctly.

othermachines’s picture

Adding a couple of related issues where this notice cropped up and was fixed in contrib.

tucho’s picture

Version: 7.34 » 7.x-dev
Status: Active » Needs review
FileSize
988 bytes

In current version (Drupal 7.39) the lines where the bug resides have changed.

When you enter in the node creation form you see this error:

Notice: Undefined index: #file in theme_file_widget_multiple() (line 861 of modules/file/file.field.inc).

And after uploading a file, you will see:

Notice: Undefined index: fid in file_field_widget_submit() (line 780 of modules/file/file.field.inc).

I attach a patch that should resolve this issue.

suit4’s picture

Patch #7 works for me, thanks.

artemboiko’s picture

Patch #7 works for me too, thanks.

vensires’s picture

Status: Needs review » Reviewed & tested by the community

Patch #7 works for me too. Also checked it's working for Drupal 7.50. Setting as RTBC.

stefan.r’s picture

Assigned: Unassigned » David_Rothstein
David_Rothstein’s picture

Assigned: David_Rothstein » Unassigned
Status: Reviewed & tested by the community » Needs review

This might turn out to be worth doing, but what are the steps to reproduce the bug?

I tried using https://www.drupal.org/project/multiupload_filefield_widget (as mentioned in #3) but with the latest version of the module and Drupal core I couldn't reproduce the problem. And none of the other comments here have details on how to reproduce it.

From the code it seems it would happen if a module inserted an element into the widget (next to the field items) and that element didn't have file_managed_file_process() run on it. Which I think is probably legit, but it's hard to be sure without an actual example to test.

-    if ($widget['#file'] == FALSE) {
+    if (!isset($widget['#file']) || $widget['#file'] == FALSE) {

This could just be simplified to if (empty($widget['#file'])), right?

tucho’s picture

Hi @david_rothstein

Sorry the delay, but I was diving into the code of our project that hit this problem, trying to reproduce it.

In our case, there was a faulty implementation of a hook_field_widget_form_alter hook that was adding to the form a field, which lacks of the #file index.

So it was throwing a warning when the code of the theme_file_widget_multiple function check it's value. The same happends on the file_field_widget_submit, the malformed field does not sumbit valid values, so the fid index does not exists on submitted_value array.

About the code:

if (!isset($widget['#file']) || $widget['#file'] == FALSE) {

I think you are right, it may be replaced with an if (empty($widget['#file'])).

Looking at the file module code (line 368 of modules/file/file.module), #file may be a file entity or an empty array (when it was unable to load the file entity) or FALSE.

I attach a re rolled patch with this change and an interdiff between both patches.

RAFA3L’s picture

Thanks tucho, I have the same problem and found the same solution, now I know I'm not alone, I hope some day this patch will be commited.

Uv516’s picture

I have just updated til Drupal 7.79 but the issue is not fixed. - After 4 years!!!

Uv516’s picture

Now we have reached version 7.93 and the error is there again.
Can't someone who writes core take care of this?

Uv516’s picture

Just updated to version 7.97.
The issue is still there. - Why?
The patch from #13 works fine.
Why could it not be implemented in core?

poker10’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs tests, +Needs steps to reproduce

@Uv516 I think that the main problem with this issue is a fact, that there is no issue summary and practically no steps to reproduce (except the #13). If someone can write steps to reproduce using the D7 core and probably other contrib modules, then we can take a look at that.

I do not understand whom faulty implementation it was in this case?

In our case, there was a faulty implementation of a hook_field_widget_form_alter hook that was adding to the form a field, which lacks of the #file index.

Other than that, when we will have steps to reproduce, it is good to include a test for the bug being fixed, so that the bug does not occur in the future again. Therefore I am adding relevant tags.

Thanks!

Uv516’s picture

FileSize
40.53 KB

I have other modules where I have used the same changes as #13.

If you check $variable['test'] == FALSE, the $variable['test'] must exists, but
if you check !empty($variable['test']), the $variable['test'] may og may not exists.

In my case I have tested

foreach ($widgets as $key => &$widget) {
    if (empty($widget['#file'])) {

I get two $key: [0] and [1].
The [0]-key is the node itself. The $widgets['#file'] IS there.
The [1]-key is about one field in the node. The field is a file field and the $widgets['#file'] is NOT there.
See the screendump code ("file-error.txt")
I wonder if no other fields is mentioned in this - or why the only one field "field_ny_bilag" is mentioned in this.
When I yse "!empty" there is no error.

Uv516’s picture

Here we are again.
Just updateted to Drupal 7.99 and the error comes again.

Is it a problem to follow #13 in next update?

poker10’s picture

I have added tags to the issue in my previous comment, which should be the next steps if we want to move this forward. Issue summary is empty - it needs to be updated according to the template and together with the steps to reproduce the problem. Then we need to create a test for this. Then we can review the issue and consider if we can commit this. Thanks!

Uv516’s picture

I wonder how it must be so difficult to solve a problem that seems so small.

What is the difference between and what significance do these two options have:
$widget[#file] == FALSE
or
empty($widget[#file])

Using $widget[#file] == FALSE requires $widget[#file] to exist to avoid a warning.

If you use empty($widget[#file]), the result will apparently be the same, even if $widget[#file] does not actually exist.

See attached documentation: "Uv516 #22 documentation for error.pdf"

poker10’s picture

It is not difficult to solve this. But all core issues need to meet some requirements to be able to be committed. For example there are core gates all issues should pass before commit, then there is an issue etiquette and so on.

So for example we cannot commit an issue without the summary, because that field serves the purpose that all contributors and committers will have a summary at hand and does not need to read all comments to see what is the purpose of the issue (what is a problem, what is the proposed solution, ...). It is also important in case we would need to return back here in case the commit will cause some side effects, etc. All these rules are in place to guarantee the quality of all contributions to the Drupal core.

I think the first step to update issue summary is relatively easy. Then there are steps to reproduce - thanks for the PDF documentation and explanation. To create a test for this patch it would help, if someone can write steps to reproduce in a way similar to this:

  1. install Drupal 7.99
  2. enable module X,Y,Z
  3. add a field X
  4. go to node edit and see the notice/warning/..
  5. ...

Then we will be able to create a test and move it forward. Currently I was unable to simulate the problem, so probably there is some custom code or contrib module needed to get this notice/warning.

Thanks!

Uv516’s picture

I've just updated to Drupal version 7.100.
To my dismay, I find that the same error that has now existed for 5 years still exists despite the fact that the fix is so simple.

poker10’s picture

@Uv516 8 years ago, David_Rothstein, previous D7 maintainer, asked for detailed steps to reproduce (in #12). You provided some info in #22, but the issue summary is still empty and I also asked for some additional information how to simulate this problem in #23. So my additional tags for this issue from #18 are still valid and I have explained them a bit more in #23. Sorry, we cannot proceed with this issue in the current state. There are policies in place we need to accept, otherwise the Drupal core code could became a mess. You can help to move this issue forward by updating the issue summary, then providing detailed steps to reproduce (see my comment #23) and then we can try to add a test for this. Once we have these things, we can evaluate and move forward. Thanks!