Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v5] Structure Field with Files Type Generates Redundant Image Data in Content Files #6992

Closed
jirichlebus opened this issue Feb 11, 2025 · 14 comments
Milestone

Comments

@jirichlebus
Copy link

I've discovered an issue in Kirby 5 beta.1 through beta.3 regarding how image data is stored in content files when using structure fields with file selections.

Description:

When an image is selected in a structure field that contains a files-type field, the generated content file contains redundant and unnecessarily verbose image data. The content file includes multiple instances of the same image information, including base64 data, URLs, and metadata.

Steps to Reproduce:

Create a blueprint with a structure field containing a files field:

structure:
  type: structure
  fields:
    image:
      type: files

Add an image to this structure field through the Panel
Examine the resulting content file

Current Behavior:

The content file generates extensive redundant data for each image, including:

Multiple URL entries
Base64 encoded data
Duplicate metadata
Multiple reference paths to the same image

Image

Environment:

Kirby Version: 5 beta.1, 5 beta.2, 5 beta.3

@jirichlebus
Copy link
Author

@medienbaecker replicated basically the same results in his environment.

@distantnative
Copy link
Member

Can you already say whether that's new in v5 or also a bug present in v4? Will only be able to try replicating it later.

@jirichlebus
Copy link
Author

Can you already say whether that's new in v5 or also a bug present in v4? Will only be able to try replicating it later.

No issue in Kirby 4.6.1

@distantnative distantnative added this to the 5.0.0 milestone Feb 11, 2025
@medienbaecker
Copy link
Contributor

Because I initially thought this was related to my plugin, I'll leave my learnings here. Maybe it helps you debug faster:

  • It happens when you upload a file inside of a drawer (also in an object field for example)
  • Interestingly enough, it doesn't matter which file you actually select in the files field, only the uploaded files' data will be saved

@distantnative
Copy link
Member

That's a great pointer.

Note to myself: Checking these lines https://github.com/getkirby/kirby/blob/v5/develop/panel/src/components/Forms/Field/FilesField.vue#L58-L60 - maybe they send the whole object to be saved and it should just send the UUID/id.

@distantnative
Copy link
Member

I haven't quite been able to replicate this. That's what ends up in my content file:

Image

@distantnative
Copy link
Member

Nevermind, I can replicate it when actually saving it. Which is interesting cause it's right in _changes and then on publish (internal term for the save) it becomes that larger object.

@bastianallgeier
Copy link
Member

I've added a nested files field test setup to the sandbox getkirby/sandbox@956587b

@medienbaecker
Copy link
Contributor

This issue is worse than I thought. It also happens outside of drawers, in blocks with preview: fields. Please let me know if you've already got something working in a branch or so, otherwise I'll have to move away from Kirby 5 for this project.

It can easily lead to content loss because it's overwriting the field of the parent page when uploading a file:

CleanShot.2025-02-21.at.10.11.12.mp4

@distantnative
Copy link
Member

@medienbaecker We are working on this but have not yet a fix anywhere. As far as we understand it, it will affect any setup where a files field is nested (e.g. in a block, structure field...).

@bastianallgeier
Copy link
Member

bastianallgeier commented Feb 21, 2025

I think it also only really happens when the field names match. "image" structure field with a nested "image" files field for example. That's the reason why I could not reproduce it in my setup. The sandbox example above has a different field name for the structure

@medienbaecker
Copy link
Contributor

medienbaecker commented Feb 21, 2025

Well, Kirby 4 it is then. Unfortunately it also happens when no matching field is found on the parent page. I've experienced it with my client yesterday. They uploaded a file, saved. Bug happened. They had unsaved changes, they saved again and then the content file was completely mangled — some fields missing completely.

Might be a nice collaboration between this issue and #6869, though.

@bastianallgeier
Copy link
Member

bastianallgeier commented Feb 21, 2025

I'm really sorry about the issue. At least, I think we are finally on a good track here. We've been pretty much in the dark for a while. Thanks for all your help to debug this.

@medienbaecker
Copy link
Contributor

No worries, that's what betas are for.

But I just tested it again in a 5.0.0-beta.3 plainkit. The phantom field gets added even if this field doesn't exist on the parent page. If that's different for you, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants