The attached patch fixes the:

feature[i] is null

message when you write a non-collection in the WKT field (needed for GEO integration)

Comments

tmcw’s picture

Status:Active» Needs work

Good direction of a patch, but is_array is not a javascript function, so this introduces a new bug. $.isArray is probably useful here.

tmcw’s picture

StatusFileSize
new2.13 KB

Updated / fixed patch. Please retest.

strk’s picture

Oopos, you're right, is_array is not a function.
But also your patch fails here with the message:

Error during map rendering: $.isArray is not a function

strk’s picture

features.length != undefined

The above does it.
In js/openlayers.js another construct is used for the same purpose, which is:

typeof(newFeatureObject[0]) === 'undefined'

I guess it'd make sense to be consistent, altought the one in js/openlayers.js would
fail to detect an array of zero elements.

strk’s picture

StatusFileSize
new838 bytes

Attached is my patch using a check of features.length

strk’s picture

So, news from the core OpenLayers side.

WKT.read returns an array _only_ in the case of GEOMETRYCOLLECTION.
Still returns a single object for MULTIPOINT, MULTILINESTRING, MULTIPOLYGON.

The reason why an array is returned for GEOMETRYCOLLECTION was to make read/write
symmetric when write is passed an array (in which case always a GEOMETRYCOLLECTION
is returned).

They'd accept a patch for OL 2.0 if backward compatibility is kept.

zzolo’s picture

Please note that isArray is a jQuery 1.3+ function and Drupal comes with 1.2 (I think). So without a depenedency on jquery_update module, we cannot use this function. The actual function is just this:

isArray: function( obj ) {
return toString.call(obj) === "[object Array]";
},

But I had some errors pop up when I tried this method.

Also, note that a String type has a length property as well.

So, I went with the following:

(features.constructor == Array)

Committed:
http://drupal.org/cvs?commit=384910

strk’s picture

May I suggest, for consistency, to use the same mechanism also in js/openlayers.js ?
Current branch, line 216 reads:
if (typeof(newFeatureObject[0]) === 'undefined'){
Should be changed to
if (typeof(newFeatureObject[0]) !== 'undefined'){

zzolo’s picture

Status:Needs work» Fixed

This is actually accurate as we are trying to create an array. The first part of the if..else, says, if this is not an array, create an array with this as the first value, otherwise, simply use the array.

I feel like checking for the first object in a array is not a very good method as it already assumes its an array in the statement and could cause problems.

I would rather come up with a consistent way to do this. Unfortunately there are many ways to do this and some are better than others. A little googleing suggests there is not definite way. My thought would be to use what jQuery uses, but when I did that, I came up with errors.

I feel like this ticket is fixed. Thanks for the patch.

strk’s picture

"The first part of the if..else, says, if this is not an array, create an array with this as the first value, otherwise, simply use the array."

Not really, that code says, if the first element of the array is undefined....

Consider this WKT: GEOMETRYCOLLECTION EMPTY

That's a valid WKT. Dunno if OpenLayers is able to parse it, but it's valid, and a parser which states to return an array
of geometries from a GEOMETRYCOLLECTION WKT should teoretically return an empty array, in which case the drupal openlayers
module would think it's not an array while instead it is.

The Array constructor check is saner.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.