?

Success

User tests: Successful: Unsuccessful:

avatar dgt41
dgt41
13 Jul 2016

Pull Request for Issue #11091 .
Hopefully this is the last time I do this

Testing Instructions

Follow the instructions from the issue above:

Steps to reproduce the issue

For the sake of using SqueezeBox (MooTools based) instead of the Bootstrap Modal, change the Administrator template to Hathor. It's damn ugly, I know :)
Make sure you're using TinyMCE as your Editor.
Go to Content > Articles > Add New Article.
In the Publishing area, click on the "User" icon so you can select a user from the modal.
The modal shows up. Click outside the modal or on the X button to close it.

Also try to insert an intro image but before you do so edit
administrator/components/com_media/views/images/tmpl/default.php:60
so it become:

// This is for Mootools compatibility ?>parent.SqueezeBox.close();<?php endif ?>" data-dismiss="modal"><?php echo JText::_('JCANCEL') ?></button>

parent.SqueezeBox.close(); is the old way for the old mootools modals...
try to close the modal with the cancel button

Also try to insert an image and a module into tinyMCE

Switch to isis and repeat the whole process

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
5.00

avatar dgt41 dgt41 - open - 13 Jul 2016
avatar OctavianC OctavianC - test_item - 13 Jul 2016 - Tested successfully
avatar OctavianC
OctavianC - comment - 13 Jul 2016

I have tested this item successfully on 9729897

Tested Hathor and Isis - no more errors. Thanks.


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

avatar dgt41 dgt41 - change - 13 Jul 2016
Title
mootools close broken again for tinyMCE
REGRESSION mootools close broken again for tinyMCE
avatar dgt41 dgt41 - change - 13 Jul 2016
Title
mootools close broken again for tinyMCE
REGRESSION mootools close broken again for tinyMCE
avatar brianteeman brianteeman - change - 13 Jul 2016
Category JavaScript Plugins
avatar dgt41
dgt41 - comment - 15 Jul 2016

Kudos to @ggppdk for the code update!

avatar infograf768
infograf768 - comment - 15 Jul 2016

I now get a recursion which I did not have before.

avatar dgt41
dgt41 - comment - 15 Jul 2016

@infograf768 which browser? cache cleared?

avatar ggppdk
ggppdk - comment - 15 Jul 2016

@dgt41
please add flag checking to both files

aka add it to behaviour.php, i forgot to add it when i replied to the thread

avatar dgt41
dgt41 - comment - 15 Jul 2016

@ggppdk oh yeah, the loading order....

44671d5 15 Jul 2016 avatar dgt41 oops
avatar joomla-cms-bot
joomla-cms-bot - comment - 15 Jul 2016

This PR has received new commits.

CC: @OctavianC

avatar infograf768
infograf768 - comment - 15 Jul 2016

Nope. Same issue (Firefox, no cache at all, Mac)

avatar ggppdk
ggppdk - comment - 15 Jul 2016

@dgt41
yes it does not work
can you use the code i suggested originally for behaviour.php
?
(i have renamed the __tmp variables to __tmp1 and __tmp2, compared to original code although it is not needed)

full code should be:
[EDIT]
deleted bogus

avatar joomla-cms-bot
joomla-cms-bot - comment - 15 Jul 2016

This PR has received new commits.

CC: @OctavianC

avatar ggppdk
ggppdk - comment - 15 Jul 2016

@dgt41

Still bogus, due to wrong variable scope for temporary variables, i am making a proper fix

avatar ggppdk
ggppdk - comment - 15 Jul 2016

@dgt41

i made PR to your branch, tested it too, it should work now (i have tested),
also code now is cleaner and readable

avatar ggppdk
ggppdk - comment - 15 Jul 2016

I have not updated the minified version of the JS, can you do it ?

avatar dgt41
dgt41 - comment - 15 Jul 2016

@ggppdk do you mind is i use

document.onreadystatechange = function () {
    if (document.readyState == "interactive") {
// Script here
    }
}

instead of the jQuery ready?

avatar ggppdk
ggppdk - comment - 15 Jul 2016

yes, it seems better, and should work too

anybody wanting to test, please wait for the changes @dgt41 , please also update the minified version, i have not updated it

avatar ggppdk
ggppdk - comment - 17 Jul 2016

hi @dgt41

this is almost done, can you finish things needed for people to test it ?

  • use document.onreadystatechange instead jQuery ready, as you said
  • minify JS which i did do
  • make the non-minified JS be loaded when debug is ON ?

thanks

avatar dgt41
dgt41 - comment - 17 Jul 2016

@ggppdk the first two should be ok with f3754e3
Will do the DEBUG switch in a bit

avatar ggppdk
ggppdk - comment - 17 Jul 2016

What happened ?

you merged my PR against your branch,

but now, i see the old code, that is bogus, the latest changes are gone

avatar ggppdk
ggppdk - comment - 17 Jul 2016

i ll redo a PR against your branch

avatar dgt41
dgt41 - comment - 17 Jul 2016

@ggppdk oh crap, I did that from the github website. So is it behaviour or the script file?

avatar ggppdk
ggppdk - comment - 17 Jul 2016

@dgt41
both files, my original suggestion is bogus for 2 reasons:

  • because of variables scope of __tmp,
  • also if someone tries to use these __tmp global variable the script will break for 1 more reason,

i ll redo it later today 15 minutes to redo it, as i remember it, have a nice Sunday, thanks
[EDIT]
nothing to redo, i found my branches, i had not deleted them yet )))

avatar dgt41
dgt41 - comment - 18 Jul 2016

@ggppdk thanks for the PRs!! I recompressed the .min.js and introduced the logic for the debug to correctly picking the right file!
I think this is ready now

avatar dgt41
dgt41 - comment - 18 Jul 2016

Just a reminder that this whole mess could have been easily avoided if we had a priority control for the scripts. Wait there is PR for that #9083 (and I voted against it??? 😡)

avatar ggppdk
ggppdk - comment - 18 Jul 2016

Tested various views with bootstrap / mootools modals,
Tested a few 3rd party extensions too
Tested also without tinymce editor
Tested with multiple tinymce editors

(modal closes and HTML goes into correct editor) this PR should not effect this, but tested anyway

  • closing and auto-closing on selection works (also debug ON now loads the uncompressed file)

I will mark a successful test tomorrow

avatar ggppdk ggppdk - test_item - 19 Jul 2016 - Tested successfully
avatar ggppdk
ggppdk - comment - 19 Jul 2016

I have tested this item successfully on c9fe04e

Works see my previous comment

Really needed fix for J3.6.1


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

avatar davdebcom davdebcom - test_item - 21 Jul 2016 - Tested successfully
avatar davdebcom
davdebcom - comment - 21 Jul 2016

I have tested this item successfully on c9fe04e

Tested, resolved issues with the modal.


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

avatar brianteeman brianteeman - change - 21 Jul 2016
Status New Ready to Commit
avatar brianteeman
brianteeman - comment - 21 Jul 2016

RTC


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

avatar brianteeman brianteeman - change - 21 Jul 2016
Milestone Added:
avatar wilsonge wilsonge - reference | 98998ab - 21 Jul 16
avatar wilsonge wilsonge - merge - 21 Jul 2016
avatar wilsonge wilsonge - close - 21 Jul 2016

Add a Comment

Login with GitHub to post a comment