? ? Pending

User tests: Successful: Unsuccessful:

avatar okonomiyaki3000
okonomiyaki3000
12 Jun 2019

Summary of Changes

  • use jquery's prop function instead of attr
  • properly and consistently encode and decode URI components
  • explicitly use global variables from the global scope

Why do this?

Well, first of all, you should almost always use the prop function instead of attr. What's the difference? One easy way to see is open up your dev tools dom inspector and look at the affected element (for example the action attribute of some forms). When being set with attr, this does not change. When set with prop, it does.

There is a correct way to encode and decode URI components and it does not involve writing your own custom regular expressions.

It's not a good idea to set variables on the global scope and then access them inside this file but, if you're going to do that, at least do it explicitly.

One more thing... Some users have been experiencing an intermittent problem where uploading a file to some nested folder in the media manager actually just sends it to the media base folder. It doesn't happen every time and I haven't been able to duplicate it personally but it does happen. I can't say for sure that these changes will fix the issue but they could be related. Has anyone else experienced this?

Testing Instructions

Use the media manager and popup image manager to navigate around your file system.

Expected result

As usual.

Actual result

As usual.

Documentation Changes Required

avatar okonomiyaki3000 okonomiyaki3000 - open - 12 Jun 2019
avatar okonomiyaki3000 okonomiyaki3000 - change - 12 Jun 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 12 Jun 2019
Category JavaScript
avatar bayareajenn
bayareajenn - comment - 13 Jun 2019

The issue with uploading images and having them go to the root of /images was happening before 3.9.8. I think perhaps it was after 3.9.6 that the issue began.

What happens is the first time you go to Content -> Media and navigate to a directory within /images (say /images/blog) and upload an image it will work and put it in the correct place. If you go to another place within Media and upload an image, it saves to the root of /images.

If you leave Content -> Media and go to say Content -> Articles or something and then go back to Media, it will work once. Then it saves to the root again.

But I'm not sure I want a whole pop up thing just to resolve that issue. Not even sure how to test it but I'll figure it out later this afternoon or tomorrow when I have time to do more testing.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25184.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 14 Jun 2019

@bayareajenn Thank you! That is a great clue! You see, when navigating around, only an iframe gets reloaded, the whole page doesn't and the url of the page doesn't change. However, when uploading, you send a post request and get redirected back to the same url + ?folder=wheverver/you/were. I now suspect that the 'folder' part of the url may have something to do with this problem. At least it's something I can test. Of course my users never tell me some useful information like they had uploaded something else previously etc...

avatar BPBlueprint
BPBlueprint - comment - 14 Jun 2019

I checked the 4 files "Improvements to mediamanager.js and popup-imagemanager.js" (and the .min.js) from Jun 12, 2019, but no effect.

Still the same problem.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 14 Jun 2019
avatar BPBlueprint
BPBlueprint - comment - 14 Jun 2019
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 14 Jun 2019

@ https://issues.joomla.org/tracker/joomla-cms/25192 ?
It´s marked as "closed"

Its the Issue and get closed if there is a Pull Request. So please mark the Pull Request.

avatar BPBlueprint
BPBlueprint - comment - 14 Jun 2019

@ https://issues.joomla.org/tracker/joomla-cms/25192 ?
It´s marked as "closed"

Its the Issue and get closed if there is a Pull Request. So please mark the Pull Request.

Sorry, I do this for the first time...
Recording the test results (https://docs.joomla.org/Testing_Joomla!_patches#Recording_test_results) seems to be within a Joomla!-environment.
The Pull Request seems to be at GitHub (#25184).
So I donot find where/how to mark my test as unsuccessfully.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 14 Jun 2019

Please mark https://issues.joomla.org/tracker/joomla-cms/25184 as unsuccessfully. How to do (please read the doc if needed): https://docs.joomla.org/Testing_Joomla!_patches

avatar BPBlueprint
BPBlueprint - comment - 14 Jun 2019

I have tested this item 🔴 unsuccessfully on 5722fbe

I checked the 4 files "Improvements to mediamanager.js and popup-imagemanager.js" (and the .min.js) from Jun 12, 2019, but no effect.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25184.

avatar BPBlueprint
BPBlueprint - comment - 14 Jun 2019

I have tested this item 🔴 unsuccessfully on 5722fbe

I checked the 4 files "Improvements to mediamanager.js and popup-imagemanager.js" (and the .min.js) from Jun 12, 2019, but no effect.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25184.

avatar BPBlueprint BPBlueprint - test_item - 14 Jun 2019 - Tested unsuccessfully
avatar okonomiyaki3000
okonomiyaki3000 - comment - 14 Jun 2019

These are legit improvements whether or not they fix that bug.

avatar bayareajenn
bayareajenn - comment - 14 Jun 2019

I have tested this item successfully on 5722fbe

I have applied the patch and navigated around the Media section of the site and all works fine. I'm not able to upload an image to the proper directory, but this has nothing to do with the patch. It's a different bug that this patch isn't meant to solve.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25184.

avatar bayareajenn bayareajenn - test_item - 14 Jun 2019 - Tested successfully
avatar bayareajenn
bayareajenn - comment - 14 Jun 2019

@okonomiyaki3000 you might want to edit your original "summary of changes" so that it doesn't include anything about maybe fixing a bug. It doesn't and I don't want it to detour people from testing and giving it an "unsuccessful" result for something it wasn't intended to do. :)

avatar Quy
Quy - comment - 14 Jun 2019

I have tested this item successfully on 5722fbe


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25184.

avatar Quy Quy - test_item - 14 Jun 2019 - Tested successfully
avatar Quy Quy - change - 14 Jun 2019
Status Pending Ready to Commit
avatar Quy
Quy - comment - 14 Jun 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25184.

avatar okonomiyaki3000 okonomiyaki3000 - change - 17 Jun 2019
The description was changed
avatar okonomiyaki3000 okonomiyaki3000 - edited - 17 Jun 2019
avatar okonomiyaki3000
okonomiyaki3000 - comment - 17 Jun 2019

@bayareajenn You're right. I've crossed that part out. An, following up on your clue, I still wasn't able to replicate the issue. It only seems to affect some users sometimes. Is it happening consistently for you?

avatar okonomiyaki3000 okonomiyaki3000 - change - 17 Jun 2019
Labels Added: ? ?
avatar Quy
Quy - comment - 17 Jun 2019

@okonomiyaki3000 Please read this comment for clarification: #25192 (comment)

avatar okonomiyaki3000
okonomiyaki3000 - comment - 21 Jun 2019

@Quy Thanks! I'll see what I can do about that.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 21 Jun 2019

I believe I have a fix.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 21 Jun 2019

These changes seem to fix it for me. I was finally (maybe) able to duplicate the problem by putting a sleep(10) in the medialist view class. That caused the iframe to load slowly and if I uploaded anything before it finished, the upload would go to the root. After these changes, things went to the right directory.

I had to kind of artificially cause the bug to occur so, if some of you are able to get it in a more consistent way, please test this and see if it solves the problem for you.

avatar Quy Quy - change - 21 Jun 2019
Status Ready to Commit Pending
avatar bayareajenn
bayareajenn - comment - 21 Jun 2019

It's not perfect, but it's better. I have one directory where it still saves it in not the place intended. Perhaps my directory is corrupted somehow because in other directories it works.

I had one instance where I uploaded an image to /images/blog and that worked. But when I went into a different subfolder /images/blog/2019 it said the file already existed. Which technically it did but not with that path.

Then I tried uploading an image to just /images/blog and it saved it in images/blog/2019 even though I wasn't there.

But when I did the same thing in images/sampledata with an image and then images/sampledata/parks with the same image, it worked. Thus maybe something is wrong with my folder. So I wasn't sure whether to mark this as an unsuccessful test or successful. Because it's sure a heck of a lot better than what was happening before.

avatar franz-wohlkoenig franz-wohlkoenig - change - 21 Jun 2019
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 21 Jun 2019

Readded RTC cause bot removed it without a Reason.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 24 Jun 2019

@bayareajenn Thanks for the details. I'll try that out. By the way, is your 'images' folder configured to be the same as your files folder or some subfolder of it? I'm not sure if it's important, just want a complete picture.

avatar bayareajenn
bayareajenn - comment - 24 Jun 2019

@bayareajenn Thanks for the details. I'll try that out. By the way, is your 'images' folder configured to be the same as your files folder or some subfolder of it? I'm not sure if it's important, just want a complete picture.

No, the images folder isn't configured to be the same as some subfolder of it. :)

avatar HLeithner HLeithner - change - 2 Jul 2019
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-07-02 07:29:02
Closed_By HLeithner
avatar HLeithner HLeithner - close - 2 Jul 2019
avatar HLeithner HLeithner - merge - 2 Jul 2019
avatar HLeithner
HLeithner - comment - 2 Jul 2019

thx

Add a Comment

Login with GitHub to post a comment