Support from Acquia helps fund testing for Drupal Acquia logo

Comments

augustus.kling’s picture

Basically the openlayers module used projections without authority codes before and did switch to a more complete projection representation in the meantime. For example where the projection was 4326 before, this needs to be given as "EPSG:4326" now (see also http://drupal.org/node/1331410#comment-7177682 ).

This does affect the 7.x-2.x branch of the openlayers module only. 7.x-2.0-beta3 is not affected.

Rob C’s picture

Status: Active » Needs review
FileSize
903 bytes

I've created this from the instructions provided by Pol on #comment-7177682 So nothing fancy, but at least it's a start.

I've tested this, seems to work alright.

Rob C’s picture

Forgot to update placeholder.

Sheldon Rampton’s picture

Here's a patch that works against Geofield 7.x-1.1. Is there a reason for me not to upgrade my distro to use the latest alpha of 7.x-2.0?

Sheldon Rampton’s picture

Oops, sorry. I patched against the wrong version. Here's an actual patch against 7.2-1.1. My patch also fixes the windows line endings (carriage return + Line feeds) in this file. It looks like the rest of the 7.x-1.1 release also uses the windows line endings, which is violation of Drupal coding standards as noted here: http://drupal.org/node/1875968

If Geofield 7.x-1.x is going to be maintained as a supported branch for awhile, those line endings should really be fixed for the entire project.

Rob C’s picture

Sheldon please create an individual issue for fixing these, please stick to the core issue the patch is trying to address. (makes reviewing also a bit easier and keeping track of everything)

BUT i more then agree. (also for openlayers/openlayers plus/etc/etc/etc)

so can you reroll this one more time? Thanks. (and if it should be for 1.x, it's 7.x-1.x-dev)

Rob C’s picture

Sheldon please create an individual issue for fixing these, please stick to the core issue the patch is trying to address. (makes reviewing also a bit easier and keeping track of everything)

BUT i more then agree. (also for openlayers/openlayers plus/etc/etc/etc)

so can you reroll this one more time? Thanks. (and if it should be for 1.x, it's 7.x-1.x-dev)

Sheldon Rampton’s picture

I'll reroll the patch for 7.x-1.x-dev, but there already is an issue for fixing the line breaks. It was posted in December, and I included it in my first comment on this issue:

http://drupal.org/node/1875968

My comment about fixing the line breaks should therefore be seen not as trying to expand the scope of this hyar ticket but rather of getting a fire lit under someone to address the line breaks ticket which has evidently been languishing in exile for lo three months now.

</rant>

Sheldon Rampton’s picture

Here's the patch rerolled for 7.x-1.x-dev, without fixing the line breaks.

Rob C’s picture

I guess linebreaks should be fixed at some point, but should not hold other development. Let's hope that patch gets in at some point soon, else it will only be postponed (and postponed, and...). (and i agree again)

Thanks for rolling a new patch!

jcisio’s picture

Status: Needs review » Reviewed & tested by the community

#9 works as expected. Applied for both 1.x and 2.x branches.

das-peter’s picture

Same patch without carriage returns as line endings.

Pol’s picture

Hi all,

I think we should commit this asap and release an update before seeing the issue queue spammed by the same problem !

Pol’s picture

There's something missing. A new patch is on it's way.

Pol’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
2.66 KB
augustus.kling’s picture

You want -20037508.34, -20037508.34, 20037508.34, 20037508.34 for the maxExtent for EPSG:900913/EPSG:3875. The numbers are derived from the earth's circumference by the way in these projections.

Pol’s picture

Ok I'll fix this, thanks :-)

Now the question is, do we need to set the maxExtent ourself on the map ?

Pol’s picture

augustus.kling’s picture

Status: Needs review » Needs work

I think the maxExtent can be derived from the map's projection. I will prepare a patch for the openlayers module to do so; you can assume that the maxExtent on the maps will disappear. Make sure to define projection and displayProjection with authority code, though.

Pol’s picture

Great, I think we'll need to release the beta5 sooner then :-)

augustus.kling’s picture

Status: Needs work » Needs review
FileSize
1.81 KB

maxExtent of maps is now derived from the maps' projection in the openlayers module. This means maxExtent will be ignored if provided as property of a map. Layers are still free to provide a maxExtent property in case they want a non-default tile grid but usually it's fine to omit the property for layers, too.

The attached patch is based on Pol's patch and untested as I don't have a Drupal with geofield available.

Pol’s picture

Testing it right now !

Thanks Augustus !

Pol’s picture

Augustus,

There's still a problem with the line 61 of openlayers.js.

If maxExtent are automatically calculated from projection, shouldn't we remove that line ?

FYI: I removed it and the map was displayed correctly.

augustus.kling’s picture

What is the problem with the line / what breaks when the line is in? It informs OpenLayers about the maxExtent that was derived from the projection object in PHP. If it is removed, OpenLayers does not know about the map extent and might fail to calculate a proper tile grid unless layers provide their maxExtent explicitly.

Pol’s picture

When using GeoField, map.maxExtent doesn't exists and so, the line 61 is causing troubles. (TypeError: Cannot read property '0' of undefined)

I'm trying to understand why:

   $map['maxExtent'] = $projection->getProjectedExtent();

(from openlayers_render_map_data())

Doesn't exists in Javascript anymore.

EDIT

Geofield is using:

function _geofield_openlayers_formatter($map_name, $items) {
  // ...
  // Get map preset
  $preset = openlayers_preset_load($map_name);
  $map = openlayers_build_map($preset->data);
  // ...

This function is not calling openlayers_render_map_data() and the map doesn't get populated with the maxExtent property.

A way to fix this would be to move all the projection stuff into the openlayers_build_map() function.

I know that we should put some order in those functions, maybe in the next beta, we should concentrate the population of the properties in one function, and the rendering of it, in another.

What do you think ?

augustus.kling’s picture

Exactly this line from the PHP should ensure that the maxExtent of the map is sensibly set. Then the line in the JavaScript file will not cause trouble. If the property gets lost before it reaches JavaScript, it needs to be fixed in PHP because this is where the bug hides.
I'm however unable to reproduce the disappearing maxExtent property. It makes it to the JavaScript objects just fine for me.

Pol’s picture

I've added more information in my previous post, posting this to send the notification email ;-)

augustus.kling’s picture

This function is not calling openlayers_render_map_data() and the map doesn't get populated with the maxExtent property.

A way to fix this would be to move all the projection stuff into the openlayers_build_map() function.

This sounds good to me. Moving everything that edits $map into openlayers_build_map would be cleaner.

Pol’s picture

Status: Needs review » Needs work

Ok, working on it :-)

Pol’s picture

Status: Needs work » Needs review

Looks like it's fixed, geofield + patch #21 is now working perfectly.

To all, if you can test it, it would be great, I need feedback before releasing beta5.

Thanks !

HongPong’s picture

This seems to have worked. i was on geofield 1.1 version and hit the same error, so i'm assuming an update is needed there too. when i switched to 2.x dev, updated DB schema, and put in the patch, well seems fine. thanks!

Veering a bit off topic - edit - re geocoder / geofield integration. While the patch got rid of the error - i'm having trouble actually displaying maps. I wonder if this projection stuff is the problem (i.e. this tutorial no dice) http://calibrate.be/labs/using-openlayers-and-geofield-display-maps-drup...

Should Map Projection & Display Projection be the same (i.e for Google Maps EPSG:3857 ). Under "Placeholder for Geofield Formatter" EPSG:3857 was not enabled when it was set to Raw. ... The upshot is that a little help text on this projection stuff would be good. Feel like I landed on the bleeding edge of a new feature at the wrong time.

Pol’s picture

This patch for Geofield 7.x-1.x, could you test it ?

eiriksm’s picture

#32 works for me (Geofield 1.1, not 2.0 beta5 as stated in patch name though ;)). I had to change a couple of exported features, and change to a new map projection for google maps to work. But this fixes my maps at least, and that is the important part.

Would you consider releasing a new 1.x version as well? So I can be on a stable version instead of a patched 1.1.

Thanks for all your hard work on these modules!

Pol’s picture

I will talk to Brandonian and we will probably release a version soon, I hope to see him today.

tomogden’s picture

Status: Needs review » Reviewed & tested by the community

This patch fixed my issues as well. Thanks @Pol.

Brandonian’s picture

Quick clarification, is the patch in #21 for 1.x or 2.x?

Pol’s picture

Patch #21 is for 2.x.

jcisio’s picture

In fact, the patch works with both branches.

joshf’s picture

@Pol thanks, #21 fixed it for me as well

Brandonian’s picture

Status: Reviewed & tested by the community » Fixed

Fixed on both 7.x-1.x and 7.x-2.x. Expect a new release for 1.x shortly, I want to wait until we get the Openlayers code handed off to the Openlayers module proper for another 2.x release.

2.x - http://drupalcode.org/project/geofield.git/commit/7b5c2b6 (Thanks @augustus.kling!)
1.x - http://drupalcode.org/project/geofield.git/commit/5856c32 (Thanks @Pol)

kingfisher64’s picture

Geofield 1.1 applying patch #32 outputs

patching file geofield.formatters.inc
Hunk #1 FAILED at 331.
Hunk #2 FAILED at 365.
2 out of 2 hunks FAILED -- saving rejects to file geofield.formatters.inc.rej
patching file geofield.install
patching file geofield.openlayers.inc
Hunk #1 FAILED at 77.
Hunk #2 FAILED at 146.
Hunk #3 FAILED at 155.
Hunk #4 FAILED at 193.
4 out of 4 hunks FAILED -- saving rejects to file geofield.openlayers.inc.rej

I'm not understanding how others have successfully applied this? I'm using drupal 7.22, geofield 1.1 (and the dev dated 7.x-1.x-dev 2013-Apr-07), openlayers beta5 - so the latest of everything stable.

I'm trying to correct the error:

Exception: Projection 900913 lacks an authority code. Read http://drupal.org/node/1944074 for hints. in openlayers_get_projection_by_identifier() (line 996 of sites/all/modules/openlayers/openlayers.module).

http://drupal.org/node/1960538 suggests http://drupal.org/node/1943968 which I believe in return suggests this thread. http://drupal.org/node/1944074 points here as well.

Could someone point me in the right direction if i'm doing something wrong. Many thanks.

Brandonian’s picture

@kingfisher64, the current dev has the patch properly applied

kingfisher64’s picture

That's what I thought. I've not tried patching that for this reason but did try using it and it's still showing the error message? I'm trying to use the feature ol_locator. But clicking on /locator view reveals this error.

http://mapping.kingfisher.so/locator shows the error.

ace11’s picture

I downloaded geofield 7.x-2.x-dev but I'm getting error when trying to run update.php.

An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: xxx/update.php?op=selection&token=sf9V0jWn2pJQuuJD0kdWkO5H11v1KvUuZ7oj1-BmZng&id=16&op=do StatusText: parsererror ResponseText: Fatal error: Call to undefined function openlayers_layers_load() in /sites/all/modules/openlayers/openlayers.install on line 378

Also tryed to update Openlayers to 7.x-2.0-beta5.

None of my maps are working because of: http://drupal.org/node/1944074

***EDIT***

I noticed that I did not have installed this module: http://drupal.org/project/proj4js
After that update.php works. So it is ok now :)

guillaumev’s picture

Version: 7.x-2.x-dev » 7.x-1.x-dev
Status: Fixed » Needs work

Reopening this because this patch, in version 1.x, causes http://drupal.org/node/1965484, because text columns in mysql can not have default values...

guillaumev’s picture

Status: Needs work » Closed (fixed)

In the end, I'm closing this and adding the discussion in http://drupal.org/node/1965484