Bug: Save & Close edited image do not save the image

[schnuti]

Steps to reproduce the issue

1. Edit an image. e.g. resize. Click on button Save & Close. Load the same image for edit. Has original size - not changed. 2. Edit an image. e.g. resize. Click on button Save. Click on button Cancel. Load the same image for edit. Has changed size.

Expected result

Changed image saved in case 1 as in case 2.

[schnuti]

No one tested this yet? I could have a look but it has first to be confirmed.

System local Windows 10, XAMPP PHP 7.1 if that could be a problem.

[schnuti]

I'm pretty sure this comes from the redirect that breaks the upload in the save task - edit-images.js. When debugging it works!

    switch (task) {
        case 'apply':
             Joomla.UploadFile.exec(name, JSON.stringify(forUpload), uploadPath, url, type);
            Joomla.MediaManager.Edit.Reset(true);
            break;
        case 'save':
            Joomla.UploadFile.exec(name, JSON.stringify(forUpload), uploadPath, url, type);
===>        window.location = pathName + '?option=com_media&path=' + fileDirectory;
            break;
        case 'cancel':

[geetanshu-m]

@schnuti I tried to reproduce this issue but I was not able to get the same result as yours. The image file got changed in each case. I am having the same configuration as your's

[schnuti]

@proman24 Thanks for testing. I gave it a new try with code version from today March 18. Still not saving the changed file after clicking on Save & Close. My guess is, as explained above, that my system for some reason is slower and the redirect breaks the save -> Joomla.UploadFile.exec.

It works in debug mode if I set a javascript breakpoint at the next line i.e the redirect -> window.location

[laoneo]

Is this another Windows issue? @proman24 did you test on Windows as well?

[schnuti]

Your question about Windows remembered me to test with another browser. And ... with Chrome it works. Maybe it's the combination of Windows and FF that is the problem.

[geetanshu-m]

@laoneo I tested this in the windows environment. Also, I have used Chrome browser.

[schnuti]

@proman24 Is it possible for you to test with Firefox? I've tried to find some hint on problems with "windows.location" in FF. So far no luck.

[geetanshu-m]

@schnuti Got the perfect results again while testing with firefox in the windows environment.

[laoneo]

@proman24 with which PHP version did you test?

[geetanshu-m]

@laoneo Tested with PHP 7.2.1 and PHP 5.6.19

[schnuti]

Not a solution but proof of problem. I made the ajax request synchronous and it works.

line 175 in edit-images.js - the false makes it

xhr.open("PUT", url, false);

I'm pretty sure that I'm not the only FF user with my configuration.

Alternative asynchronous solutions on MDN

[laoneo]

Did you try to run it with true?

[schnuti]

true (= asynchronous) is the original value i.e that's when it doesn't work with my FF.

Now I think my FF has the correct behaviour in the asynchronous world of Javascript. When searching the web I found issues with the synchronous behaviour of windows.location in browsers. e.g. if a page have a button "Go to latest Article" and a long running background request, the user has to wait for the request to end. (or some code for "Stop request" has to be added)

@proman24 Do you use Firefox 64Bit? I do.

[laoneo]

Can you try it with the latest version of FF? For me it worked as expected.

[schnuti]

@laoneo I'll test this and the new save events over the weekend.

[schnuti]

@laoneo After a fresh J!MM install, code from May 26. Using Firefox latest (normal and developer edition) with my own FF profile and the one created by the developer edition. It's the same. The resized image do not get saved. (save&close) As before: it works with chrome. Did you use Windows 10?

I found a new issue.

[laoneo]

No I was using Linux. But if the save action is working, then should also the save and close working. Really strange.

[schnuti]

I'm not that surprised. The difference is the line following the save (apply) action. As I wrote, the window.location somehow breaks the asynchronous Ajax call before the server can start the save method. It works in debug mode (with a breakpoint at those lines) or if the Ajax call is made synchronous as described above. My local client/server combination is probably to slow (or to fast? only FF?)
I've tried to somehow add a promise, callback or worker but my JS knowledge is too limited. window.location should wait until the Ajax call returns.

    case 'apply':
       Joomla.UploadFile.exec(name, JSON.stringify(forUpload), uploadPath, url, type);
       Joomla.MediaManager.Edit.Reset(true);
       break;
    case 'save':
       Joomla.UploadFile.exec(name, JSON.stringify(forUpload), uploadPath, url, type);
       ==> window.location = pathName + '?option=com_media&path=' + fileDirectory;
       break;

Most of the problems and info I've found are related to the use of JQuery but here is one link. Without date but with JS worker. The image under "Irreplaceability of the synchronous use" is interesting.

Synchronous and Asynchronous_Requests

[schnuti]

I made some tests. Chrome Developer Tools told me my system is slow and also detected [Violation] Forced reflow while executing JavaScript took 80ms. Probably Chrome handles this violation and waits for the Javascript to come to an end. I added some console.log

Console logs from Chrome, Firefox and Edge.

Chrome System is slow. [Violation] Forced reflow while executing JavaScript took 80ms cropper.min.js?... [Violation] 'resize' handler took 183ms

start ajax send ajax data before window.locaton (proves it is asynchronous) success true (returns from ajax) Navigated to "...//...option=com_media&path=local-0:/sampledata"

Firefox start ajax send ajax data before window.locaton (proves it is asynchronous) Navigated to "...//...option=com_media&path=local-0:/sampledata" error ajax (from ajax error handling)

Edge works. The HTML message could get sent before waiting for the response? start ajax send ajax data before window.locaton (proves it is asynchronous) HTML1300: User has navigated success true (returns from ajax)

I optimized apache and mySql a bit. The web server got faster but error remains.

[schnuti]

@laoneo Do you know the reason for not using Promise and Joomla.request (as is) for the requests as for upload, rename ....? The Promise would solve this issue.

    upload(name, parent, content, override) {
        // Wrap the ajax call into a real promise
        return new Promise((resolve, reject) => {
    ...
            Joomla.request({
                url: url,
                method: 'POST',

I managed to add callbacks and then it works in my tests. As my own experiment I'll try to follow the upload example and use Promise and Joomla.request instead.

Shouldn't Notifications.js be moved to media/com_media/js and in the end to media/system/js ? Reuse!

[dneukirchen]

Shouldn't Notifications.js be moved to media/com_media/js and in the end to media/system/js ?

Yes... Notifications.js is/was just an adapter for a non-existing joomla api. Our goal was to focus on media manager and not spent too much time modernizing outdated core apis. As soon as we have promise support or a good notification and request api in joomla, we can change the internals.

[schnuti]

@dneukirchen I copied a small part of Notifications into the code I edit. A nice helper. There is a colourful bug in there. I'll add an issue.

i changed the edit-images to use promises. That solves this issue. I also added progress using tag with the same name. I'll add it as a PR for review.