Problem/Motivation

Sites that use IMCE as their file browser cannot use it with the Image component. The component's settings form always renders a managed_file upload element, so an editor has to re-upload an image that already exists in the file system rather than picking it from the IMCE browser.

A patch by ecj on #3318209 implements this: when the imce module is enabled it swaps the managed_file element for a plain textfield attached to the imce/drupal.imce.input library, which turns the field into an IMCE-backed file picker. Credit to ecj for the idea and the working prototype. Splitting it out of #3318209 so it can get the design discussion it needs.

Proposed resolution

This is not a drop-in change and the patch should not be committed as-is. Swapping the element type changes both validation and the stored data model. Specifically:

  • File extension validation is lost on the IMCE path. The managed_file element carries #upload_validators; the replacement textfield does not, so nothing constrains what path an editor can enter. Whatever the final design is, the IMCE path needs its own validation.
  • The stored value changes type. Today the block configuration stores an array of file IDs. The IMCE path stores a string path instead. That is why the patch has to branch on is_array / is_string / is_int inside build() - the branching is a symptom of two incompatible value shapes sharing one configuration key.
  • That is a data model change and needs a migration story. Existing block configurations hold file-ID arrays. Any site that enables imce after the fact, or disables it later, has to keep rendering its existing blocks. An update path, or a value shape that is unambiguous for both sources, has to be part of the design.
  • The patch's string branch can fatal. It calls $file->id() on the result of loadByProperties() without checking that anything was returned. If the entered path does not correspond to a managed file - a file uploaded outside Drupal, or a typo - loadByProperties() returns an empty array and the call is on NULL.

Design questions to settle before implementation:

  1. Should the source (upload vs. IMCE) be an explicit per-block setting rather than implicitly keyed off whether the imce module is installed? Enabling or disabling a module should not silently change how existing blocks store their data.
  2. Should both paths normalise to a single stored shape - for example always resolving to a file ID, creating a managed file record for an IMCE selection - so build() has one code path instead of three?
  3. What validation applies to the IMCE path, and where does it live?

Steps to reproduce

  1. Install and configure the imce module with a file browser profile.
  2. Add an Image component to a layout and open its settings form.
  3. Only a direct upload field is offered; there is no way to browse existing files through IMCE.

Remaining tasks

  • Agree the design - answers to the three questions above - before writing code.
  • Implement with validation on the IMCE path and a null-safe file lookup.
  • Provide an update path for existing block configurations if the stored shape changes.
  • Add test coverage for both sources, including the case where the entered path matches no managed file.

Comments

aangel created an issue.