processwire / processwire-issues

ProcessWire issue reports.
44 stars 2 forks source link

image-gallery custom fields drag'n'drop issue #1586

Closed pine3ree closed 1 year ago

pine3ree commented 2 years ago

Jump to https://github.com/processwire/processwire-issues/issues/1586#issuecomment-1157135129 for steps to reproduce the issue(s)

Custom image fields values are lost after replacing the image with drag and drop

I am using custom fields for an image gallery (to be used in a carousel/slider).

Actual behavior

When I try to replace the image of one item in the gallery field the following happens:

Here is the screenshot for the multilanguage version:

pw-issue-4

UPDATE (2020-06-20) multilanguage tabs get messy if the internal image description is disabled (n.lines=0) other fields (like autocomplate page select) loose their functionality as well

pine3ree commented 2 years ago

WORTH NOTING

I experimented a little bit: using drag and drop results in input name changes, and custom fields seem not handle that. But MORE IMPORTANTLY, even the image description field is emptied with a second image replacement (d'n'd) without saving the page in between and this happens even without custom fields enabled.

pine3ree commented 2 years ago

OTHER ISSUES

I made some other tests with gallery image field (pw 3.0.200, chrome latest, firefox DE latest) without custom fields and/or multilanguage on

  1. If I drag and drop the same image (with the same name of the one in the cms) twice the second operation results in the description field being emptied, but when I save the originla image is still there with the original description and a new image field is added w/o description, and with indexed name (slide-1.jpg => slide-1-1.jpg)
  2. If I drag'n'drop (once is enough in this case) the same name image file on a field with "Overwrite existing files", on save the image is deleted from the page and I get this error " WireFileTools: rename: Given pathname ($oldName) that does not exist: /srv/www/processwire.lan/30/web/blank/site/assets/files/1/slide-1.jpg"
pine3ree commented 2 years ago

SINGLE IMAGE FIELD

I tested the above with a single image fields, with a custom "title" field...both w and w/o the overwite flag, replacing fils with the same or different names several times and everything works fine here (not tested multilanguage custom fields for single image yet). SO the issie is appearing only with multiple fiel/image field.

pine3ree commented 2 years ago

SINGLE IMAGE FIELD with custom multilingual title

For single image even with language fields there are no issue, title and description are not lost, with and without the OVERWRITE FILE flag (which is a very useful flag). Languag tabs work too.

So the issues (more or less serious depending on the overwrite flag setting) only happen for MULTI-IMAGE fields with or w/o language-ields or custom-fields at all. The also happen in the previous stable/master release. Given the language tabs messy behaviour as a hunch I believe it has to do with the automatic change of inputs' names on drag'n'drop. ...nope, the single upload fields adjust its inputs names using the pagefile->hash, but it works.

Mutliple Files(not-image) fields (with custom fields) seem unaffected, even in overwrite mode, I can drag'n'drop several times the same file or another file with the same name without losing the description/custom-fields (even language fields) and without having the file+fields erroneously deleted.

kind regards

pine3ree commented 2 years ago

Steps to reproduce the issue

Preparation (common)

Add a multiple-image field (and optionally add a custom field like "title" or better a not-required text-field like "headline" to it). . Attach the field to a template and edit a page with that template.

ISSUE 1
  1. Activate the Overwrite existing files flag
  2. Add an image and fill the description (and the optional custom field), Save the page
  3. Edit the previously added image, drag'n'drop the same image or an image with the same filename into it and Save the page
  4. The image is deleted with a message: WireFileTools: rename: Given pathname ($oldName) that does not exist...
ISSUE 2
  1. Activate the Overwrite flag
  2. Add an image and fill the description (and the optional custom field), Save the page
  3. Edit the previously added image, drag'n'drop an image with a different filename into it => custom fields are emptied
  4. Drag'n'drop an image with a different filename into it => description field is emptied
  5. Save the page
  6. You end up with 2 images, the original with all the custom and description fields untouched and a new one with a filename matching the second filename, but the image matching the 3rd image dropped into the old image (in the edit image box)
ISSUE 3
  1. Deactivate the Overwrite flag
  2. Add an image and fill the description (and the optional custom field), Save the page
  3. Edit the previously added image, drag'n'drop an image with the same filename into it => custom fields are emptied (this happen with a different filename image too)
  4. Drag'n'drop the same image or an image with a same filename again into it => description field is emptied
  5. Save
  6. You end up with 2 images, the original with all the custom and description fields untouched and a new one (same image though) with a filename with increment index (-1) and empty fields.
pine3ree commented 2 years ago

Digging into related code... when i check the Pageimage::filedata after dropping a new image for replacement before the loop in https://github.com/processwire/processwire/blob/3acd7709c1cfc1817579db00c2f608235bdfb1e7/wire/modules/Inputfield/InputfieldFile/InputfieldFile.module#L1370, I get an empty array (while I get the custom field values when I load the page for editing). So $item->getFieldValue($name) returns null for each custom field and the original image field values are not populated after the replacement image has been dropped.

pine3ree commented 2 years ago

Where it happens (?) a.k.a. $pagefile vs $metaPageFile

I believe I found where custom field data is lost on image replace (in multi-file fields). After the image is dropped into the image detail area (in edit mode) Inputfile(Image|File)::fileAdded is called to send an ajax response, which make inner calls to fileAddedGetMarkup, renderItem, getMetaPagefile (which still has custom fields data).

$metaPageFile is used to build the description and tag inputs, but then $pagefile (not containing customfields data) is used to build custom fields inputs which in turn ends up emtpy https://github.com/processwire/processwire/blob/3acd7709c1cfc1817579db00c2f608235bdfb1e7/wire/modules/Inputfield/InputfieldImage/InputfieldImage.module#L668

Using $metaPageFile in the getItemInputfields call rebuilds the older inputs keeping the value, but those are not persisted as they have a different input name (file hash), while the description fields are built from the $metaPageFile data and from the new $id renderItemDescriptionField(Pagefile $pagefile, $id, $n). On second image drop w/o page save, all the fields get empty anyway.

ryancramerdesign commented 2 years ago

@pine3ree Thanks for the reports and testing. I went through these in reverse order, duplicating and applying your fix for the 3rd issue you mentioned first. I wasn't able to duplicate issue 1 or 2, could your fix have resolved those as well? Everything worked as expected for 1 and 2. I'm on OS X, so also wondering if maybe there's some platform difference affecting this, such as directory slash type or something.

pine3ree commented 2 years ago

@ryancramerdesign , I am on Linux so it shoud be the same as the freebsd-derived OS X.

Just to be sure I've just tried with MS Edge (on linux). (when editing a single image field, the edit area always reset itself so when you reopen the edit area it loads fresh values)

When editing a gallery item field, i still experience the issues:

Can I upload a short mp4 screen recording here?

pine3ree commented 2 years ago

Oh, I was sure I mentioned it somewhere... but ...anyway: even with my quick fix, custom language fields formatting is lost if the description field is disabled (rows=0)

@ryancramerdesign, the actual code that removes the image-file is at this line: https://github.com/processwire/processwire/blob/3acd7709c1cfc1817579db00c2f608235bdfb1e7/wire/modules/Inputfield/InputfieldFile/InputfieldFile.module#L951

matjazpotocnik commented 1 year ago

@pine3ree did you test Ryan's fix? Is it working for you? Can we close this issue?

pine3ree commented 1 year ago

Oh yes, @matjazpotocnik , that fix worked for me a long time ago, I closed the PR, but I forgot to close this issue... thanks!