Fixed XF.AttachmentManager/XF.ImageTools destroys EXIF data

[DragonByte Tech]

Affected version

2.3.6

Currently, it is impossible to fully preserve an attachment's EXIF data. There's a number of factors at play here:

• The bundled ExifReader library fails to obtain all EXIF data from sample photos found here: https://www.geoimgr.com/sample-photos.html and other sample photos I have tested with
• XF.ImageTools.resize() does not abort resizing if the global image size limits are set to 0 or left blank
• XF.ImageTools.resize() will always discard the old image because the canvas size will always be smaller than the original image

I have attached a sample image which, when adding debug statements, causes these prints:


This is the relevant portion of XF.ImageTools.resize():

                            const newFile = new File([blob], file.name, {
                                type: asType,
                                lastModified: file.lastModified,
                            })
                            console.log("New file size:", newFile.size)
                            console.log("Old file size:", file.size)
                            if (!neededResizing && newFile.size >= file.size)
                            {
                                console.log("Preserved old file")
                                resolve(file)
                                return
                            }

                            console.log("Replaced with new file")
                            resolve(newFile)

As seen in this suggestion: https://xenforo.com/community/threads/retain-exif-data-on-optimized-processed-images.225525/ there are numerous instances where preserving all EXIF data is beneficial. Therefore, I suggest the following fix:

Index: js/xf/attachment_manager.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/xf/attachment_manager.js b/js/xf/attachment_manager.js
--- a/js/xf/attachment_manager.js
+++ b/js/xf/attachment_manager.js
@@ -1073,6 +1073,12 @@
                         neededResizing = false
                     }
 
+                    if (!neededResizing)
+                    {
+                        resolve(file)
+                        return
+                    }
+
                     if (maxWidth && width > maxWidth)
                     {
                         height *= maxWidth / width

 

[Kirby]

The bundled ExifReader library fails to obtain all EXIF data from sample photos found here: https://www.geoimgr.com/sample-photos.html and other sample photos I have tested with

This is intentional / working as designed.
If you need full EXIF data you either have to use a custom build of ExifReader or disable client side image processing.

XF 2.3 Post in thread 'Image optimization, enhanced image resizing, and more!'

Nov 11, 2023

Not yet, but we'd likely just take the approach I outlined where add-ons are free to swap in a build of the library with a bigger set. We don't have a built-in option to disable client-side processing, but it is a simple toggle on the attachment manager (data-resize-images="false").

• Jeremy P

Index: js/xf/attachment_manager.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/xf/attachment_manager.js b/js/xf/attachment_manager.js
--- a/js/xf/attachment_manager.js
+++ b/js/xf/attachment_manager.js
@@ -1073,6 +1073,12 @@
                         neededResizing = false
                     }
 
+                    if (!neededResizing)
+                    {
+                        resolve(file)
+                        return
+                    }
+
                     if (maxWidth && width > maxWidth)
                     {
                         height *= maxWidth / width

This kinda defeats the purpose of client side image processing:
Even if no resizing is required converting to WebP (if enabled) usually still decreases upload size (and server processing).

 

[DragonByte Tech]

If you need full EXIF data you either have to use a custom build if ExifReader or disable client side image processing.

How would you disable client side image processing globally?

 

[Kirby]

Extend XF.AttachmentManager and set this.options.resizeImages = false;

 

[Jeremy P]

It's true that this is deliberate on the whole, and that we don't abort when the resulting file size is smaller (though I would note it's not actually always smaller) since smaller files mean less data and faster uploads. We also only capture basic EXIF data out of the box to save on JS size, and geolocation is a deliberate omission.

However I'm currently looking at making this easier to change for add-ons. We'll maybe ship a full version of ExifReader and allow add-ons to swap it in. Unfortunately even if you do that manually right now, some other adjustments are needed to map the data back to the shape PHP captures natively so trying to sort that first.

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.3.7).

Change log:

Improve supported EXIF data when client-side image resizing is enabled

There may be a delay before changes are rolled out to the XenForo Community.

 

[Jeremy P]

OK, we can ship a full JPEG/EXIF JS bundle for only ~5kb additional gzipped, so we've gone ahead with that for the next release. The EXIF data should now be retained to the greatest extent possible without requiring changes to third-party code. I've attached a diff.