Skip to content
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

[#33965] Update the ImageManager js object #3919

Closed
wants to merge 5 commits into from

Conversation

okonomiyaki3000
Copy link
Contributor

  • No dependence on Mootools
  • Support both jQuery and Mootools version of Chosen (whichever is
    installed will just work)
  • Remove/consolidate unneeded/repetitious code
  • Fix bugs such as uninitialized variables

Tracker: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33965&start=0

test instructions

This file is used only by the 'images' view of com_media which is used inside an iframe in a modal dialog when the 'Image' button is clicked under an editor. So this can be tested on any page that uses an editor such as the 'Edit Article' page. The script manages such things as changing directories (using the select or the 'up' button) and it handles generating the img tag (or figure tag set) and inserting into the editor. So you can test that those functions are behaving as usual.

@okonomiyaki3000 okonomiyaki3000 changed the title Update the ImageManager js object [#33965] Update the ImageManager js object Jul 18, 2014
@okonomiyaki3000
Copy link
Contributor Author

Tracker has some test instructions.

@brianteeman
Copy link
Contributor

I added the test instructions from joomlacode to the original post

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

@ronnikc
Copy link

ronnikc commented Oct 17, 2014

Tested and is working generated this code:

Test me

Test me

URL to testbed: http://gothard.dk/joomla/index.php?option=com_content&view=article&id=1:getting-started&catid=2:uncategorised&Itemid=101

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

@ronnikc
Copy link

ronnikc commented Oct 17, 2014

The code was ofcause filtered out - check it out on the testbed link.

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

@AnishaTailored
Copy link

@test,
This works when adding the image for the first time, When i re-edit the article already containing the image, one javascript error is coming when i open image modal by clicking on the "image" button.
It does not hamper the functionality, but error is coming so attaching the screen-shot of the error here.screen shot 2014-10-17 at 05 14 31

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

@tairedweb
Copy link

@test it happen when i'm double-click on folder in frame
screen shot 2014-10-17 at 05 22 02

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

@superfoght
Copy link

I see the same issues as AnishaVora and tairedweb



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

@okonomiyaki3000
Copy link
Contributor Author

Thanks guys. I'll try to have a look at to these issues next week.

@anibalsanchez
Copy link
Contributor

@test OK, but:

TypeError: u is undefined
.../)?(([^:\/?#])(?::(\d))?)((/(?:^?#)_/?)...


_This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3919.*

@okonomiyaki3000
Copy link
Contributor Author

I can't seem to duplicated these errors in chrome. I'll try other browsers. What browsers are y'all using?

@okonomiyaki3000
Copy link
Contributor Author

Nevermind, I'm finding them now.

@okonomiyaki3000
Copy link
Contributor Author

This ought to fix it. Do you ever look at some code you wrote a while ago and think: how did this even work at all?

@anibalsanchez
Copy link
Contributor

@test, tested on Joomla 3.3.6, 'Edit Article'

  • Changing directories OK
  • Using the select or the 'up' button, OK
  • Select a directory, FAILED
ReferenceError: com_content is not defined
ImageManager.setFolder(this.options[this.selectedIndex].value, com_content, 55)
  • Generating the img tag (or figure tag set) and inserting into the editor, OK

@okonomiyaki3000
Copy link
Contributor Author

@anibalsanchez the 'Select a directory' bug is not related to this file (try it on a clean install, you'll see) so I'd prefer not to include it in this PR. The bug is actually in MediaModelManager (administrator/components/com_media/models/manager.php). It's a very easy fix:

$list = JHtml::_('select.genericlist', $options, 'folderlist', 'size="1" onchange="ImageManager.setFolder(this.options[this.selectedIndex].value, '.$asset.', '.$author.')" ', 'value', 'text', $base);

should change to

$list = JHtml::_('select.genericlist', $options, 'folderlist', 'size="1" onchange="ImageManager.setFolder(this.options[this.selectedIndex].value, ' . json_encode($asset) . ', ' . $author . ')" ', 'value', 'text', $base);

I'll put in a different PR for that one.

@okonomiyaki3000
Copy link
Contributor Author

Here #5077

@anibalsanchez
Copy link
Contributor

@test Ok with #3919 and #5077

@brianteeman brianteeman removed the PBF14 label Jan 1, 2015
@Chalkin
Copy link

Chalkin commented Mar 6, 2015

@test
Joomla 3.4.0
Google Chrome 40.0.2214.111 Mac
with com_articles - New Article

OK:

  • Browsing through folders -> OK
  • Using "UP" Button -> OK
  • Selecting Image -> OK
  • Image Path is applied to field "Image URL" after selecting -> OK
  • Selecting Image and Inserting into Joomla Article -> OK

Error:

@okonomiyaki3000
Copy link
Contributor Author

@Chalkin is this not fixed by #5077 ?

@Chalkin
Copy link

Chalkin commented Mar 7, 2015

@okonomiyaki3000 the fix from #5077 is already applied in joomla 3.4.0 - but it's not fixing the problem with the current directory not showing up.

Specific the fixed line in #5077 I'm refering to:
administrator/components/com_media/models/manager.php Line 104

$asset = htmlspecialchars(json_encode(trim($input->get('asset', 0, 'cmd'))));

@peterpeter
Copy link
Contributor

In this modal window, when I switched in a subfolder and upload an image it is allways stored in the images root folder, no matter in wich subfolder I actually am. This happend with and without this patch in J-3.4. As this behaviour is not in J-3.3.6, it seems to be a 3.4 bug.

@okonomiyaki3000
I've searched a while and found this bug not reported so far, so you can smash another bug with this PR.


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

@roland-d
Copy link
Contributor

@okonomiyaki3000 Could you please have a look at the issues mentioned? Thank you.


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

@okonomiyaki3000
Copy link
Contributor Author

Rebased with latest staging but couldn't reproduce the bug. Everything is working perfectly in Chrome. @Chalkin @peterpeter , what browsers are you guys using?

@okonomiyaki3000
Copy link
Contributor Author

Ah, you are using Chrome. Well, I can't imagine why you got this error. Any messages in Chrome's console?

@Chalkin
Copy link

Chalkin commented Aug 25, 2015

I'm using chrome on mac (current chrome) - I will check in the evening if the problem still exists since it's been a while.

@peterpeter
Copy link
Contributor

I've used FF on windows....I check it later on too, with different browsers

@anibalsanchez
Copy link
Contributor

I have reproduced the reported issue.

If you navigate to a subfolder and upload an image, it works Ok.

However, if you choose a subfolder from "Directory", it shows the subfolder. But, when you upload an image, it is saved in / and jumps to /.

@okonomiyaki3000
Copy link
Contributor Author

Thanks! That might help. I'll try it tomorrow.

@okonomiyaki3000
Copy link
Contributor Author

Thanks for your help guys, this should fix it. The problem was that, in some cases a url was getting double encoded. There's an iframe that contains the current directory list and it gets changed when changing directories. This can happen by a link (clicking a folder in the list) which worked fine because it was a perfectly "natural" process. It can also happen by javascript (a result of using the folder select or 'up' button). The javascript way had a problem because I was forming the query string part of the url by using JQuery's $.param() function. That function is pretty convenient and even does url encoding automatically. You'd think that would be perfect but then the next step is to assign the url to frame.location.href. You might not know this (I didn't) but assigning a url to location.href also automatically url encodes. So this was resulting in a double encoding. Specifically, the / character was getting encoded as %2F and then that was getting encoded as %252F which is obviously a mess that makes no sense at all.

So now it's fine.

@anibalsanchez
Copy link
Contributor

@text OK

@okonomiyaki3000
Copy link
Contributor Author

conflicts, eh? We'll see about this...

- No dependence on Mootools
- Support both jQuery and Mootools version of Chosen (whichever is
  installed will just work)
- Remove/consolidate unneeded/repetitious code
- Fix bugs such as uninitialized variables
@okonomiyaki3000
Copy link
Contributor Author

Rebased with staging. Should merge now.

@okonomiyaki3000
Copy link
Contributor Author

@anibalsanchez I think you meant to say @test OK

@anibalsanchez
Copy link
Contributor

Ahhh fat fingers!!! @test OK

@coolcat-creations
Copy link
Contributor

I have tested this item ✅ successfully on 95a3099

Tested successfully on Chrome / Os X


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

@zero-24
Copy link
Contributor

zero-24 commented Oct 24, 2015

@okonomiyaki3000 can you fix merge conflicts? Than we can RTC based on @designbengel and @anibalsanchez 's tests 😄

@dgrammatiko
Copy link
Contributor

This can be easily fixed by a commiter:
remove JHtml::_('script', 'media/popup-imagemanager.min.js', true, true); from administrator/components/com_media/views/images/view.html.php and push just the scripts

@roland-d roland-d closed this in 56c4a3f Nov 3, 2015
@okonomiyaki3000 okonomiyaki3000 deleted the imagemanager branch April 20, 2016 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.