-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improvements to mediamanager.js and popup-imagemanager.js #25184
Conversation
- use jquery's prop function instead of attr - properly and consistently encode and decode URI components - explicitly use global variables from the global scope
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. |
@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 + |
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. |
@BPBlueprint please mark your test as unsuccessfully > https://docs.joomla.org/Testing_Joomla!_patches#Recording_test_results |
@ https://issues.joomla.org/tracker/joomla-cms/25192 ? |
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... |
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 |
I have tested this item 🔴 unsuccessfully on 5722fbe This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25184. |
These are legit improvements whether or not they fix that bug. |
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. |
@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. :) |
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. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25184. |
@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? |
@okonomiyaki3000 Please read this comment for clarification: #25192 (comment) |
@Quy Thanks! I'll see what I can do about that. |
I believe I have a fix. |
…iframe to be loaded.
…0/joomla-cms into fix-media-manager
These changes seem to fix it for me. I was finally (maybe) able to duplicate the problem by putting a 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. |
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. |
Readded RTC cause bot removed it without a Reason. |
@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. :) |
thx |
Summary of Changes
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