Problem uploading gif, undefined properties $m_nDelay, $m_bTrans, $m_nTrans

[justb3a]

Short description of the issue

I'm not able to upload certain gif files(*). Default images field, valid file extensions: gif jpg jpeg png.

(*) This behaviour doesn't occur for every gif file.

Expected behavior

The gif gets successfully uploaded ;)

Actual behavior

JavaScript enabled:

The gif won't be uploaded (the loading icon never stops spinning).
The following error message occurs:

VM1720:1 Uncaught SyntaxError: Unexpected token < in JSON at position 0
    at JSON.parse (<anonymous>)
    at Function.parseJSON (JqueryCore.js?v=1475855324:2)
    at XMLHttpRequest.<anonymous> (InputfieldImage.js?v=119-1475855324:1403)

InputfieldImage.js: var response = $.parseJSON(xhr.responseText),

JavaScript disabled

The gif gets uploaded but the following PHP Notices occurs:

Notice: Undefined property: ProcessWire\PWGIFIMAGE::$m_nDelay in /../public/wire/core/ImageInspector.php on line 231

Notice: Undefined property: ProcessWire\PWGIFIMAGE::$m_bTrans in /../public/wire/core/ImageInspector.php on line 232

Notice: Undefined property: ProcessWire\PWGIFIMAGE::$m_nTrans in /../public/wire/core/ImageInspector.php on line 233

Screenshots/Links that demonstrate the issue

This is the gif file I'm trying to upload.

Suggestion for a possible fix

@ ImageInspector.php:231 @ class ImageInspector extends WireData {
-    $i['delay']     = $gi->m_nDelay;
-    $i['trans']     = $gi->m_bTrans;
-    $i['transcolor']  = $gi->m_nTrans;
+    $i['delay']     = isset($gi->m_nDelay) ? $gi->m_nDelay : '';
+    $i['trans']     = isset($gi->m_bTrans) ? $gi->m_bTrans : '';
+    $i['transcolor']  = isset($gi->m_nTrans) ? $gi->m_nTrans : '';

Steps to reproduce the issue

1. Try to upload the gif file linked above.

Setup/Environment

• ProcessWire version: 3.0.36
• PHP version: PHP 7.0.5

[adrianbj]

Just to let you know, that gif uploaded just fine for me on 3.0.36 and PHP 7.1.x. Not sure if it matters, but I am also running imagick.

Not saying your isset shouldn't be implemented, but wondering if it also needs a property_exists check as well?

[justb3a]

I'm not running imagick:

===================== ============================
Image Diagnostics
===================== ============================
GD library Bundled (2.1.0 compatible)
GD JPG Support Supported
GD PNG Support Supported
GD GIF Support Supported
GD FreeType Support Supported
Exif read data Available
Imagick Extension Not available
===================== ============================

[adrianbj]

Sorry, I take that back - even though that image is successfully uploading for me, those same notices are being logged. I think it was maybe a case of Tracy capturing them and sending to the console that prevented them from being in the ajax response, which is why the upload didn't break, but that's just a guess.

[justb3a]

Sorry, I take that back
Good to know, because I just installed imagick and the problem still occurs.

[ryancramerdesign]

Thanks, great find – I have added your fix. Admittedly I don't totally understand what all is happening in this block of code so would like @horst-n to take a look at it too just in case we should be substituting different empty values or something like that. Also I noticed that the thumbnail loses the animation and wasn't sure if it was supposed to or not.

[horst-n]

I will look into this later this evening. But Beas fix looks good!

@justb3a: have you installed the AnimatedGif module, too?

[justb3a]

@justb3a: have you installed the AnimatedGif module, too?

Nope. I did not install any image modules.

[isellsoap]

@horst-n Any updates?

@justb3a Can the issue be closed?

[horst-n]

@ryancramerdesign sorry for the delay.
The fix is good. Maybe only the $gi->m_bTrans should be changed to boolean false:

ImageInspector.php:232
- $i['trans'] = isset($gi->m_bTrans) ? $gi->m_bTrans : '';
+ $i['trans'] = isset($gi->m_bTrans) ? $gi->m_bTrans : false;

---

additional, related TODO: animated GIF support in core. At the moment it only is supported through ImageAnimatedGif module. I need to port that into an ImageSizerEngine.

[ryancramerdesign]

Thanks Horst, I've updated that. I'll close this one out.