"Files displayed by default" not respected

Tested the patch from #1 and it seems te work perfectly!

The patch from #1 is simple and sweet, and it works perfectly. Can we get this committed to core?

This looks very reasonable, but needs to go into Drupal 8 first.

It also seems like there should be tests?

I tested patch from #1 and it works for me.

Can you move this to D7 core please? :) Let add it into D7 instead of having system where some things are broken ^^

Let's see if this change breaks anything in the current D8 test suite. Then we'll write a test for this case (I'm not super up on how to write tests so don't want to wait to get this small piece moving along regardless.)

Modified the File Field Display Test.

Still need to get the FileFieldRSSContentTest to work.

Rerolled. Plus, progress on a fix for FileFieldRSSContentTest.

Maybe the original tests were ok.

14: 1520715-14.patch queued for re-testing.

I can't believe the patch in #1 hasn't been committed to D7 core yet. I have to apply it every time I upgrade Drupal. Can we please get it committed?

Can somebody update the issue summary? The patch in #20 still applies.

+++ b/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php
@@ -349,11 +349,16 @@ public static function process($element, FormStateInterface $form_state, $form)
+      if(isset($item['display'])) {
+        $display_value = $item['display'] ? '1' : '';
+      } else {

There's a spacing issue between the 'if' and the opening parenthesis here.

Also, since isset($item['display']) is already being called, the line below can probably be simplified to '#value' => $item['display'].

$item['display'] is a boolean, the display value is '1' if checked.

This fixes a bug that also exists in Drupal 7.

I've updated the issue summary to include a beta phase evaluation.

+++ b/core/modules/file/src/Tests/FileFieldDisplayTest.php
@@ -93,4 +93,30 @@ function testNodeDisplay() {
+    // Create a new node with the uploaded file.
+    $nid = $this->uploadNodeFile($test_file, $field_name, $type_name);
+
+    $this->drupalGet('node/' . $nid . '/edit');
+    $this->assertNoFieldByXPath('//input[@type="checkbox" and @name="' . $field_name . '[0][display]" and @checked="checked"]', NULL, 'Default file display is off.');

It looks like we also need an assertion that the expected display is there if the setting is changed. Just having an assertNoFiledByXPath is prone to creating false positives.

Very good point. Do you like this better?

Since the test changed, could you re-attach that as a separate patch to show current failures?

Test only to show the fail.

I think the patch in #31 addresses the concerns in #30. The patch in #33 is the test only and was expected to fail. Back to RTBC I think.

Yep I do prefer #31 - nice work @lokapujya. Committed d6be512 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

and the D7 versions.

forgot something.

Changed the tests as we did in the D8 patch.

Re-rolled and RTBC.

Edit: to review but...really should be RTBC after this.

Tested. I removed the fix and verified that the test fails without the fix, too.

Committed to 7.x - thanks!

I know this was a couple of years ago but I am having the same issue, was the patch realised into Drupal 7 core?

Yes, this was committed in October 2015 and included in Drupal 7.40 and above.

I tested now and it still works for me in the latest Drupal 7 release. If it isn't working for you, please create a new issue with details on how to reproduce the problem (and link to it from here). Thanks!