- π©π°Denmark uv516 Denmark
Just updated to version 7.97.
The issue is still there. - Why?
The patch from #13 π Notice: Undefined index: #file in theme_file_widget_multiple() (line 838 of /public_html/modules/file/file.field.inc Needs work works fine.
Why could it not be implemented in core? - Status changed to Needs work
over 1 year ago 9:14pm 25 April 2023 - πΈπ°Slovakia poker10
@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!
- π©π°Denmark uv516 Denmark
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. - π©π°Denmark uv516 Denmark
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?
- πΈπ°Slovakia poker10
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!
- π©π°Denmark uv516 Denmark
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"
- πΈπ°Slovakia poker10
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:
- install Drupal 7.99
- enable module X,Y,Z
- add a field X
- go to node edit and see the notice/warning/..
- ...
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!
- π©π°Denmark uv516 Denmark
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. - πΈπ°Slovakia poker10
@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!
- π©π°Denmark uv516 Denmark
I don't have the tools or capacity to do more research when I can see such a simple solution to the problem.
In addition to the comments given to me, it would be nice to have an answer to the question I asked in #22
and it would be nice to know what is wrong with patch #7 that fixes the problem?