Missing preview image in widget

Moving to entity browser.

I was able to reproduce. Problem is that we assume that there will be no view builder if file_entity isn't installed, but that is not the case. It seems that core adds \Drupal\Core\Entity\EntityViewBuilder if no other is defined. Attached patch fixes the problem, but I am not sure if it takes the best approach.

This could use a test :)

Just ran into the same issue. Rolled back from Alpha 9 to Alpha 8 to workaround for now. Happy to test any patches.

@slashrsm Are you asking for someone to write a test or for someone to test your patch?

This patch is actually wrong. Should use ->getViewBuilderClass() instead of ->hasViewBuilderClass().

Are you asking for someone to write a test or for someone to test your patch?

Both would be very helpful.

This should work. Test coverage is still missing.

working on it

width/height attributes cause problems when the image is initially selected since they end up being 0, which seems to force image to be 0x0px in size. If we default to NULL that isn't the case.

Created a test for the image field. Not sure if the coverage is enough though.

The same but creating a new browser, in order not to mess with other tests.

argh sorry

Testing title and alt on the images as well.

Just few nitpiks:

1. +++ b/tests/modules/entity_browser_test/config/install/entity_browser.browser.test_entity_browser_iframe2.yml
   @@ -0,0 +1,30 @@
   +name: test_entity_browser_iframe2
   +label: 'Test entity browser iframe2'
   

   Let's try to figure out a better name. Maybe something that indicates which plugins are used in it.

2. +++ b/tests/modules/entity_browser_test/config/install/entity_browser.browser.test_entity_browser_iframe2.yml
   @@ -0,0 +1,30 @@
   +    id: view
   \ No newline at end of file
   

   Newline missing

3. +++ b/tests/src/FunctionalJavascript/EntityBrowserJavascriptTestBase.php
   @@ -177,4 +180,25 @@ abstract class EntityBrowserJavascriptTestBase extends JavascriptTestBase {
   +  /**
   +   * Debugger method to save additional HTML output.
   +   *
   +   * The base class will only save browser output when accessing page using
   +   * ::drupalGet and providing a printer class to PHPUnit. This method
   +   * is intended for developers to help debug browser test failures and capture
   +   * more verbose output.
   +   */
   +  protected function saveHtmlOutput() {
   +    $out = $this->getSession()->getPage()->getContent();
   +    // Ensure that any changes to variables in the other thread are picked up.
   +    $this->refreshVariables();
   +    if ($this->htmlOutputEnabled) {
   +      $html_output = '<hr />Ending URL: ' . $this->getSession()->getCurrentUrl();
   +      $html_output .= '<hr />' . $out;
   +      $html_output .= $this->getHtmlOutputHeaders();
   +      $this->htmlOutput($html_output);
   +    }
   +  }
   +
   

   It might make sense to see if we could move this to core. Let's keep it here and create a follow-up.

4. +++ b/tests/src/FunctionalJavascript/ImageFieldTest.php
   @@ -0,0 +1,133 @@
   + * Tests the entity_browser.
   

   Should better explain what we are testing (mention image field widget).

Addressing feedback.

Concerning

3. It might make sense to see if we could move this to core. Let's keep it here and create a follow-up.

After asking some people about it, it looks like after #2763401: PHPunit browser tests should log all Mink requests is in, it should be easier to have the output log inside the tests, but I'm not sure to what extent we could abandon this helper function completely or just to modify it.

I would prefer not putting explicit module checks in the codebase (in this issue or #2807997: EntityReferenceBrowserWidget shouldn't expose an "edit" button if the entity is not editable), as we can't assume that File Entity is the only contrib module that enhances Files by adding view modes and an edit form. Why was the logic changed in #8 to depend on File Entity?

We discussed and agreed on this. My reason is that we know how file_entity works, while we have no clue about what it means when another module ads a view builder. We can change this if we see other actual use cases of people doing this.

So there's no other way to check for the functionality that File Entity provides?

This patch was initially checking for the presence of view builder on file entities, but it turned out that core add a default one, which made this approach more or less unusable. I am also not completely happy with the moduleExists() check, but it seems that this is the best option that we have.

Here's an alternate solution which seems to work for me. Instead of checking for File Entity, we check for the presence of View Modes. If there are any View Modes, there must be some system that allows for the configuration of those View Modes (i.e. File Entity). I've tested this manually with and without File Entity, and the logic is holding up so far. Let me know what you think.

And some quick validation with Drush:

~/Sites/drupal/modules (8.2.x) $ drush si standard -y && drush ev 'echo count(\Drupal::service("entity_display.repository")->getViewModes("file"))'
[truncated]
0
~/Sites/drupal/modules (8.2.x) $ drush en file_entity -y && drush ev 'echo count(\Drupal::service("entity_display.repository")->getViewModes("file"))'
[truncated]
1

View modes can be deleted so there could be none. But, as you can't actually *select* a view mode when ther are none to be selected, that makes kind of sense ;)

Works for me.

View modes can be deleted so there could be none

Ah, totally didn't think of that. If we go with this solution we assume that your don't want the full/default View Mode displayed as the preview, which was unintended when I wrote the patch but works for me as well.

Committed. Thanks!

Does not work.

Using latest dev (as of 15 minutes ago, includes this patch). Not using file_entity module.

drush ev 'echo count(\Drupal::service("entity_display.repository")->getViewModes("file"))'
2

@glynnr What contributed modules are you using? I haven't run into a site not using File Entity with multiple view modes for files yet.

Here is output from "drush pml"

 A/B Testing          Optimizely (optimizely)                                  Module  8.x-0.1         
 Administration       Admin Toolbar (admin_toolbar)                            Module  8.x-1.17        
 Administration       Admin Toolbar Extra Tools (admin_toolbar_tools)          Module  8.x-1.17        
 Administration       Adminimal Admin Toolbar (adminimal_admin_toolbar)        Module  8.x-1.3         
 Blazy                Blazy (blazy)                                            Module  8.x-1.0-beta3   
 Chaos tool suite     Chaos tools (ctools)                                     Module  8.x-3.0-alpha27 
 Core                 Block (block)                                            Module  8.2.0           
 Core                 Breakpoint (breakpoint)                                  Module  8.2.0           
 Core                 CKEditor (ckeditor)                                      Module  8.2.0           
 Core                 Color (color)                                            Module  8.2.0           
 Core                 Configuration Manager (config)                           Module  8.2.0           
 Core                 Contextual Links (contextual)                            Module  8.2.0           
 Core                 Custom Block (block_content)                             Module  8.2.0           
 Core                 Custom Menu Links (menu_link_content)                    Module  8.2.0           
 Core                 Database Logging (dblog)                                 Module  8.2.0           
 Core                 Field (field)                                            Module  8.2.0           
 Core                 Field UI (field_ui)                                      Module  8.2.0           
 Core                 Filter (filter)                                          Module  8.2.0           
 Core                 Help (help)                                              Module  8.2.0           
 Core                 Menu UI (menu_ui)                                        Module  8.2.0           
 Core                 Node (node)                                              Module  8.2.0           
 Core                 Path (path)                                              Module  8.2.0           
 Core                 RDF (rdf)                                                Module  8.2.0           
 Core                 Responsive Image (responsive_image)                      Module  8.2.0           
 Core                 System (system)                                          Module  8.2.0           
 Core                 Taxonomy (taxonomy)                                      Module  8.2.0           
 Core                 Text Editor (editor)                                     Module  8.2.0           
 Core                 Toolbar (toolbar)                                        Module  8.2.0           
 Core                 Update Manager (update)                                  Module  8.2.0           
 Core                 User (user)                                              Module  8.2.0           
 Core                 Views (views)                                            Module  8.2.0           
 Core                 Views UI (views_ui)                                      Module  8.2.0           
 Development          Devel (devel)                                            Module                  
 Development          Devel Kint (kint)                                        Module                  
 Development          Features (features)                                      Module  8.x-3.0-rc1     
 Development          Features UI (features_ui)                                Module  8.x-3.0-rc1     
 Display Suite        Display Suite (ds)                                       Module  8.x-2.6         
 Entityqueue          Entityqueue (entityqueue)                                Module                  
 Feeds                Feeds (feeds)                                            Module                  
 Field types          Datetime (datetime)                                      Module  8.2.0           
 Field types          File (file)                                              Module  8.2.0           
 Field types          Image (image)                                            Module  8.2.0           
 Field types          Link (link)                                              Module  8.2.0           
 Field types          Options (options)                                        Module  8.2.0           
 Field types          Text (text)                                              Module  8.2.0           
 Fields               Inline Entity Form (inline_entity_form)                  Module  8.x-1.0-alpha6  
 Filters              Entity Embed (entity_embed)                              Module  8.x-1.0-beta1   
 GatherContent        GatherContent (gathercontent)                            Module                  
 Images               Focal Point (focal_point)                                Module  8.2.0           
 Layout               Layout Plugin (layout_plugin)                            Module  8.x-1.0-alpha22 
 Media                Colorbox (colorbox)                                      Module  8.x-1.1         
 Media                Crop API (crop)                                          Module  8.x-1.0         
 Media                dropzonejs (dropzonejs)                                  Module                  
 Media                DropzoneJS entity browser widget (dropzonejs_eb_widget)  Module                  
 Media                Entity Browser (entity_browser)                          Module                  
 Media                Image Effects (image_effects)                            Module                  
 Media                ImageMagick (imagemagick)                                Module  8.x-1.0-alpha2                  
 Other                Configuration Update Base (config_update)                Module  8.x-1.1         
 Other                Configuration Update Reports (config_update_ui)          Module  8.x-1.1         
 Other                Content Browser (content_browser)                        Module                  
 Other                Embed (embed)                                            Module  8.x-1.0-rc3     
 Other                Entity (entity)                                          Module  8.x-1.0-alpha3  
 Other                File Browser (file_browser)                              Module                  
 Other                Imagecache External (imagecache_external)                Module  8.x-1.0         
 Other                Link class (link_class)                                  Module  8.x-1.0                 
 Other                Node clone (node_clone)                                  Module                  
 Other                Pathauto (pathauto)                                      Module  8.x-1.0-beta1   
 Other                Redirect (redirect)                                      Module  8.x-1.0-alpha1  
 Other                Token (token)                                            Module  8.x-1.0-beta2   
 Other                Viewfield (viewfield)                                    Module  8.2.0           
 Other                Views Accordion (views_accordion)                        Module                  
 SEO                  Metatag (metatag)                                        Module                  
 Slick                Slick (slick)                                            Module  8.x-1.0-beta4   
 Slick                Slick UI (slick_ui)                                      Module  8.x-1.0-beta4   
 Slick                Slick Views (slick_views)                                Module  8.x-1.0-beta4   
 Statistics           Google Analytics (google_analytics)                      Module  8.x-2.1         
 User authentication  SAML Authentication (samlauth)                           Module  8.x-1.3         
 Video Embed Field    Video Embed Field (video_embed_field)                    Module  8.x-1.3         
 Web services         Default content (default_content)                        Module                  
 Web services         HAL (hal)                                                Module  8.2.0           
 Web services         RESTful Web Services (rest)                              Module  8.2.0           
 Web services         Serialization (serialization)                            Module  8.2.0           
 Workbench            Workbench (workbench)                                    Module  8.x-1.0-alpha2  
 Core                 Seven (seven)                                            Theme   8.2.0           
 Other                Adminimal (adminimal_theme)                              Theme   8.x-1.1         

It's possible that I may have enabled file_entity module in the past, and then uninstalled at some point.

More info:

drush ev 'drush_print_r( \Drupal::service("entity_display.repository")->getViewModes("file"))'
Array
(
    [thumbnail] => Array
        (
            [uuid] => 5cae7462-2785-466b-8cc2-c2d00b376331
            [langcode] => en
            [status] => 1
            [dependencies] => Array
                (
                    [module] => Array
                        (
                            [0] => file
                        )

                )

            [_core] => Array
                (
                    [default_config_hash] => xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
                )

            [id] => file.thumbnail
            [label] => Thumbnail
            [targetEntityType] => file
            [cache] => 1
        )

    [token] => Array
        (
            [uuid] => 0ee6d9ba-a5e3-4685-ad40-71dc555cb07e
            [langcode] => en
            [status] => 1
            [dependencies] => Array
                (
                    [module] => Array
                        (
                            [0] => file
                        )

                )

            [id] => file.token
            [label] => Token
            [targetEntityType] => file
            [cache] => 1
        )

)

It's possible that I may have enabled file_entity module in the past, and then uninstalled at some point.

That's one option, another is that token_install() actually creates view modes for all entity types with a view builder now, which is now all of them. (I want to remove that, but didn't happen yet).

Didn't think of that before.

Let's just go with the module check. It's not perfect, but we know exactly what the check does and how it behaves.

Edit: Actually had a cross-post. Yes, so it is indeed both. file_entity installed once which added thumbnail and also token.module.

Re-opening this.

The patch in #20 is out of date and doesn't apply. If someone can re-roll that, I will be happy to test it right now.

Thanks
Glynn

Going with the patch from #20 is the easiest solution at this point, the only other idea I have is to check that there's a way to edit the display mode for File entities, since that's more specific. The problem is that I don't know how to do that consistently, and if Field UI is disabled that may mess up any other logic I try to write.

Considering that, I'm OK with going with the moduleExists check. I assume that at some point in the D8 cycle File Entity will get into core, so this is really just making up for core's shortcomings.

@glynnr I reverted previous commit and #20 applies again.

Confirmed. Patch in #20 now applies to latest dev, and works.

Thanks for your quick response an this.

Thanks! Committed.