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)

git-svn-id: http://svn.openlayers.org/trunk/openlayers@7897 dc9f47b5-9b13-0410-9fdd-eb0c1a62fdaf

lib/OpenLayers/Popup.js

@@ -47,6 +47,12 @@ OpenLayers.Popup = OpenLayers.Class({
      */
     div: null,
 
+    /** 
+     * Property: contentSize 
+     * {<OpenLayers.Size>} the width and height of the content.
+     */
+    contentSize: null,    
+
     /** 
      * Property: size 
      * {<OpenLayers.Size>} the width and height of the popup.
@@ -183,21 +189,22 @@ OpenLayers.Popup = OpenLayers.Class({
     *               an identifier will be automatically generated. 
     * lonlat - {<OpenLayers.LonLat>}  The position on the map the popup will
     *                                 be shown.
-    * size - {<OpenLayers.Size>}      The size of the popup.
+    * contentSize - {<OpenLayers.Size>} The size of the content.
     * contentHTML - {String}          An HTML string to display inside the   
     *                                 popup.
     * closeBox - {Boolean}            Whether to display a close box inside
     *                                 the popup.
     * closeBoxCallback - {Function}   Function to be called on closeBox click.
     */
-    initialize:function(id, lonlat, size, contentHTML, closeBox, closeBoxCallback) {
+    initialize:function(id, lonlat, contentSize, contentHTML, closeBox, closeBoxCallback) {
         if (id == null) {
             id = OpenLayers.Util.createUniqueID(this.CLASS_NAME + "_");
         }
 
         this.id = id;
         this.lonlat = lonlat;
-        this.size = (size != null) ? size 
+
+        this.contentSize = (contentSize != null) ? contentSize 
                                   : new OpenLayers.Size(
                                                    OpenLayers.Popup.WIDTH,
                                                    OpenLayers.Popup.HEIGHT);
@@ -218,7 +225,7 @@ OpenLayers.Popup = OpenLayers.Class({
                                                     "hidden");
 
         var id = this.div.id + "_contentDiv";
-        this.contentDiv = OpenLayers.Util.createDiv(id, null, this.size.clone(), 
+        this.contentDiv = OpenLayers.Util.createDiv(id, null, this.contentSize.clone(), 
                                                     null, "relative");
         this.contentDiv.className = this.contentDisplayClass;
         this.groupDiv.appendChild(this.contentDiv);
@@ -310,8 +317,8 @@ OpenLayers.Popup = OpenLayers.Class({
         }
 
         this.moveTo(px);
-        if (!this.autoSize) {
+        if (!this.autoSize && !this.size) {
-            this.setSize(this.size);
+            this.setSize(this.contentSize);
         }
         this.setBackgroundColor();
         this.setOpacity();
@@ -399,13 +406,11 @@ OpenLayers.Popup = OpenLayers.Class({
      * Used to adjust the size of the popup. 
      *
      * Parameters:
-     * size - {<OpenLayers.Size>} the new size of the popup's contents div
+     * contentSize - {<OpenLayers.Size>} the new size for the popup's 
-     *     (in pixels).
+     *     contents div (in pixels).
      */
-    setSize:function(size) { 
+    setSize:function(contentSize) { 
-        this.size = size; 
+        this.size = contentSize.clone(); 
-
-        var contentSize = this.size.clone();
         
         // if our contentDiv has a css 'padding' set on it by a stylesheet, we 
         //  must add that to the desired "size". 
@@ -433,8 +438,10 @@ OpenLayers.Popup = OpenLayers.Class({
         // div itself bigger to take its own padding into effect. this makes 
         // me want to shoot someone, but so it goes.
         if (OpenLayers.Util.getBrowserName() == "msie") {
-            contentSize.w += contentDivPadding.left + contentDivPadding.right;
+            this.contentSize.w += 
-            contentSize.h += contentDivPadding.bottom + contentDivPadding.top;
+                contentDivPadding.left + contentDivPadding.right;
+            this.contentSize.h += 
+                contentDivPadding.bottom + contentDivPadding.top;
         }
 
         if (this.div != null) {

lib/OpenLayers/Popup/Anchored.js

@@ -35,17 +35,18 @@ OpenLayers.Popup.Anchored =
     * Parameters:
     * id - {String}
     * lonlat - {<OpenLayers.LonLat>}
-    * size - {<OpenLayers.Size>}
+    * contentSize - {<OpenLayers.Size>}
     * contentHTML - {String}
     * anchor - {Object} Object which must expose a 'size' <OpenLayers.Size> 
     *     and 'offset' <OpenLayers.Pixel> (generally an <OpenLayers.Icon>).
     * closeBox - {Boolean}
     * closeBoxCallback - {Function} Function to be called on closeBox click.
     */
-    initialize:function(id, lonlat, size, contentHTML, anchor, closeBox,
+    initialize:function(id, lonlat, contentSize, contentHTML, anchor, closeBox,
                         closeBoxCallback) {
-        var newArguments = new Array(id, lonlat, size, contentHTML, closeBox,
+        var newArguments = [
-                                     closeBoxCallback);
+            id, lonlat, contentSize, contentHTML, closeBox, closeBoxCallback
+        ];
         OpenLayers.Popup.prototype.initialize.apply(this, newArguments);
 
         this.anchor = (anchor != null) ? anchor 
@@ -108,9 +109,10 @@ OpenLayers.Popup.Anchored =
      * APIMethod: setSize
      * 
      * Parameters:
-     * size - {<OpenLayers.Size>}
+     * contentSize - {<OpenLayers.Size>} the new size for the popup's 
+     *     contents div (in pixels).
      */
-    setSize:function(size) { 
+    setSize:function(contentSize) { 
         OpenLayers.Popup.prototype.setSize.apply(this, arguments);
 
         if ((this.lonlat) && (this.map)) {
@@ -164,12 +166,15 @@ OpenLayers.Popup.Anchored =
      */
     calculateNewPx:function(px) {
         var newPx = px.offset(this.anchor.offset);
+        
+        //use contentSize if size is not already set
+        var size = this.size || this.contentSize;
 
         var top = (this.relativePosition.charAt(0) == 't');
-        newPx.y += (top) ? -this.size.h : this.anchor.size.h;
+        newPx.y += (top) ? -size.h : this.anchor.size.h;
         
         var left = (this.relativePosition.charAt(1) == 'l');
-        newPx.x += (left) ? -this.size.w : this.anchor.size.w;
+        newPx.x += (left) ? -size.w : this.anchor.size.w;
 
         return newPx;   
     },

lib/OpenLayers/Popup/AnchoredBubble.js

@@ -28,7 +28,7 @@ OpenLayers.Popup.AnchoredBubble =
      * Parameters:
      * id - {String}
      * lonlat - {<OpenLayers.LonLat>}
-     * size - {<OpenLayers.Size>}
+     * contentSize - {<OpenLayers.Size>}
      * contentHTML - {String}
      * anchor - {Object} Object to which we'll anchor the popup. Must expose 
      *     a 'size' (<OpenLayers.Size>) and 'offset' (<OpenLayers.Pixel>) 
@@ -36,7 +36,7 @@ OpenLayers.Popup.AnchoredBubble =
      * closeBox - {Boolean}
      * closeBoxCallback - {Function} Function to be called on closeBox click.
      */
-    initialize:function(id, lonlat, size, contentHTML, anchor, closeBox,
+    initialize:function(id, lonlat, contentSize, contentHTML, anchor, closeBox,
                         closeBoxCallback) {
         
         this.padding = new OpenLayers.Bounds(
@@ -81,9 +81,10 @@ OpenLayers.Popup.AnchoredBubble =
      * APIMethod: setSize
      * 
      * Parameters:
-     * size - {<OpenLayers.Size>}
+     * contentSize - {<OpenLayers.Size>} the new size for the popup's 
+     *     contents div (in pixels).
      */
-    setSize:function(size) { 
+    setSize:function(contentSize) { 
         OpenLayers.Popup.Anchored.prototype.setSize.apply(this, arguments);
 
         this.setRicoCorners();

lib/OpenLayers/Popup/Framed.js

@@ -85,7 +85,7 @@ OpenLayers.Popup.Framed =
      * Parameters:
      * id - {String}
      * lonlat - {<OpenLayers.LonLat>}
-     * size - {<OpenLayers.Size>}
+     * contentSize - {<OpenLayers.Size>}
      * contentHTML - {String}
      * anchor - {Object} Object to which we'll anchor the popup. Must expose 
      *     a 'size' (<OpenLayers.Size>) and 'offset' (<OpenLayers.Pixel>) 
@@ -93,7 +93,7 @@ OpenLayers.Popup.Framed =
      * closeBox - {Boolean}
      * closeBoxCallback - {Function} Function to be called on closeBox click.
      */
-    initialize:function(id, lonlat, size, contentHTML, anchor, closeBox, 
+    initialize:function(id, lonlat, contentSize, contentHTML, anchor, closeBox, 
                         closeBoxCallback) {
 
         OpenLayers.Popup.Anchored.prototype.initialize.apply(this, arguments);
@@ -188,9 +188,10 @@ OpenLayers.Popup.Framed =
      *     of the popup has changed.
      * 
      * Parameters:
-     * size - {<OpenLayers.Size>}
+     * contentSize - {<OpenLayers.Size>} the new size for the popup's 
+     *     contents div (in pixels).
      */
-    setSize:function(size) { 
+    setSize:function(contentSize) { 
         OpenLayers.Popup.Anchored.prototype.setSize.apply(this, arguments);
 
         this.updateBlocks();
@@ -296,7 +297,7 @@ OpenLayers.Popup.Framed =
             this.createBlocks();
         }
         
-        if (this.relativePosition) {
+        if (this.size && this.relativePosition) {
             var position = this.positionBlocks[this.relativePosition];
             for (var i = 0; i < position.blocks.length; i++) {
     

lib/OpenLayers/Popup/FramedCloud.js

@@ -203,7 +203,7 @@ OpenLayers.Popup.FramedCloud =
      * Parameters:
      * id - {String}
      * lonlat - {<OpenLayers.LonLat>}
-     * size - {<OpenLayers.Size>}
+     * contentSize - {<OpenLayers.Size>}
      * contentHTML - {String}
      * anchor - {Object} Object to which we'll anchor the popup. Must expose 
      *     a 'size' (<OpenLayers.Size>) and 'offset' (<OpenLayers.Pixel>) 
@@ -211,7 +211,7 @@ OpenLayers.Popup.FramedCloud =
      * closeBox - {Boolean}
      * closeBoxCallback - {Function} Function to be called on closeBox click.
      */
-    initialize:function(id, lonlat, size, contentHTML, anchor, closeBox, 
+    initialize:function(id, lonlat, contentSize, contentHTML, anchor, closeBox, 
                         closeBoxCallback) {
 
         this.imageSrc = OpenLayers.Util.getImagesLocation() + 'cloud-popup-relative.png';

tests/Popup.html

@@ -16,7 +16,7 @@
         t.ok(OpenLayers.String.startsWith(popup.id, "OpenLayers.Popup"),
              "valid default popupid");
         var firstID = popup.id;
-        t.ok(popup.size.equals(size), "good default popup.size");
+        t.ok(popup.contentSize.equals(size), "good default popup.size");
         t.eq(popup.contentHTML, null, "good default popup.contentHTML");
         t.eq(popup.backgroundColor, OpenLayers.Popup.COLOR, "good default popup.backgroundColor");
         t.eq(popup.opacity, OpenLayers.Popup.OPACITY, "good default popup.opacity");
@@ -54,7 +54,7 @@
         t.ok( popup instanceof OpenLayers.Popup, "new OpenLayers.Popup returns Popup object" );
         t.eq(popup.id, id, "popup.id set correctly");
         t.ok(popup.lonlat.equals(ll), "popup.lonlat set correctly");
-        t.ok(popup.size.equals(sz), "popup.size set correctly");
+        t.ok(popup.contentSize.equals(sz), "popup.size set correctly");
         t.eq(popup.contentHTML, content, "contentHTML porpoerty of set correctly");
 
         // test that a browser event is registered on click on popup closebox