Posted by fred0 on March 26, 2009 at 8:05pm
5 followers
| Project: | Popups API (Ajax Dialogs) |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
Now that I figured out a workaround for my previous issue (with google analytics) I have the alpha installed and am seeing another problematic behavior. Namely, if click on the popup link once, the window pops up as expected. I then close the popup and *without doing anything else* click on the link again. This time I get the loading spinner indefinitely. I've cleared the Drupal cache, browser cache and quit the browser and tried again with the same result.
Comments
#1
Following up, this occurs specifically when specifically when Optimize Javascript files is turned on under Performance.
Optimize CSS and Page Cache (Normal) slow down the popup and sometimes make the underlying page jitter, but the popup does appear.
#2
Ok, I give up. Besides the above, 2.x-dev totally freaks out IE with my theme.
Went back to 1.3. At least it works and the only conflict is the maintenance mode.
#3
duped msg
#4
This problem appears to be fixed in the -dev version of the module.
#5
seem okay?
#6
It's ok in the -dev version. I've not tried the current release version since I had the problem.
#7
OK, I just did some testing. My April 21 version of the -dev version works fine. But the June 9 version of the -dev version is broken just like the beta0 version. By "broken" I mean that the second time the popup is loaded, it does not popup, but rather takes the user to the page containing the popup.
As the original post says: Namely, if click on the popup link once, the window pops up as expected. I then close the popup and *without doing anything else* click on the link again. This time I get the loading spinner indefinitely.
Except for me, it does not spin indefinitely. Instead, it takes the user to the source page of the popup text.
So I need to stick with the April 21 version of the -dev version. Can/should this be fixed?
#8
I'll change this status to active since it's now not fixed in the most recent dev version.
#9
Having exactly the same issue... hopefully it's an easy fix :)
#10
Quartercreative: are you, like me, seeing that the April 21 version works as it should but that the most recent dev version does not? --Dan
#11
Hey Dan,
I've only got the April 22 'Beta0' running... it's definitely a problem in that version. Where do you get the April 21'dev' version?
I'll give it a whirl if I can get it :)
#12
I can't seem to find that particular -dev version I have, but clearly in my test, an older one worked and a newer one did not.
http://drupal.org/node/199723/release
Try that top beta there dated April 21.
I'm not sure why this bug is getting no attention from the developers. I guess everyone else just doesn't cache the java script. I don't like to complain when I'm using someone else's work for free. On the other hand, you'd think they'd want it to work properly.
If all else fails, I can provide a link for you to use my -dev version. Seems silly though.
Dan
#13
hmmmm.... that one still has the problem. If you could, chuck us a link to the version you have (that works) and I'll give that a run. Yeah, in a perfect world everyone would jump at our every wish, but until then all we can do is talk it through and hopefully we gain a bit of traction on the fault :)
Thanks mate.
#14
Sure . . . here you go:
https://www.sugarsync.com/pf/D6642931_7142144_729458
If you are able to use -diff or something to compare versions, perhaps someone can locate the line of code that is causing the issue. Let me know how this works out.
Best,
Dan
#15
Ok, this gets stranger still... the version you sent me works, on the second 'click' the popup works. BUT that second click triggers a problem with the layout of the page behind it. So I'm thinking I will need to go through and analyse the differences between the two and try to find the happy medium :)
Keep you posted.
#16
OK, just for the record, I am caching my CSS and my javascript in the Performance settings.
I believe that neither of us have the problem with things opening if we don't cache our javascript. But that needs to be cached on my site.
I'm curious, is your site public?
My use of it is here:
http://www.esplanner.com/our-products
I assume my page does not have the problems you are seeing on yours. I hope not!
Dan
#17
Sorry about my disappearance, but I a new arrival last week (baby)... so I'm kind of burning the candle at both ends right now.
The site I'm using the module on is not public yet but I may get it up shortly. I have a feeling that my limited coding prowess will prevent me from solving this one but I'm still hopeful :) BUT, i think by doing the following on the more recent versions, we may achieve something.
On line 24 of the popups.js file change the line:
var Popups = function(){};
to:
Popups = function(){};
This actually gets my version to open twice, but on the second load I am still getting a 'messed up' layout behind the popup. Almost like it is nullifying any floats that were set up in the css????
#18
Link in comment 14 doesn't work so I can't run a diff to see what has changed from the version you all have working with caching.
This problem exists in 6.x-2.x-dev and 2.0-beta0
I think what is happening is the css and js receives a different name when it is cached as it is all aggregated into one js/css file, then when the popups module attempts to see if it has already loaded the css/js it thinks it hasn't and loads each aggregated css/js file that it needs again it also doesn't log these for removal since it thinks they weren't loaded. Watch your firebug console when you click a popup, close it and reclick it with caching on versus off to see what I mean. The second add of popups.js causes the Popups namespace to collide with itself clobbering the saved Drupal.settings in Popups.originalSettings.
Regarding the broken CSS please try the patch in #1233874: restorePage does not properly remove added css I was experiencing the same issue with caching turned off so that appears unrelated to the caching issue.
#19
Attached is a patch that resolves the issue for me, I'm not sure of the completeness of this. It seems like this case should be accounted for in Popups.addJS().
All it does is prevent popups_get_js() from returning popups.js. The assumption is that we got to popups_get_js() from popups_render_as_json() or popups_preprocess_page() and I think in both cases it is safe to remove the popups.js file because if the javascript triggers either scenario we already have popups.js loaded and therefore don't need to account for it as long as it is explicitly excluded.
I'm unsure why Popups.addJS() didn't trap popups.js from being added again, but I'm guessing there is some oddity with the Popups namespace being clobbered somehow. I think this patch is safe, but there may be a more elegant solution.
#20