popup grows larger with each pop

Actually, you're missing a close brace in your patch. It should have another } to close both 'if's.

this.moveTo(px); 
   if (!this.autoSize) { 
   if (!this.autoSize && !this.sizeAlreadySet) { 
      this.setSize(this.size); 
      this.sizeAlreadySet = true; 
   }} 
   this.setBackgroundColor(); 
   this.setOpacity(); 

The comment on 6/20 seems wrong, there aren't two conditionals here, it seems like he just didn't understand that this is a patch / how patches work.

patch to Popup.js, Anchored.js, and tests

The new patch is an alternative solution to the problem.

It corrects the ambiguity of the size property on popups with an additional property, contentSize. 'size' is not set until the popup is drawn.

Anchored.js is adjusted to reflect this change.

Tests are modified to expect contentSize, not size, to be set on initialization.

Pushing this at Erik

excellent patch, seb. thanks for taking this on. solution makes code cleaner and more accessible for everyone.

(In [7647]) Fix for non auto-size popups that were progressively expanding each time they were opened. a little investigation showed us that there was some unfortunate ambiguity with the 'size' property, which was alternatively being used as 'size' and 'contentSize'. cheers to seb for an elegant solution r=cr5,me (Closes #1586)

(In [7656]) backing out patch for r7647 as it breaks framedcloud popups. (See #1586)

fixes the issue with framed cloud popups by backing out of the statement if this.size is not defined -- the func was getting called twice anyways for each new addition.

i still want to run tests on this but i think it's all fixed. too late to think straight

sending to you, seb. check out the small change in the updateBlocks() function.

i am going to rework this patch to fit in with the recent changes to popup.js

updated version of this patch

all tests pass ff2 and ie7.

if it's not readily clear, this patch basically corrects for the longstanding confusion between the 'size' and 'contentSize' property. The latter being what users interact with, the former being an internal deal that handles frames and padding and borders, etc.

please review

To me it feels like we have an unwanted API change here. What was size in 2.6 is now contentSize. But maybe I am missing something. He not, you could e.g. name the properties size and popupSize. Apart from that, the patch looks good.

thanks for the comment, andreas. it's a valid concern... but actually I think it depends on the definition of 'API change'. In fact, these changes will not break anyone's code... we are merely renaming a parameter to be more faithful to what it actually is.

The problem is that all this time, people have really been specifying the content size with the parameter that they pass into the function. Instead of confusing it with the 'size' property on the popup (which is *not* the same thing) we rename it to contentSize.

Ok, that makes sense. And I agree that your new naming is more intuitive than the old one. I think this is good to commit then, given that the unit tests pass.

(In [7897]) Fixing the sizing issues with the popups -- all this time, the 'size' parameter we were passing in to init a popup has been the *content* size, not the actual size of the popup. This confusion has stemmed from the fact that we weren't ever differentiating between the two. Now we are. Rename that parameter 'contentSize' in all the constructors. NOTE that this does not *break* API, it is merely renaming a variable. As such, no one should see any difference... other than the fact that autosized popups now stay their correct size when opened and closed repeatedly. Big thanks to jpulles for finding this bug and filing an accurate, informative, and detailed bug report. Thanks to ahocevar and crschmidt for reviewing. (Closes #1586)