Module Custom Upload Names

[ukyo]

20 hours ago, adrian said:

Hi @ukyo - thanks for those details. Unfortunately I can't reproduce the problem here.

I put this Tracy dump call just before line 139 (as reported in the error):

bd($pagefile->getCrop($suffix)->url);

and it is correctly reporting the url to the cropped version of the file.

Perhaps there is a certain sequence of events that is required to trigger the error. Could you please detail exactly the steps involved starting from a new images field, including the crop settings you have configured for the field.

 

 

19 hours ago, horst said:

@adrian & @ukyo: I'm not aware of any upload processing done by CroppableImage3. It only extends the coreimage field in some points, but nothing in regard of uploading.

Here is step by step screenshots. Created new field, added crop settings added this field to custom upload names module settings and tried to upload files result is same on my side.

If i remove this field from Custom Upload Names module settings there is no error. Upload is done without error.

[horst]

@ukyo: what I tried to say is, that CroppableImage3 is an extention of the core imagefield and it incorporates all stuff related to uploading as is, without making any changes. Therefore, maybe it would be interesting if you have the same behave when trying with a regular core imagefield, or how that differs.

[ukyo]

19 minutes ago, horst said:

@ukyo: what I tried to say is, that CroppableImage3 is an extention of the core imagefield and it incorporates all stuff related to uploading as is, without making any changes. Therefore, maybe it would be interesting if you have the same behave when trying with a regular core imagefield, or how that differs.

I understood what you say @horst, but if i use core imagefield, don't have any error with CustomUploadNames module everything working well. When i try to use it with CroppableImage3 module i have this error.

With CroppableImage3 module, file upload is ok, but after upload progress javascript callback not working, Because there is an error : 

Notice: Trying to get property of non-object in /website/site/modules/CroppableImage3/InputfieldCroppableImage3/InputfieldCroppableImage3.module on line 139

like i said if i refresh the page, images uploaded and i can use CroppableImage3 module for resizing.

[adrian]

Sorry @ukyo - I can't reproduce here. Could you please try dumping $pagefile, $suffix, and $pagefile->getCrop($suffix) just before line 139 to see what we are dealing with. 

Thanks!

 

[horst]

Ok, thanks for clarification.

No time here to investigate much further, have no free dev env here atm. Looking into the code, I believe this is the chain:

• InputfieldFile::fileAdded ($pagefile)   // line 460
• InputfieldImage::fileAddedGetMarkup ($pagefile)  // line 490
• InputfieldImage::renderItem ($pagefile)   // line 522
• InputfieldCroppableImage3::renderButtons ($pagefile)  // line 56,   coming from InputfieldImage::renderItem line 545
• InputfieldCroppableImage3::getCropLinks ($pagefile)    // line 74,    calls many properties from $pagefile (!)

Debugging that three vars ($pagefile, $suffix, $pagefile->getCrop($suffix)) around line 128, like Adrian said, would be useful.

[ukyo]

1 hour ago, adrian said:

Sorry @ukyo - I can't can't reproduce here. Could you please try dumping $pagefile, $suffix, and $pagefile->getCrop($suffix) just before line 139 to see what we are dealing with. 

Thanks!

 

I updated TracyDebugger from 3.3.4 to latest version 3.3.8 and enabled Tracy Debugger for backend. And tested to upload images again, everything worked. Upload done, images refreshed after upload, debug result is ok. Also i checked my other CroppableImage3 module issue, don't have this issue, if TracyDebugger enabled for backend.

bd($pagefile);
bd($suffix);
bd($pagefile->getCrop($suffix)->url);

If i disabled TracyDebuger again for backend, my problem appeared again. I uninstalled TracyDebugger for make test, i have same problem also witout TracyDebugger.

[adrian]

Wow, that is very weird, but it turns out I can reproduce your problem when I have Tracy disabled on the backend (or completely uninstalled). I don't really know what is going on though - almost seems like Tracy is suppressing the error, which is the opposite of what usually happens  

Looking at that code in CroppableImage:

$attr['data-image'] = $pagefile->getCrop($suffix)->url;

This fixes it for me - can you try at your end?

if($pagefile->getCrop($suffix)) $attr['data-image'] = $pagefile->getCrop($suffix)->url;

The Pagefile exists, but during upload, the crop itself doesn't exist yet, so I think that makes sense, but I am sure it's just that I don't fully understand how Croppable Image works

@horst - any thoughts?

[horst]

@adrian: the normal behave of CroppableImage is, that it creates any requested crop variations on upload. (To be more precise: just after upload, but before or during markup for the Inputfield-Item gets rendered)

It does it in FieldtypeCroppableImage3::getCrop($suffix) // (exact the function that fails here!), so you need to debug this further:

• ...
• InputfieldCroppableImage3::getCropLinks ($pagefile)
• FieldtypeCroppableImage3::getCrop ($suffix)  // called from InputfieldCroppableImage3::getCropLinks ($pagefile) in line 139

getCrop() is a hook event, defined in FieldtypeCroppableImage3 init(), line 44. It starts at line 172. You should look (step by step, if possible) where it has an early return, without creating and passing back the expected pageimage object.

What part is failing in the process, when trying to get:

• $suffix  // 175
• $inputFieldInstance  // 190
• $templateName  // 197
• $cropSettings   // 198
• $img   // 220
• $imgPath   // 222
• $isReady  // 262 ( <= is it this? // do we have a file? : is_file($imgPath). - So, if yes, the values above, starting at line 221, $img->basename seems to be not updated with the changed ones from your module??

hope this helps a bit. If you find out what failes, we can look further what is the best way to fix it.

 

Edited October 15, 2016 by horst

[horst]

another thing: I haven't used the custom upload names module and thought to check if you have hooked before or after fileAdded

Surprisingly, you hooked into saveReady?! When I want manipulate uploaded files, I always use something like this:

// code snippet that belongs into the site/ready.php file:
$wire->addHookBefore("InputfieldFile::fileAdded", function(HookEvent $event) {

    $inputfield = $event->object;                      // handle to the image field
    if(!$inputfield instanceof InputfieldImage) {      // we need an images field, not a file field
        return;                                        // early return
    }

    if(version_compare(wire('config')->version, '2.8.0', '<')) {
        $p = $inputfield->value['page'];               // get the page, PW < 2.8
    } else {
        $p = $inputfield->attributes['value']->page;   // get the page, PW >= 2.8 | 3.0 (or only from 3.0.17+ ??)
    }

    $image = $event->argumentsByName('pagefile');      // get the image

	// now do something with the image or file
         ...
});

 

[ukyo]

I applied @adrian advice. Also I applied same solution for other issue with CroppableImage3 module.

if(property_exists($globalOptions, 'manualSelectionDisabled'))
if(property_exists($globalOptions, 'useImageEngineDefaults'))

After applied these i don't have error module look like work, but i don't know is this really a true solution? But for the moment its look like ok for continue to work.

[adrian]

3 hours ago, horst said:

Surprisingly, you hooked into saveReady?!

Thanks for all the details @horst - I will investigate shortly, but just wanted to point out that I am actually hooking before fiileAdded:
https://github.com/adrianbj/CustomUploadNames/blob/29985a70e5caeac6e2da3774b8be4a1be725a192/ProcessCustomUploadNames.module#L97

The saveReady hook is only there if the module settings specify to "rename on save" which is designed to keep the filename up to date with any future changes to the name of the page (or any other fields on the page that might be used to set the name of the file).

 

[horst]

@adrian AH! Ok, that's what I have expected, but overseen. Sorry!

@ukyo I don't think this is a real solution, because it should be able to create the crop and return a pageimage object. If you suppress the error so that it currently works for the upload, the crop will be created on the next request of getCrop(). But if there would be something other that fails, you wouldn't be informed and wouldn't get any crop variations. So this only can be a currrent workaround until @adrian has found the real issue.

OT: @ukyo With that other issue you mentioned, it seems that you are the only one who has this. Please can you answer in the CroppableImage Thread my questions?

[adrian]

@horst - I might be oversimplifying here, or maybe not

I think I have a solution - if you replace:

public function _getInputFieldInstance(HookEvent $event) {

        $field = null; // where we'll keep the field we're looking for
        $image = $event->object;
        $page = $image->page;
        $action = $event->arguments[0];

        // find all fields of type FieldtypeImage that are part of the page we're using
        // or regular image fields with InputfieldImage inputfield assigned
        $imageFields = array();
        foreach($page->fields as $f) {
            if ($f->type instanceof FieldtypeImage || ($f->inputfieldClass && $f->inputfieldClass == 'InputfieldImage')) {
                $imageFields[] = $f;
            }
        }

        // loop through to find the one we're looking for
        foreach($imageFields as $imageField) {

            // good to get unformatted in case it's a single image field,
            // because it'll still be an array rather than 1 image
            $pagefiles = $page->getUnformatted($imageField->name);          // TODO: name to lowercase ???

            // if the image's pagefiles property matches the one with the
            // field we're looking at, we have a match. save in $field
            if ($image->pagefiles === $pagefiles) {
                $field = $imageField->getInputfield($page);
                break;
            }
        }

        if ($field) {
            //$event->return = $out;
            return $field;
        }

        return null;
    }

 

with this:

    public function _getInputFieldInstance(HookEvent $event) {

        if ($event->object->pagefiles->field) {
            //$event->return = $out;
            return $event->object->pagefiles->field;
        }

        return null;
    }  

 

then it always returns the correct field for the pagefile and it works with Custom Upload Names.

Is there a situation you know of where my version won't work?

[horst]

@adrian: this is taken over from the old legacy thumbnail module. But, if I remember right, it is also present in core file and image modules.

I need to test this.

[adrian]

4 minutes ago, horst said:

@adrian: this is taken over from the old legacy thumbnail module. But, if I remember right, it is also present in core file and image modules.

I need to test this.

 

Ryan added this back in Oct 2013 after the old thumbnails module was released - see his note here: 

Quote

You can now retrieve the Field object that is part of the Pagefiles/Pagefile in the same way you can retrieve the $pagefile->page, by accessing $pagefile->field (accessible from the pagefile or from the pagefiles array).

 

[ukyo]

@adrian @horst I test this replacement on my side and tried to upload images by using CroppableImage3 input and there is no error, but i don't know is there a side effect ?

[horst]

@ukyo: have you changed back the first workaround, and then added this change as only one?

Does this work and does it also create a crop variation just with or after the upload?

Many thanks for your help here!

[horst]

@adrian: Ok, your solution wasn't present when Antti wrote the legacy module. Is it right to assume that getting the " $inputFieldInstance  at line 190" failes and it does an early return here? If so, and if it works for you and @ukyo this way, I will push this to the Github repo.

[ukyo]

@horst Step by step what i made :

1- removed module folder completely and copied downloaded module (from github) files to module folder

2- i replaced @adrian codes

3- Admin > Modules > Refresh

5- uploaded images to page

4- when i over mouse to my crop variation button there is no preview image for cropped version

6- after save page i can see cropped image on preview, when i hover crop variation button

[adrian]

1 minute ago, horst said:

Ok, your solution wasn't present when Antti wrote the legacy module.

Exactly! No judgement intended for you not using having used it  

2 minutes ago, horst said:

Is it right to assume that getting the " $inputFieldInstance  at line 190" failes and it does an early return here?

Yes, that is where it was failing and returning null. 

[tpr]

It's always the legacy code!

[adrian]

@horst - you can also simply use: $event->object->field 

There is no need for $event->object->pagefiles->field

You might even want to get rid of that _getInputFieldInstance function completely - doesn't really seem necessary anymore?

[adrian]

Just a follow up on the issue of Tracy suppressing these notices. I have posted a Github issue here: https://github.com/nette/tracy/issues/233

Note that you will see the actual original error message if you have your browser dev console open, although obviously this is not ideal and I really want it shown in the Errors panel on the AJAX bar in Tracy.

[adrian]

Hi @ukyo and @horst - just another follow up on the suppression of notices/warnings by Tracy. I have discovered that it's because PW image and file uploads use vanilla JS calls that don't use xhr.getAllResponseHeaders(); This prevents Tracy from being able to update its AJAX debug bar with any notices/warnings returned from the AJAX request. I have just posted an issue (https://github.com/processwire/processwire-issues/issues/137) in the hopes that Ryan will consider adding xhr.getAllResponseHeaders(); to both of these. I have tested here and it works great and I actually think it will be of great benefit to beginners debugging image/file upload problems who aren't used to looking for AJAX errors in the Network panel of the browser dev console because now any notices/errors will appear very obviously on the Tracy AJAX debug bar.

If you're willing, I'd love a show of support for this change on that Github issue.

Thanks!

[adrian]

Ryan just added those xhr.getAllResponseHeaders() calls to the dev branch!

Keep in mind that Tracy will still remove warning/notices from the AJAX response, so uploads won't be stalled but now you'll at least be notified about the problem.