Pullups for 2.3:

svn merge svn.openlayers.org/trunk/openlayers/@2230 svn.openlayers.org/trunk/openlayers/@2233 svn.openlayers.org/branches/openlayers/2.3/
#480   Grid funkiness
#491   improper URL encoding of LAYERS list in WMS GetMap request
#500   layer.destroy() should remove itself from the map but not set new baselayer


git-svn-id: http://svn.openlayers.org/branches/openlayers/2.3@2234 dc9f47b5-9b13-0410-9fdd-eb0c1a62fdaf

lib/OpenLayers/Layer.js

@@ -137,10 +137,17 @@ OpenLayers.Layer.prototype = {
     /**
      * Destroy is a destructor: this is to alleviate cyclic references which
      * the Javascript garbage cleaner can not take care of on its own.
+     * 
+     * @param {Boolean} setNewBaseLayer Should a new baselayer be selected when
+     *                                  this has been removed?
+     *                                  Default is true
      */
-    destroy: function() {
+    destroy: function(setNewBaseLayer) {
+        if (setNewBaseLayer == null) {
+            setNewBaseLayer = true;
+        }
         if (this.map != null) {
-            this.map.removeLayer(this);
+            this.map.removeLayer(this, setNewBaseLayer);
         }
         this.map = null;
         this.name = null;

lib/OpenLayers/Map.js

@@ -77,6 +77,18 @@ OpenLayers.Map.prototype = {
     /** @type int */
     zoom: 0,    
 
+    /** Used to store a unique identifier that changes when the map view 
+     * changes. viewRequestID should be used when adding data asynchronously 
+     * to the map: viewRequestID is incremented when you initiate your 
+     * request (right now during changing of baselayers and changing of zooms). 
+     * It is stored here in the map and also in the data that will be coming 
+     * back asynchronously. Before displaying this data on request completion, 
+     * we check that the viewRequestID of the data is still the same as that 
+     * of the map. Fix for #480
+     * 
+     * @type String */
+    viewRequestID: 0,
+
   // Options
 
     /** @type OpenLayers.Size */
@@ -201,7 +213,9 @@ OpenLayers.Map.prototype = {
     destroy:function() {
         if (this.layers != null) {
             for(var i=0; i< this.layers.length; i++) {
-                this.layers[i].destroy();
+                //pass 'false' to destroy so that map wont try to set a new 
+                // baselayer after each baselayer is removed
+                this.layers[i].destroy(false);
             } 
             this.layers = null;
         }
@@ -344,8 +358,13 @@ OpenLayers.Map.prototype = {
      *     its own personal list of popups, removing them from the map.
      * 
      * @param {OpenLayers.Layer} layer
+     * @param {Boolean} setNewBaseLayer Default is true
      */
-    removeLayer: function(layer) {
+    removeLayer: function(layer, setNewBaseLayer) {
+        if (setNewBaseLayer == null) {
+            setNewBaseLayer = true;
+        }
+
         if (layer.isFixed) {
             this.viewPortDiv.removeChild(layer.div);
         } else {
@@ -355,7 +374,7 @@ OpenLayers.Map.prototype = {
         OpenLayers.Util.removeItem(this.layers, layer);
 
         // if we removed the base layer, need to set a new one
-        if (this.baseLayer == layer) {
+        if (setNewBaseLayer && (this.baseLayer == layer)) {
             this.baseLayer = null;
             for(i=0; i < this.layers.length; i++) {
                 var iLayer = this.layers[i];
@@ -448,6 +467,11 @@ OpenLayers.Map.prototype = {
 
                 // set new baselayer and make it visible
                 this.baseLayer = newBaseLayer;
+                
+                // Increment viewRequestID since the baseLayer is 
+                // changing. This is used by tiles to check if they should 
+                // draw themselves.
+                this.viewRequestID++;
                 this.baseLayer.setVisibility(true, noEvent);
 
                 //redraw all layers
@@ -743,6 +767,9 @@ OpenLayers.Map.prototype = {
                 for (var i = 0; i < this.popups.length; i++) {
                     this.popups[i].updatePosition();
                 }
+
+                // zoom level has changed, increment viewRequestID.
+                this.viewRequestID++;
             }    
             
             var bounds = this.getExtent();

lib/OpenLayers/Tile/Image.js

@@ -34,6 +34,7 @@ OpenLayers.Tile.Image.prototype =
     destroy: function() {
         if ((this.imgDiv != null) && (this.imgDiv.parentNode == this.layer.div)) {
             this.layer.div.removeChild(this.imgDiv);
+            this.imgDiv.map = null;
         }
         this.imgDiv = null;
         OpenLayers.Tile.prototype.destroy.apply(this, arguments);
@@ -52,6 +53,8 @@ OpenLayers.Tile.Image.prototype =
         if (this.imgDiv == null) {
             this.initImgDiv();
         }
+
+        this.imgDiv.viewRequestID = this.layer.map.viewRequestID;
         
         this.url = this.layer.getURL(this.bounds);
   
@@ -132,6 +135,10 @@ OpenLayers.Tile.Image.prototype =
                                              null, null, null, 
                                              this.layer.opacity);
         }
+
+        // we need this reference to check back the viewRequestID
+        this.imgDiv.map = this.layer.map;
+
     },
 
     /**

lib/OpenLayers/Util.js

@@ -217,8 +217,25 @@ OpenLayers.Util.setOpacity = function(element, opacity) {
 }
 
 OpenLayers.Util.onImageLoad = function() {
-    this.style.backgroundColor = null;
+    // The complex check here is to solve issues described in #480.
-    this.style.display = "";  
+    // Every time a map view changes, it increments the 'viewRequestID' 
+    // property. As the requests for the images for the new map view are sent
+    // out, they are tagged with this unique viewRequestID. 
+    // 
+    // If an image has no viewRequestID property set, we display it regardless, 
+    // but if it does have a viewRequestID property, we check that it matches 
+    // the viewRequestID set on the map.
+    // 
+    // If the viewRequestID on the map has changed, that means that the user
+    // has changed the map view since this specific request was sent out, and
+    // therefore this tile does not need to be displayed (so we do not execute
+    // this code that turns its display on).
+    //
+    if (!this.viewRequestID ||
+        (this.viewRequestID == this.map.viewRequestID)) { 
+        this.style.backgroundColor = null;
+        this.style.display = "";  
+    }
 };
 
 OpenLayers.Util.onImageLoadErrorColor = "pink";
@@ -377,17 +394,32 @@ OpenLayers.Util.applyDefaults = function (to, from) {
 * @returns a concatenation of the properties of an object in 
 *    http parameter notation. 
 *    (ex. <i>"key1=value1&key2=value2&key3=value3"</i>)
+*    If a parameter is actually a list, that parameter will then
+*    be set to a comma-seperated list of values (foo,bar) instead
+*    of being URL escaped (foo%3Abar). 
 * @type String
 */
 OpenLayers.Util.getParameterString = function(params) {
     paramsArray = new Array();
     
     for (var key in params) {
-        var value = params[key];
+      var value = params[key];
-        if ((value != null) && (typeof value != 'function')) {
+      if ((value != null) && (typeof value != 'function')) {
-            paramsArray.push(encodeURIComponent(key) + "=" +
+        var encodedValue;
-                             encodeURIComponent(value));
+        if (typeof value == 'object' && value.constructor == Array) {
+          /* value is an array; encode items and separate with "," */
+          var encodedItemArray = new Array();
+          for (var itemIndex=0; itemIndex<value.length; itemIndex++) {
+            encodedItemArray.push(encodeURIComponent(value[itemIndex]));
+          }
+          encodedValue = encodedItemArray.join(",");
         }
+        else {
+          /* value is a string; simply encode */
+          encodedValue = encodeURIComponent(value);
+        }
+        paramsArray.push(encodeURIComponent(key) + "=" + encodedValue);
+      }
     }
     
     return paramsArray.join("&");

tests/grid_inittiles.html

@@ -11,7 +11,7 @@
     <script type="text/javascript">
         <!--
         function init(){
-            map = new OpenLayers.Map('map', {'maxResolution': 1.40625/2, tileSize: new OpenLayers.Size(256,256)});
+            var map = new OpenLayers.Map('map', {'maxResolution': 1.40625/2, tileSize: new OpenLayers.Size(256,256)});
             ww = new OpenLayers.Layer.WMS( "Basic", 
             "http://labs.metacarta.com/wms-c/Basic.py?", 
             {layers:"basic"});

tests/test_Util.html

@@ -177,7 +177,7 @@
     }
 
     function test_07_Util_getParameterString(t) {
-        t.plan( 2 );
+        t.plan( 4 );
 
         var params = { foo: "bar",
                        chicken: 1.5
@@ -185,6 +185,15 @@
 
         t.eq( OpenLayers.Util.getParameterString(params), "foo=bar&chicken=1.5", "getParameterString returns correctly");    
         t.eq( OpenLayers.Util.getParameterString({'a:':'b='}), "a%3A=b%3D", "getParameterString returns correctly with non-ascii keys/values");    
+        
+
+        // Parameters which are a list should end up being a comma-seperated
+        // list of the URL encoded strings
+        var params = { foo: ["bar,baz"] };
+        t.eq( OpenLayers.Util.getParameterString(params), "foo=bar%2Cbaz", "getParameterString encodes , correctly in arrays");    
+        
+        var params = { foo: ["bar","baz,"] };
+        t.eq( OpenLayers.Util.getParameterString(params), "foo=bar,baz%2C", "getParameterString returns with list of CSVs when given a list. ");    
     }
 
     function test_08_Util_createAlphaImageDiv(t) {