Posted by sirkitree on January 22, 2009 at 11:25pm
7 followers
| Project: | Popups API (Ajax Dialogs) |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Issue Summary
I've upgraded to 1.3.1 and just wanted to inform all that I've seen nothing but faster speeds with Popups. Good stuff!
Comments
#1
Thanks for the feedback!
I was just updating the D7 jQuery 1.3.1 patch.
cheers
#2
Automatically closed -- issue fixed for 2 weeks with no activity.
#3
Since D7 core now has jQuery 1.3.2 I did a little testing again on this newest version.
It seem there is a difference in how $('script') is evaluated. This is a big deal to Popups.addInlineJs() where you do
...$('script:not([src]):contains...Not sure what the fix here will be but wanted to make a note of it.
--
I first noticed this because after upgrading to jQuery 1.3.2 I noticed that there were a LOT more js files (like all of them) being loaded when clicking a popup link.
#4
Drat.
#5
A little more research here.
It seems that within an ajaxOptions:success(json) - the json.js object no longer has an 'inline' type. Instead there is 'core', 'module' and 'setting'.
So in Popups.addJS() when we pass json.js into this we can't do a detection for type == 'inline'
#6
Here is a patch with some very basic changes that seem to make this work with jQuery 1.3.2
#7
I don't think this is actually needed, now that I've been able to play with it a bit more. The installation i was originally using was not completely patched to work with jquery 1.3.2 and hence when popups was pulling in the script to eval() it was erring out on them because the scripts being pulled in were not themselves compatible. Now that I am on an installation where everything else is working well with jquery 1.3.2, these patches are uneccessary -
I will post some updates as soon as i get everything working. Sorry for any unnecessary chatter.
#8
Ok so the only thing that is really needed here is the following change:
Where scripts are added:
...if (!$('script[src='+ src + ']').length && !Popups.addedJS[src]) {
...
to
...if (!$('script[src*='+ src + ']').length && !Popups.addedJS[src]) {
...
This change seems to successfully disallow the re-eval() upon success of the $.ajax:success
#9
Wheee! This change was covered in http://drupal.org/node/224333#jquery_13 . Is this what jQuery Update does in Drupal 6?
#10
This is slightly different. We're not having to deal with the @ sign, just explicit vs wildcard. - in any case, I'm wondering if that change in behaviors mentioned at that link can be applied; the whole { attach: function(context, settings) ... thing. I have a feeling since the setting are being overwritten in popups that might be what is causing this issue: #388406: multiple values nodereference field
#11
Ok, so here is the definitive changes that need to be made to make popups work with jQuery 1.3.2
Index: popups.js
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/popups/popups.js,v
retrieving revision 1.9.8.12
diff -u -p -r1.9.8.12 popups.js
--- popups.js 21 Mar 2009 00:57:15 -0000 1.9.8.12
+++ popups.js 15 Apr 2009 16:37:05 -0000
@@ -759,7 +759,7 @@ Popups.addJS = function(js) {
for (var i in scripts) {
var src = scripts[i];
- if (!$('script[src='+ src + ']').length && !Popups.addedJS[src]) {
+ if (!$('script[src*='+ src + ']').length && !Popups.addedJS[src]) {
// Get the script from the server and execute it.
$.ajax({
type: 'GET',
@@ -789,7 +789,7 @@ Popups.addInlineJS = function(inlines) {
// Load the inlines into the page.
for (var n in inlines) {
// If the script is not already in the page, execute it.
- if (!$('script:not([src]):contains(' + inlines[n] + ')').length) {
+ if (!$("script:not([src*='" + inlines[n] + "'])").length) {
eval(inlines[n]);
}
}
#12
works like a charm, thank you, sirkitree!
#13
Patch from #11 works for us too!
#14
I'd suggest trying #468218-19: Compatibility with jQuery Update ($form.ajaxForm is not a function) and #436094-20: Optimize JavaScript files = breaks POPUPS module. We're using the combination with jQuery update and it works great. Also allows CSS and JS aggregation.
Update: Fixed the first link.
#15
Seems like we need the attached to get it working with jquery_update.
#16
I take that back. That totally doesn't work right. If the text in inlines has quotes it totally breaks the contains() and kills the browser.