Remove unused #theme "file_widget"

Wondering if 'widget' should be in the name, because 'file_widget' extends from #type 'managed_file'

That makes sense to me! I wonder if we can still get this kind of change in for D8 beta?

Not sure if this is related or not...

I was looking into this while trying to solve another issue and realized that file_widget theme isn't even used any more. It seems that file_managed_file gets picked every time instead of it.

As part of #2511036: image-widget.html.twig never gets used (image_widget gets overridden by FieldWidget::process) this hunk went in:

diff --git a/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php b/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php
index acba472..5852ab4 100644
--- a/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php
+++ b/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php
@@ -370,7 +370,10 @@ public static function process($element, FormStateInterface $form_state, $form)
     $item = $element['#value'];
     $item['fids'] = $element['fids']['#value'];
 
-    $element['#theme'] = 'file_widget';
+    // Prevent the file widget from overriding the image widget.
+    if (!isset($element['#theme'])) {
+      $element['#theme'] = 'file_widget';
+    }

Intent of this hunk was to prevent file widget from overriding theme that image widget sets. Since file managed element sets theme to file_managed_file this condition always stays unsatisfied and file_widget theme unused as a result of that.

I am wondering if we need file_widget at all?

@slashrsm we probably don't need it in core but I'm thinking the fact that it's there and it's RC means we can't remove it.

@Cottser: I had the same thought too. This probably means D9 material?

This is very confusing and I think we should consider it as borderline 'dead code removal' for an RC target.

If a module is relying on this, I'd be very surprised.

If not, we should at least deprecate it somehow.

Removed, let's see what happens:)

Looks great, except following

+++ b/core/lib/Drupal/Core/Template/Attribute.php
@@ -117,10 +117,11 @@ public function offsetSet($name, $value) {
-    // If the value is already an AttributeValueBase object, return it
-    // straight away.
+    // If the value is already an AttributeValueBase object,
+    // return a new instance of the same class, but with the new name.
     if ($value instanceof AttributeValueBase) {
-      return $value;
+      $class = get_class($value);
+      return new $class($name, $value->value());

how this change related?

Yeah I think there's some definitely some unrelated changes in there.

Whoops, thanks for the review. Well all works;)

Looks good my only thought is there's maybe some code that would be dead in core/modules/file/file.js as a result:

  Drupal.behaviors.filePreviewLinks = {
    attach: function (context) {
      $(context).find('div.js-form-managed-file .file a, .file-widget .file a').on('click', Drupal.file.openInNewWindow);
    },
    detach: function (context) {
      $(context).find('div.js-form-managed-file .file a, .file-widget .file a').off('click', Drupal.file.openInNewWindow);
    }
  };

Found that grepping for file_widget and file-widget.

That JS is not dead. It is still used by file/image widget. Standard install, node/add/article, upload an image in image field, image link that appears is handeled by that code.

In JS console on the same page:

> jQuery('div.js-form-managed-file .file a, .file-widget .file a');

[<a href=​"http:​/​/​localhost/​drupal8/​sites/​default/​files/​2015-11/​18129594179_a842d51637_k.jpg" type=​"image/​jpeg;​ length=2018497" target=​"_blank">​18129594179_a842d51637_k.jpg​</a>​]

I'm not saying all those lines of JS are dead but the .file-widget .file a selector sure seems like it would be :)

I agree, not changing status because I believe that is all the things.

I discussed this with @xjm to triage, but @catch wasn't online. Assigning to him, so that we figure it out when he is.

Ah, comment #4 explains why/how file-widget.html.twig is currently dead code. Yeah, this is better to clear up pre-release than have to support until 9.x.

Committed/pushed to 8.0.x, thanks!