Use X, Y or I, J depending on WMS version

This also adds consistency - Pixel coordinates are now always
rounded to integers.

src/ol/source/singleimagewmssource.js

@@ -1,7 +1,6 @@
 goog.provide('ol.source.SingleImageWMS');
 
 goog.require('goog.asserts');
-goog.require('goog.uri.utils');
 goog.require('ol.Image');
 goog.require('ol.ImageUrlFunction');
 goog.require('ol.extent');
@@ -92,17 +91,9 @@ ol.source.SingleImageWMS.prototype.getFeatureInfoForPixel =
       bottomLeft = map.getCoordinateFromPixel([0, size[1]]),
       topRight = map.getCoordinateFromPixel([size[0], 0]),
       extent = [bottomLeft[0], topRight[0], bottomLeft[1], topRight[1]],
-      x = pixel[0],
-      y = pixel[1],
       url = this.imageUrlFunction(extent, size, projection);
   goog.asserts.assert(goog.isDef(url),
       'ol.source.SingleImageWMS#imageUrlFunction does not return a url');
-  // TODO: X, Y is for WMS 1.1 and I, J for 1.3 - without a closure url
-  // function this could be set conditionally.
-  url = goog.uri.utils.appendParamsFromMap(url, {
-    'X': x, 'I': x,
-    'Y': y, 'J': y
-  });
-  ol.source.wms.getFeatureInfo(url, this.getFeatureInfoOptions_, success,
+  ol.source.wms.getFeatureInfo(url, pixel, this.getFeatureInfoOptions_, success,
       opt_error);
 };

src/ol/source/tiledwmssource.js

@@ -5,7 +5,6 @@ goog.provide('ol.source.TiledWMS');
 goog.require('goog.array');
 goog.require('goog.asserts');
 goog.require('goog.math');
-goog.require('goog.uri.utils');
 goog.require('ol.TileCoord');
 goog.require('ol.TileUrlFunction');
 goog.require('ol.extent');
@@ -107,22 +106,15 @@ ol.source.TiledWMS.prototype.getFeatureInfoForPixel =
       view2D = map.getView().getView2D(),
       projection = view2D.getProjection(),
       tileGrid = goog.isNull(this.tileGrid) ?
-          ol.tilegrid.getForProjection(projection) : tileGrid,
+          ol.tilegrid.getForProjection(projection) : this.tileGrid,
       tileCoord = tileGrid.getTileCoordForCoordAndResolution(coord,
           view2D.getResolution()),
       tileExtent = tileGrid.getTileCoordExtent(tileCoord),
       offset = map.getPixelFromCoordinate(ol.extent.getTopLeft(tileExtent)),
-      x = Math.round(pixel[0] - offset[0]),
-      y = Math.round(pixel[1] - offset[1]),
       url = this.tileUrlFunction(tileCoord, projection);
   goog.asserts.assert(goog.isDef(url),
       'ol.source.TiledWMS#tileUrlFunction does not return a url');
-  // TODO: X, Y is for WMS 1.1 and I, J for 1.3 - without a closure url
-  // function this could be set conditionally.
-  url = goog.uri.utils.appendParamsFromMap(url, {
-    'X': x, 'I': x,
-    'Y': y, 'J': y
-  });
-  ol.source.wms.getFeatureInfo(url, this.getFeatureInfoOptions_, success,
-      opt_error);
+  ol.source.wms.getFeatureInfo(url,
+      [pixel[0] - offset[0], pixel[1] - offset[1]], this.getFeatureInfoOptions_,
+      success, opt_error);
 };

src/ol/source/wmssource.js

@@ -63,29 +63,36 @@ ol.source.wms.getUrl =
 
 
 /**
- * @param {string} url URL as provided by the url function, with added I, J, X
- *     and Y params.
+ * @param {string} url URL as provided by the url function.
+ * @param {ol.Pixel} pixel Pixel.
  * @param {Object} options Options as defined in the source.
  * @param {function(string)} success Callback function for successful queries.
  * @param {function()=} opt_error Optional callback function for unsuccessful
  *     queries.
  */
-ol.source.wms.getFeatureInfo = function(url, options, success, opt_error) {
+ol.source.wms.getFeatureInfo =
+    function(url, pixel, options, success, opt_error) {
+  // TODO: This could be done in a smarter way if the url function was not a
+  // closure
+  url = url.replace('REQUEST=GetMap', 'REQUEST=GetFeatureInfo')
+      .replace(ol.source.wms.regExes.layers, 'LAYERS=$1&QUERY_LAYERS=$1');
   options = goog.isDef(options) ? goog.object.clone(options) : {};
   var localOptions = {
     method: ol.source.WMSGetFeatureInfoMethod.IFRAME,
     params: {}
   };
   goog.object.extend(localOptions, options);
-  var params = {
-    'INFO_FORMAT': 'text/html'
-  };
+  var params = {'INFO_FORMAT': 'text/html'},
+      version = parseFloat(url.match(ol.source.wms.regExes.version)[1]),
+      x = Math.round(pixel[0]),
+      y = Math.round(pixel[1]);
+  if (version >= 1.3) {
+    goog.object.extend(params, {'I': x, 'J': y});
+  } else {
+    goog.object.extend(params, {'X': x, 'Y': y});
+  }
   goog.object.extend(params, localOptions.params);
   url = goog.uri.utils.appendParamsFromMap(url, params);
-  // TODO: This could be done in a smarter way if the url function was not a
-  // closure
-  url = url.replace('REQUEST=GetMap', 'REQUEST=GetFeatureInfo')
-      .replace(/LAYERS=([^&]+)/, 'LAYERS=$1&QUERY_LAYERS=$1');
   if (localOptions.method == ol.source.WMSGetFeatureInfoMethod.IFRAME) {
     goog.global.setTimeout(function() {
       success('<iframe seamless src="' + url + '"></iframe>');
@@ -101,3 +108,12 @@ ol.source.wms.getFeatureInfo = function(url, options, success, opt_error) {
     });
   }
 };
+
+
+/**
+ * @enum {RegExp}
+ */
+ol.source.wms.regExes = {
+  layers: (/LAYERS=([^&]+)/),
+  version: (/VERSION=([^&]+)/)
+};

test/spec/ol/source/wmssource.test.js

@@ -28,22 +28,22 @@ describe('ol.source.wms', function() {
 
   describe('ol.source.wms.getFeatureInfo', function() {
     it('calls a callback with a feature info IFRAME as result', function(done) {
-      ol.source.wms.getFeatureInfo('?REQUEST=GetMap&LAYERS=foo',
-          {params: {'INFO_FORMAT': 'text/plain'}},
+      ol.source.wms.getFeatureInfo('?REQUEST=GetMap&VERSION=1.3&LAYERS=foo',
+          [5, 10], {params: {'INFO_FORMAT': 'text/plain'}},
           function(info) {
             expect(info).to.eql('<iframe seamless src="' +
-                '?REQUEST=GetFeatureInfo&LAYERS=foo&QUERY_LAYERS=foo&' +
-                'INFO_FORMAT=text%2Fplain"></iframe>');
+                '?REQUEST=GetFeatureInfo&VERSION=1.3&LAYERS=foo&QUERY_LAYERS=' +
+                'foo&INFO_FORMAT=text%2Fplain&I=5&J=10"></iframe>');
             done();
           });
     });
     it('can do xhr to retrieve feature info', function(done) {
-      ol.source.wms.getFeatureInfo('?REQUEST=GetMap&LAYERS=foo', {
-        method: ol.source.WMSGetFeatureInfoMethod.XHR_GET
-      }, function(info) {
-        expect(info).to.contain('<html>');
-        done();
-      });
+      ol.source.wms.getFeatureInfo('?REQUEST=GetMap&VERSION=1.1.1&LAYERS=foo',
+          [5, 10], {method: ol.source.WMSGetFeatureInfoMethod.XHR_GET},
+          function(info) {
+            expect(info).to.contain('<html>');
+            done();
+          });
     });
   });